commit-archive password does not allow for all special characters
Closed, WontfixPublicBUG
Actions

Assigned To

Authored By

	c-po
	Oct 14 2023, 6:23 AM

Description

To reproduce

set system config-management commit-archive location 'scp://test123:foo%bar12/34!k@172.18.254.201/config/tftpboot'

The password / is treated wrongly and generates:

VyOS 1.3 equuleus

cpo@LR3.wue3# commit
Archiving config...
  scp://test123:foo%bar12/34!k@172.18.254.201/config/tftpboot Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python3/dist-packages/vyos/remote.py", line 312, in upload
    urlc(urlstring, *args, **kwargs).upload(local_path)
  File "/usr/lib/python3/dist-packages/vyos/remote.py", line 304, in urlc
    return url_classes[url.scheme](url, *args, **kwargs)
  File "/usr/lib/python3/dist-packages/vyos/remote.py", line 172, in __init__
    self.port = url.port or 22
  File "/usr/lib/python3.7/urllib/parse.py", line 169, in port
    port = int(port, 10)
ValueError: invalid literal for int() with base 10: 'foo%bar12'

VyOS 1.4 sagitta

cpo@LR2.wue3# set system config-management commit-archive location 'scp://test123:foo%bar12/34!k@172.18.254.201/config/tftpboot'

  "//test123:foo%bar12/34!k@172.18.254.201/config/tftpboot" is not a valid URI for the scp URL scheme


  Invalid value
  Value validation failed
  Set failed

I do not know which one is more bad, at least the 1.4 error will generate errors during image upgrades as now the validator fails

Details

Version: 1.3.3 1.4-rolling and 1.5-rolling
Is it a breaking change?: Perfectly compatible
Issue type: Bug (incorrect behavior)

Event Timeline

c-po created this task.Oct 14 2023, 6:23 AM

c-po renamed this task from commit-archive password doe not allow for all special characters to commit-archive password does not allow for all special characters.

pasik subscribed.Oct 14 2023, 8:08 AM

erkin claimed this task.Oct 19 2023, 2:14 PM

syncer edited projects, added VyOS 1.3 Equuleus (1.3.6); removed VyOS 1.3 Equuleus.Dec 17 2023, 11:33 PM

After some research, I have concluded that the error was caused by the string's violation of RFC 3986, which recommends using percent encoding instead. I'll look into the feasibility of implementing percent encoding/decoding (which urllib natively supports) for spicy characters if there's demand, but the recommended method in VyOS right now is using the protocol-agnostic REMOTE_USERNAME and REMOTE_PASSWORD environment variables, which bypass the URL decoding step. The alternative would involve messing with urllib.parse's internal parser to implement a laxer and less greedy regexp to accept passwords like this, which is probably a bad idea.