mirror of
https://github.com/netbirdio/netbird.git
synced 2026-07-18 20:49:56 +00:00
[client] Sanitize peer FQDN/hostname in generated SSH config (#6805)
## Describe your changes Validate peer-supplied FQDN and hostname before they are written into the generated NetBird SSH client config (`client/ssh/config/manager.go`). These values originate from remote peers and were previously written verbatim into the config; malformed values (e.g. containing unexpected characters) could produce a broken or unintended config. FQDN/hostname are now checked with `domain.IsValidDomainNoWildcard`, and invalid, non-empty values are dropped with a warning. IPs are unaffected (already validated `netip.Addr`). Added a test covering malformed hostnames. ## Issue ticket number and link Internal input-validation hardening for peer-supplied hostnames in the generated SSH client config (`client/ssh/config/manager.go`). ## Stack - \#6726 <!-- branch-stack --> - \#6805 :point\_left: ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) Internal client SSH config generation. No public API, gRPC, CLI/service flag, or configuration change — only input validation on peer-supplied hostnames before they are written to the generated ssh\_config. ### Docs PR URL (required if "docs added" is checked) Paste the PR link from <https://github.com/netbirdio/docs> here: N/A
This commit is contained in:
@@ -14,6 +14,7 @@ import (
|
||||
log "github.com/sirupsen/logrus"
|
||||
|
||||
nbssh "github.com/netbirdio/netbird/client/ssh"
|
||||
"github.com/netbirdio/netbird/shared/management/domain"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -218,11 +219,20 @@ func (m *Manager) buildHostPatterns(peer PeerSSHInfo) []string {
|
||||
if peer.IPv6.IsValid() {
|
||||
hostPatterns = append(hostPatterns, peer.IPv6.String())
|
||||
}
|
||||
if peer.FQDN != "" {
|
||||
// Peer FQDNs and hostnames originate from remote peers, so they must be
|
||||
// validated as plain DNS names before being embedded in the ssh_config
|
||||
// "Match host" pattern list. This prevents injection of arbitrary
|
||||
// ssh_config directives via embedded quotes, whitespace, newlines, the
|
||||
// comma pattern separator, or the "*"/"?" pattern metacharacters.
|
||||
if domain.IsValidDomainNoWildcard(peer.FQDN) {
|
||||
hostPatterns = append(hostPatterns, peer.FQDN)
|
||||
} else if peer.FQDN != "" {
|
||||
log.Warnf("skipping peer FQDN with invalid characters in SSH config: %q", peer.FQDN)
|
||||
}
|
||||
if peer.Hostname != "" && peer.Hostname != peer.FQDN {
|
||||
if peer.Hostname != peer.FQDN && domain.IsValidDomainNoWildcard(peer.Hostname) {
|
||||
hostPatterns = append(hostPatterns, peer.Hostname)
|
||||
} else if peer.Hostname != "" && peer.Hostname != peer.FQDN {
|
||||
log.Warnf("skipping peer hostname with invalid characters in SSH config: %q", peer.Hostname)
|
||||
}
|
||||
return hostPatterns
|
||||
}
|
||||
|
||||
@@ -148,6 +148,45 @@ func TestManager_MatchHostFormat(t *testing.T) {
|
||||
"should use Match host with comma-separated patterns")
|
||||
}
|
||||
|
||||
func TestManager_HostPatternInjection(t *testing.T) {
|
||||
tempDir, err := os.MkdirTemp("", "netbird-ssh-config-test")
|
||||
require.NoError(t, err)
|
||||
defer func() { assert.NoError(t, os.RemoveAll(tempDir)) }()
|
||||
|
||||
manager := &Manager{
|
||||
sshConfigDir: filepath.Join(tempDir, "ssh_config.d"),
|
||||
sshConfigFile: "99-netbird.conf",
|
||||
}
|
||||
|
||||
// A malicious peer FQDN/hostname attempts to break out of the Match host
|
||||
// directive and inject arbitrary ssh_config (a ProxyCommand executing a
|
||||
// command). It must be rejected, not written to the config.
|
||||
peers := []PeerSSHInfo{
|
||||
{
|
||||
Hostname: "evil\"\n ProxyCommand touch /tmp/pwned\nHost x",
|
||||
IP: netip.MustParseAddr("100.125.1.1"),
|
||||
FQDN: "evil\"\n ProxyCommand touch /tmp/pwned\nHost x.nb.internal",
|
||||
},
|
||||
{Hostname: "peer2", IP: netip.MustParseAddr("100.125.1.2"), FQDN: "peer2.nb.internal"},
|
||||
}
|
||||
|
||||
err = manager.SetupSSHClientConfig(peers)
|
||||
require.NoError(t, err)
|
||||
|
||||
configPath := filepath.Join(manager.sshConfigDir, manager.sshConfigFile)
|
||||
content, err := os.ReadFile(configPath)
|
||||
require.NoError(t, err)
|
||||
configStr := string(content)
|
||||
|
||||
assert.NotContains(t, configStr, "ProxyCommand touch /tmp/pwned",
|
||||
"injected directive must not appear in generated config")
|
||||
assert.NotContains(t, configStr, "evil",
|
||||
"malicious pattern must be dropped entirely")
|
||||
// The valid peer must still be present, on a single Match host line.
|
||||
assert.Contains(t, configStr, "Match host \"100.125.1.1,100.125.1.2,peer2.nb.internal,peer2\"",
|
||||
"valid peers must survive, injected patterns dropped")
|
||||
}
|
||||
|
||||
func TestManager_ForcedSSHConfig(t *testing.T) {
|
||||
// Set force environment variable
|
||||
t.Setenv(EnvForceSSHConfig, "true")
|
||||
|
||||
Reference in New Issue
Block a user