Repository: srl-labs/containerlab (go)

Issue:
Use `unbound` ssh option only for ssh clients supporting it.
When we started to use unbound option to mitigate SR OS SSH server not understanding modern ssh options we started to do this for all systems.

But apparently picky ssh clients bail out when they see unbound value being used which they don't recognize:

```
ssh -l admin clab-nc-sr1
/etc/ssh/ssh_config.d/clab-nc.conf line 6: unsupported option "unbound".

ssh -V
OpenSSH_8.0p1, OpenSSL 1.1.1k  FIPS 25 Mar 2021
```

I think we need to
1. identify which ssh client version started to support this option by running `ssh -V`
2. grep the version from it
3. and only set it for clients newer than the release that first started to support this option.

## OpenSSH version

OpenSSH version 8.9 is the first release where unbound option is supported:


```
OpenSSH_8.8p1, OpenSSL 1.1.1v  1 Aug 2023 - does not work
```

```
OpenSSH_8.9p1 Ubuntu-3ubuntu0.3, OpenSSL 3.0.2 15 Mar 2022 - works
```

```
OpenSSH_9.0p1, OpenSSL 1.1.1v  1 Aug 2023 - works
```

Pull request description:
Remove unbound ssh option for older clients
fix #1719 

Patch:
diff --git a/clab/ssh_config.go.tpl b/clab/ssh_config.go.tpl
new file mode 100644
index 00000000..20a4cca7
--- /dev/null
+++ b/clab/ssh_config.go.tpl
@@ -0,0 +1,13 @@
+# Containerlab SSH Config for the {{ .TopologyName }} lab
+
+{{- range  .Nodes }}
+Host {{ .Name }}
+	{{-  if ne .Username ""}}
+	User {{ .Username }}
+	{{- end }}
+	StrictHostKeyChecking=no
+	UserKnownHostsFile=/dev/null
+	{{- if ne .SSHConfig.PubkeyAuthentication "" }}
+	PubkeyAuthentication={{ .SSHConfig.PubkeyAuthentication.String }}
+	{{- end }}
+{{ end }}
\ No newline at end of file
diff --git a/clab/sshconfig.go b/clab/sshconfig.go
index 2536d1c0..f58eea58 100644
--- a/clab/sshconfig.go
+++ b/clab/sshconfig.go
@@ -1,6 +1,7 @@
 package clab
 
 import (
+	_ "embed"
 	"os"
 	"path"
 	"text/template"
@@ -8,6 +9,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/srl-labs/containerlab/types"
 	"github.com/srl-labs/containerlab/utils"
+	"golang.org/x/mod/semver"
 )
 
 // SSHConfigTmpl is the top-level data structure for the
@@ -25,20 +27,10 @@ type SSHConfigNodeTmpl struct {
 	SSHConfig *types.SSHConfig
 }
 
