From 8cbee1e1c3ab7f667193033064cf2480b912a809 Mon Sep 17 00:00:00 2001 From: Sylvie Crowe <107814465+oneirocosm@users.noreply.github.com> Date: Thu, 4 Jan 2024 10:16:26 -0800 Subject: [PATCH] Allow dots in alias and add port to canonical name (#209) * allow numerical start and allow dots in ssh alias We previously restricted the ssh alias to start with an alphabetic character and did not allow dots within it. This caused problems with users wanting to use an ip address as an alias. This lifts that restriction so both can freely be used. Note that while it is common to use the hostname as the alias, hostnames are not allowed to use the underscore character. However, we do allow the alias to contain an underscore. I do not think we can remove this from aliases now but it is something to watch out for in the future. * allow backslash in username This involved converting the regexp string into a raw string to able to use the backslash character. this is important. * Revert "allow backslash in username" This reverts commit cafe2812411cfba15b7ece1250a6def29d092366. Upon reflection, more time is needed to evaluate that this actually corrects the issue. It will be performed with proper diligence at a later time. * add port to end of canoncial names when not 22 The canonical name is the identifying key in the database, so it causes problems if another remote entry has the same canonical name. By adding the port number to the end of this, it is possible to differentiate the two. * add db migrations for adding port to canonicalname The up migration adds the port to the existing canonical id if it exists and is not 22. The down migration strips the port off the canonical name and deletes extra remotes with the same canonical name. If all remotes with that canonical name have been archived, it keeps the first one added to the database. If any have not been archived, it keeps the first added to the database from the non-archived group. * remove ability to edit port number via ssh import Previously, ssh imports could edit the port number since it was possible to change them in the config file without changin the canonical name. Now that the canonical name contains the port, a change in the port will simply create a new database entry. For this reason, the ability to modify the ssh port is dead code and has been removed. * allow backslash in username This involved converting the regexp string into a raw string to able to use the backslash character. this is important. * simplify up migration logic The previous up migration was suboptimal because it was accounting for a corner case not found in production. That case no longer needs to be considered, so the procedure was simplified. --- .../db/migrations/000029_canonicalport.down.sql | 12 ++++++++++++ wavesrv/db/migrations/000029_canonicalport.up.sql | 3 +++ wavesrv/pkg/cmdrunner/cmdrunner.go | 15 +++++++-------- wavesrv/pkg/sstore/dbops.go | 5 ----- wavesrv/pkg/sstore/migrate.go | 2 +- 5 files changed, 23 insertions(+), 14 deletions(-) create mode 100644 wavesrv/db/migrations/000029_canonicalport.down.sql create mode 100644 wavesrv/db/migrations/000029_canonicalport.up.sql diff --git a/wavesrv/db/migrations/000029_canonicalport.down.sql b/wavesrv/db/migrations/000029_canonicalport.down.sql new file mode 100644 index 00000000..35cf4fcd --- /dev/null +++ b/wavesrv/db/migrations/000029_canonicalport.down.sql @@ -0,0 +1,12 @@ +UPDATE remote +SET remotecanonicalname = SUBSTR(remotecanonicalname, 1, INSTR(remotecanonicalname, ':') - 1) +WHERE INSTR(remotecanonicalname, ':'); + +DELETE FROM remote +WHERE remoteid NOT IN ( + SELECT remoteid FROM ( + SELECT MIN(archived), remoteid, remotecanonicalname + FROM remote + GROUP BY remotecanonicalname + ) +); \ No newline at end of file diff --git a/wavesrv/db/migrations/000029_canonicalport.up.sql b/wavesrv/db/migrations/000029_canonicalport.up.sql new file mode 100644 index 00000000..085e1640 --- /dev/null +++ b/wavesrv/db/migrations/000029_canonicalport.up.sql @@ -0,0 +1,3 @@ +UPDATE remote +SET remotecanonicalname = remotecanonicalname || COALESCE( ":" || json_extract(sshopts, '$.sshport'), "") +WHERE json_extract(sshopts, '$.sshport') != 22; \ No newline at end of file diff --git a/wavesrv/pkg/cmdrunner/cmdrunner.go b/wavesrv/pkg/cmdrunner/cmdrunner.go index b7d94d20..d0af9581 100644 --- a/wavesrv/pkg/cmdrunner/cmdrunner.go +++ b/wavesrv/pkg/cmdrunner/cmdrunner.go @@ -115,8 +115,8 @@ var SetVarScopes = []SetVarScope{ {ScopeName: "remote", VarNames: []string{}}, } -var userHostRe = regexp.MustCompile("^(sudo@)?([a-z][a-z0-9._@-]*)@([a-z0-9][a-z0-9.-]*)(?::([0-9]+))?$") -var remoteAliasRe = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_-]*$") +var userHostRe = regexp.MustCompile(`^(sudo@)?([a-z][a-z0-9._@\\-]*)@([a-z0-9][a-z0-9.-]*)(?::([0-9]+))?$`) +var remoteAliasRe = regexp.MustCompile("^[a-zA-Z0-9][a-zA-Z0-9._-]*$") var genericNameRe = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_ .()<>,/\"'\\[\\]{}=+$@!*-]*$") var rendererRe = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_.:-]*$") var positionRe = regexp.MustCompile("^((S?\\+|E?-)?[0-9]+|(\\+|-|S|E))$") @@ -1257,6 +1257,9 @@ func parseRemoteEditArgs(isNew bool, pk *scpacket.FeCommandPacketType, isLocal b } sshOpts.SSHPort = portVal canonicalName = remoteUser + "@" + remoteHost + if portVal != 0 && portVal != 22 { + canonicalName = canonicalName + ":" + strconv.Itoa(portVal) + } if isSudo { canonicalName = "sudo@" + canonicalName } @@ -1528,7 +1531,8 @@ func NewHostInfo(hostName string) (*HostInfoType, error) { portStr, _ := ssh_config.GetStrict(hostName, "Port") var portVal int - if portStr != "" { + if portStr != "" && portStr != "22" { + canonicalName = canonicalName + ":" + portStr var err error portVal, err = strconv.Atoi(portStr) if err != nil { @@ -1539,7 +1543,6 @@ func NewHostInfo(hostName string) (*HostInfoType, error) { return nil, fmt.Errorf("could not parse port \"%d\": number is not valid for a port\n", portVal) } } - identityFile, _ := ssh_config.GetStrict(hostName, "IdentityFile") passwordAuth, _ := ssh_config.GetStrict(hostName, "PasswordAuthentication") @@ -1626,13 +1629,9 @@ func RemoteConfigParseCommand(ctx context.Context, pk *scpacket.FeCommandPacketT if previouslyImportedRemote != nil && !previouslyImportedRemote.Archived { // this already existed and was created via import // it needs to be updated instead of created - editMap := make(map[string]interface{}) editMap[sstore.RemoteField_Alias] = hostInfo.Host editMap[sstore.RemoteField_ConnectMode] = hostInfo.ConnectMode - // changing port is unique to imports because it lets us avoid conflicts - // if the port is changed in the ssh config - editMap[sstore.RemoteField_SSHPort] = hostInfo.Port if hostInfo.SshKeyFile != "" { editMap[sstore.RemoteField_SSHKey] = hostInfo.SshKeyFile } diff --git a/wavesrv/pkg/sstore/dbops.go b/wavesrv/pkg/sstore/dbops.go index 2ccb0d66..1f0dbd42 100644 --- a/wavesrv/pkg/sstore/dbops.go +++ b/wavesrv/pkg/sstore/dbops.go @@ -1704,7 +1704,6 @@ const ( RemoteField_ConnectMode = "connectmode" // string RemoteField_SSHKey = "sshkey" // string RemoteField_SSHPassword = "sshpassword" // string - RemoteField_SSHPort = "sshport" // string RemoteField_Color = "color" // string ) @@ -1736,10 +1735,6 @@ func UpdateRemote(ctx context.Context, remoteId string, editMap map[string]inter query = `UPDATE remote SET sshopts = json_set(sshopts, '$.sshpassword', ?) WHERE remoteid = ?` tx.Exec(query, sshPassword, remoteId) } - if sshPort, found := editMap[RemoteField_SSHPort]; found { - query = `UPDATE remote SET sshopts = json_set(sshopts, '$.sshport', ?) WHERE remoteid = ?` - tx.Exec(query, sshPort, remoteId) - } if color, found := editMap[RemoteField_Color]; found { query = `UPDATE remote SET remoteopts = json_set(remoteopts, '$.color', ?) WHERE remoteid = ?` tx.Exec(query, color, remoteId) diff --git a/wavesrv/pkg/sstore/migrate.go b/wavesrv/pkg/sstore/migrate.go index 91253a01..8e46cdcb 100644 --- a/wavesrv/pkg/sstore/migrate.go +++ b/wavesrv/pkg/sstore/migrate.go @@ -22,7 +22,7 @@ import ( "github.com/golang-migrate/migrate/v4" ) -const MaxMigration = 28 +const MaxMigration = 29 const MigratePrimaryScreenVersion = 9 const CmdScreenSpecialMigration = 13 const CmdLineSpecialMigration = 20