-// tmplSshConfig is the SSH config template.
-const tmplSshConfig = `# Containerlab SSH Config for the {{ .TopologyName }} lab
-
-{{- range  .Nodes }}
-Host {{ .Name }}
-	{{-  if ne .Username ""}}
-	User {{ .Username }}
-	{{- end }}
-	StrictHostKeyChecking=no
-	UserKnownHostsFile=/dev/null
-	{{- if ne .SSHConfig.PubkeyAuthentication "" }}
-	PubkeyAuthentication={{ .SSHConfig.PubkeyAuthentication.String }}
-	{{- end }}
-{{ end }}`
+// sshConfigTemplate is the SSH config template.
+//
+//go:embed ssh_config.go.tpl
+var sshConfigTemplate string
 
 // RemoveSSHConfig removes the lab specific ssh config file
 func (c *CLab) RemoveSSHConfig(topoPaths *types.TopoPaths) error {
@@ -63,6 +55,12 @@ func (c *CLab) AddSSHConfig() error {
 		Nodes:        make([]SSHConfigNodeTmpl, 0, len(c.Nodes)),
 	}
 
+	// get the ssh client version to determine if are allowed
+	// to use the PubkeyAuthentication=unbound
+	// which is only available in OpenSSH 8.9+
+	// if we fail to parse the version the return value is going to be empty
+	sshVersion := utils.GetSSHVersion()
+
 	// add the data for all nodes to the template input
 	for _, n := range c.Nodes {
 		// get the Kind from the KindRegistry and and extract
@@ -73,10 +71,19 @@ func (c *CLab) AddSSHConfig() error {
 			Username:  NodeRegistryEntry.Credentials().GetUsername(),
 			SSHConfig: n.GetSSHConfig(),
 		}
+
+		// if we couldn't parse the ssh version we assume we can't use unbound option
+		// or if the version is lower than 8.9
+		// and the node has the PubkeyAuthentication set to unbound
+		// we set it to empty string since it is not supported by the SSH client
+		if (sshVersion == "" || semver.Compare("v"+sshVersion, "v8.9") < 0) && nodeData.SSHConfig.PubkeyAuthentication == types.PubkeyAuthValueUnbound {
+			nodeData.SSHConfig.PubkeyAuthentication = ""
+		}
+
 		tmpl.Nodes = append(tmpl.Nodes, nodeData)
 	}
 
-	t, err := template.New("sshconfig").Parse(tmplSshConfig)
+	t, err := template.New("sshconfig").Parse(sshConfigTemplate)
 	if err != nil {
 		return err
 	}
diff --git a/docs/manual/kinds/vr-sros.md b/docs/manual/kinds/vr-sros.md
index 0a5d51cf..5b635162 100644
--- a/docs/manual/kinds/vr-sros.md
+++ b/docs/manual/kinds/vr-sros.md
@@ -335,6 +335,12 @@ A:admin@sros1# info | match boot-goo
 
 By combining file bindings and the automatic script execution of SROS it is possible to create a workaround for persistent BOF settings.
 
+#### SSH keys
+
+Containerlab v0.48.0+ supports SSH key injection into the Nokia SR OS nodes. First containerlab retrieves all public keys from `~/.ssh` directory of a user running containerlab, then it retrieves public keys from the ssh agent if one is running.
+
+Next it will filter out public keys that are not of RSA/ECDSA type. The remaining valid public keys will be configured for the admin user of the Nokia SR OS node. This will enable key-based authentication next time you connect to the node.
+
 ### License
 
 Path to a valid license must be provided for all Nokia SR OS nodes with a [`license`](../nodes.md#license) directive.
diff --git a/utils/ssh.go b/utils/ssh.go
new file mode 100644
index 00000000..ed639957
--- /dev/null
+++ b/utils/ssh.go
@@ -0,0 +1,34 @@
+package utils
+
+import (
+	"os/exec"
+	"regexp"
+
+	log "github.com/sirupsen/logrus"
+)
+
+// GetSSHVersion returns the version of the ssh client
+// that is installed on the host.
+func GetSSHVersion() string {
+	cmd := exec.Command("ssh", "-V")
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		log.Warnf("Failed to get ssh client version: %v", err)
+	}
+
+	version := parseSSHVersion(string(out))
+
+	return version
+}
+
+func parseSSHVersion(in string) string {
+	re := regexp.MustCompile(`OpenSSH_(\d+\.\d+).+`)
+	match := re.FindStringSubmatch(in)
+
+	if len(match) < 2 {
+		log.Warnf("Failed to parse ssh version from string: %s", in)
+		return ""
+	}
+
+	return match[1]
+}


Test patch:
diff --git a/utils/ssh_test.go b/utils/ssh_test.go
new file mode 100644
index 00000000..ed1d04b5
--- /dev/null
+++ b/utils/ssh_test.go
@@ -0,0 +1,38 @@
+package utils
+
+import (
+	"testing"
+)
+
+func TestParseSSHVersion(t *testing.T) {
+	tests := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{
+			name: "valid version",
+			in:   "OpenSSH_8.1p1 Debian-8, OpenSSL 1.1.1d  10 Sep 2019",
+			want: "8.1",
+		},
+		{
+			name: "another valid version",
+			in:   "OpenSSH_8.9p1 Ubuntu-3ubuntu0.3, OpenSSL 3.0.2 15 Mar 2022",
+			want: "8.9",
+		},
+		{
+			name: "invalid version",
+			in:   "Invalid version string",
+			want: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := parseSSHVersion(tt.in)
+			if got != tt.want {
+				t.Errorf("parseSSHVersion() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}