From ce6a51468a7edddac4259eea66d271a3e7a57e02 Mon Sep 17 00:00:00 2001 From: Fabian Hardt Date: Thu, 4 Jun 2026 07:32:47 +0200 Subject: [PATCH] sftpd: support SSH user certificates signed by a trusted CA (#9815) * sftpd: support SSH user certificates signed by a trusted CA Adds a new "certificate" auth method to weed sftp. When enabled, the server loads trusted CA public keys from -trustedUserCAKeysFile (OpenSSH authorized_keys format, one or more keys) and accepts only ssh.Certificate blobs of type UserCert on the public-key channel. Validation uses ssh.CertChecker: CA signature, ValidAfter/ValidBefore, non-empty ValidPrincipals and SSH login user must appear in ValidPrincipals. The authenticated user must exist in the user store; home dir and permissions resolve as before. Behaviour mirrors MinIO's --sftp=trusted-user-ca-key and OpenSSH's TrustedUserCAKeys: when certificate auth is active, plain (non-cert) public keys are rejected even if "publickey" is also listed. Default authMethods remain "password,publickey", so existing deployments are unaffected. * Update weed/sftpd/auth/certificate.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * sftpd: address review feedback on certificate auth - Pre-marshal trusted CA public keys in IsUserAuthority instead of re-marshaling on every authentication attempt (gemini-code-assist). - Differentiate user-not-found from underlying store errors via errors.As(*user.UserNotFoundError) so backend/read failures are no longer reported as bad credentials (coderabbitai). - Fix the corresponding sanity check in the missing-file test to use errors.As instead of errors.Is (UserNotFoundError has no Is method, so the previous check never matched) (coderabbitai). * sftpd: register trustedUserCAKeysFile flag in filer and server commands The new field on SftpOptions is dereferenced unconditionally in resolvePaths(), but only the standalone `weed sftp` command was wiring its flag. `weed filer` and `weed server` both embed an SftpOptions value and call resolvePaths() on it, so they hit a nil pointer dereference at startup. Register `-sftp.trustedUserCAKeysFile` in both commands and update the -sftp.authMethods help text to mention the new "certificate" method. Fixes the SFTP Integration Tests CI failure on this PR. * helm: expose SFTP certificate auth in the SeaweedFS chart Adds Helm-chart support for the new SSH user-certificate auth method: - values.yaml (sftp:) gains `trustedUserCAKeys` (inline OpenSSH authorized_keys-format CA public keys) and `existingCAKeysSecret` (reference an externally managed Secret). Same pair added under allInOne.sftp with a null default that falls back to the top-level sftp.* setting. - New template templates/sftp/sftp-ca-secret.yaml renders a chart-managed Secret -sftp-ca-secret with `ca_user.pub`, but only when SFTP is enabled, "certificate" is in authMethods, inline keys are provided, and no existingCAKeysSecret is set. - templates/sftp/sftp-deployment.yaml and the all-in-one deployment template add `-trustedUserCAKeysFile=/etc/sw/sftp_ca/ca_user.pub` to the weed sftp command, mount the CA secret at /etc/sw/sftp_ca and add the corresponding volume. All cert-auth bits are guarded by `contains "certificate" authMethods` so existing users see no change. - authMethods help text updated to mention "certificate". Verified end-to-end on a local k3d cluster: cert login succeeds, plain-pubkey login is rejected with "public key without certificate not allowed". * helm: fail render when SFTP certificate auth lacks CA keys When certificate is in authMethods but neither trustedUserCAKeys nor existingCAKeysSecret is set, the deployment mounted a secret that the chart never renders, leaving the pod stuck on a missing volume. Fail at template time with a clear message instead. * sftpd: fix stale auth-method list in SFTPServiceOptions comment keyboard-interactive was never implemented; certificate is the new supported method. Match the CLI help text. * sftpd: test Manager wiring of certificate vs public-key channel Cover the channel takeover at the Manager level: certificate auth displaces plain public-key auth when both are enabled, public-key auth stays put otherwise, and enabling certificate without a CA file errors. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Chris Lu --- .../all-in-one/all-in-one-deployment.yaml | 21 ++ .../templates/sftp/sftp-ca-secret.yaml | 40 +++ .../templates/sftp/sftp-deployment.yaml | 21 ++ k8s/charts/seaweedfs/values.yaml | 18 +- weed/command/filer.go | 3 +- weed/command/server.go | 3 +- weed/command/sftp.go | 38 ++- weed/sftpd/auth/auth.go | 31 +- weed/sftpd/auth/auth_test.go | 79 +++++ weed/sftpd/auth/certificate.go | 150 +++++++++ weed/sftpd/auth/certificate_test.go | 312 ++++++++++++++++++ weed/sftpd/sftp_service.go | 11 +- 12 files changed, 700 insertions(+), 27 deletions(-) create mode 100644 k8s/charts/seaweedfs/templates/sftp/sftp-ca-secret.yaml create mode 100644 weed/sftpd/auth/auth_test.go create mode 100644 weed/sftpd/auth/certificate.go create mode 100644 weed/sftpd/auth/certificate_test.go diff --git a/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml b/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml index b0d1fc784..d8aa7b960 100644 --- a/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml +++ b/k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml @@ -272,6 +272,9 @@ spec: {{- $authMethods := .Values.allInOne.sftp.authMethods | default .Values.sftp.authMethods }} {{- if $authMethods }} -sftp.authMethods={{ $authMethods }} \ + {{- if contains "certificate" $authMethods }} + -sftp.trustedUserCAKeysFile=/etc/sw/sftp_ca/ca_user.pub \ + {{- end }} {{- end }} {{- $maxAuthTries := .Values.allInOne.sftp.maxAuthTries | default .Values.sftp.maxAuthTries }} {{- if $maxAuthTries }} @@ -319,6 +322,12 @@ spec: name: config-users readOnly: true {{- end }} + {{- $aioAuthMethods := .Values.allInOne.sftp.authMethods | default .Values.sftp.authMethods }} + {{- if and $aioAuthMethods (contains "certificate" $aioAuthMethods) }} + - mountPath: /etc/sw/sftp_ca + name: config-sftp-ca + readOnly: true + {{- end }} {{- end }} {{- if .Values.filer.notificationConfig }} - name: notification-config @@ -454,6 +463,18 @@ spec: defaultMode: 420 secretName: {{ default (printf "%s-sftp-secret" (include "seaweedfs.fullname" .)) (or .Values.allInOne.sftp.existingConfigSecret .Values.sftp.existingConfigSecret) }} {{- end }} + {{- $aioAuthMethodsVol := .Values.allInOne.sftp.authMethods | default .Values.sftp.authMethods }} + {{- if and $aioAuthMethodsVol (contains "certificate" $aioAuthMethodsVol) }} + {{- $aioCASecret := or .Values.allInOne.sftp.existingCAKeysSecret .Values.sftp.existingCAKeysSecret }} + {{- $aioCAKeys := or .Values.allInOne.sftp.trustedUserCAKeys .Values.sftp.trustedUserCAKeys }} + {{- if and (not $aioCASecret) (not $aioCAKeys) }} + {{- fail "allInOne.sftp.authMethods includes \"certificate\" but neither trustedUserCAKeys nor existingCAKeysSecret is set" }} + {{- end }} + - name: config-sftp-ca + secret: + defaultMode: 420 + secretName: {{ default (printf "%s-sftp-ca-secret" (include "seaweedfs.fullname" .)) $aioCASecret }} + {{- end }} {{- end }} {{- if .Values.filer.notificationConfig }} - name: notification-config diff --git a/k8s/charts/seaweedfs/templates/sftp/sftp-ca-secret.yaml b/k8s/charts/seaweedfs/templates/sftp/sftp-ca-secret.yaml new file mode 100644 index 000000000..2149256cf --- /dev/null +++ b/k8s/charts/seaweedfs/templates/sftp/sftp-ca-secret.yaml @@ -0,0 +1,40 @@ +{{- include "seaweedfs.compat" . -}} +{{- /* +Render a chart-managed Secret carrying the SSH user CA public key(s) for +certificate-based SFTP authentication, but only when: + - SFTP (standalone or all-in-one) is enabled, + - "certificate" is in the relevant authMethods list, + - the user provided inline CA keys via sftp.trustedUserCAKeys, and + - no existingCAKeysSecret is set (which would supersede this Secret). +*/}} +{{- $sftpEnabled := or .Values.sftp.enabled (and .Values.allInOne.enabled .Values.allInOne.sftp.enabled) -}} +{{- $authMethods := .Values.sftp.authMethods -}} +{{- if and .Values.allInOne.enabled .Values.allInOne.sftp.authMethods -}} +{{- $authMethods = .Values.allInOne.sftp.authMethods -}} +{{- end -}} +{{- $certInMethods := and $authMethods (contains "certificate" $authMethods) -}} +{{- $inlineCAKeys := .Values.sftp.trustedUserCAKeys -}} +{{- if and .Values.allInOne.enabled .Values.allInOne.sftp.trustedUserCAKeys -}} +{{- $inlineCAKeys = .Values.allInOne.sftp.trustedUserCAKeys -}} +{{- end -}} +{{- $existingSecret := .Values.sftp.existingCAKeysSecret -}} +{{- if and .Values.allInOne.enabled .Values.allInOne.sftp.existingCAKeysSecret -}} +{{- $existingSecret = .Values.allInOne.sftp.existingCAKeysSecret -}} +{{- end -}} +{{- if and $sftpEnabled $certInMethods $inlineCAKeys (not $existingSecret) }} +apiVersion: v1 +kind: Secret +type: Opaque +metadata: + name: {{ include "seaweedfs.fullname" . }}-sftp-ca-secret + namespace: {{ .Release.Namespace }} + labels: + app.kubernetes.io/name: {{ template "seaweedfs.name" . }} + helm.sh/chart: {{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }} + app.kubernetes.io/managed-by: {{ .Release.Service }} + app.kubernetes.io/instance: {{ .Release.Name }} + app.kubernetes.io/component: sftp +stringData: + ca_user.pub: | +{{ $inlineCAKeys | indent 4 }} +{{- end }} diff --git a/k8s/charts/seaweedfs/templates/sftp/sftp-deployment.yaml b/k8s/charts/seaweedfs/templates/sftp/sftp-deployment.yaml index 46bb32207..736963a5b 100644 --- a/k8s/charts/seaweedfs/templates/sftp/sftp-deployment.yaml +++ b/k8s/charts/seaweedfs/templates/sftp/sftp-deployment.yaml @@ -135,6 +135,9 @@ spec: {{- end }} {{- if .Values.sftp.authMethods }} -authMethods={{ .Values.sftp.authMethods }} \ + {{- if contains "certificate" .Values.sftp.authMethods }} + -trustedUserCAKeysFile=/etc/sw/sftp_ca/ca_user.pub \ + {{- end }} {{- end }} {{- if .Values.sftp.maxAuthTries }} -maxAuthTries={{ .Values.sftp.maxAuthTries }} \ @@ -176,6 +179,11 @@ spec: - mountPath: /etc/sw/ssh name: config-ssh readOnly: true + {{- if and .Values.sftp.authMethods (contains "certificate" .Values.sftp.authMethods) }} + - mountPath: /etc/sw/sftp_ca + name: config-sftp-ca + readOnly: true + {{- end }} {{- if include "seaweedfs.securityConfigEnabled" . }} - name: security-config readOnly: true @@ -256,6 +264,19 @@ spec: {{- else }} secretName: {{ include "seaweedfs.fullname" . }}-sftp-ssh-secret {{- end }} + {{- if and .Values.sftp.authMethods (contains "certificate" .Values.sftp.authMethods) }} + {{- if and (not .Values.sftp.existingCAKeysSecret) (not .Values.sftp.trustedUserCAKeys) }} + {{- fail "sftp.authMethods includes \"certificate\" but neither sftp.trustedUserCAKeys nor sftp.existingCAKeysSecret is set" }} + {{- end }} + - name: config-sftp-ca + secret: + defaultMode: 420 + {{- if .Values.sftp.existingCAKeysSecret }} + secretName: {{ .Values.sftp.existingCAKeysSecret }} + {{- else }} + secretName: {{ include "seaweedfs.fullname" . }}-sftp-ca-secret + {{- end }} + {{- end }} {{- if eq .Values.sftp.logs.type "hostPath" }} - name: logs hostPath: diff --git a/k8s/charts/seaweedfs/values.yaml b/k8s/charts/seaweedfs/values.yaml index 5871f9e24..e2f83384e 100644 --- a/k8s/charts/seaweedfs/values.yaml +++ b/k8s/charts/seaweedfs/values.yaml @@ -1138,7 +1138,7 @@ sftp: # SSH server configuration sshPrivateKey: "/etc/sw/seaweedfs_sftp_ssh_private_key" # Path to the SSH private key file for host authentication hostKeysFolder: "/etc/sw/ssh" # path to folder containing SSH private key files for host authentication - authMethods: "password,publickey" # Comma-separated list of allowed auth methods: password, publickey, keyboard-interactive + authMethods: "password,publickey" # Comma-separated list of allowed auth methods: password, publickey, certificate maxAuthTries: 6 # Maximum number of authentication attempts per connection bannerMessage: "SeaweedFS SFTP Server" # Message displayed before authentication loginGraceTime: "2m" # Timeout for authentication @@ -1155,6 +1155,18 @@ sftp: # Set to the name of an existing kubernetes Secret with the list of ssh private keys for sftp existingSshConfigSecret: null + # SSH user-certificate authentication (CA-signed user certs). Mirrors + # OpenSSH `TrustedUserCAKeys` and MinIO `--sftp=trusted-user-ca-key`. + # Add "certificate" to `authMethods` to activate; when active, plain + # public keys are rejected on the public-key channel. + # + # Inline CA public keys in OpenSSH authorized_keys format (one per + # line). Ignored when `existingCAKeysSecret` is set. + trustedUserCAKeys: "" + # Set to the name of an existing kubernetes Secret carrying the CA + # public keys under data key `ca_user.pub`. + existingCAKeysSecret: null + # Additional resources sidecars: [] initContainers: "" @@ -1545,6 +1557,10 @@ allInOne: existingConfigSecret: null # Set to the name of an existing kubernetes Secret with the SSH keys existingSshConfigSecret: null + # SSH user-certificate authentication. See sftp.trustedUserCAKeys above. + # (null on either field inherits from the top-level sftp.* setting.) + trustedUserCAKeys: null + existingCAKeysSecret: null # Service settings service: diff --git a/weed/command/filer.go b/weed/command/filer.go index 8412feaf4..bc9098ba8 100644 --- a/weed/command/filer.go +++ b/weed/command/filer.go @@ -178,13 +178,14 @@ func init() { filerSftpOptions.port = cmdFiler.Flag.Int("sftp.port", 2022, "SFTP server listen port") filerSftpOptions.sshPrivateKey = cmdFiler.Flag.String("sftp.sshPrivateKey", "", "path to the SSH private key file for host authentication") filerSftpOptions.hostKeysFolder = cmdFiler.Flag.String("sftp.hostKeysFolder", "", "path to folder containing SSH private key files for host authentication") - filerSftpOptions.authMethods = cmdFiler.Flag.String("sftp.authMethods", "password,publickey", "comma-separated list of allowed auth methods: password, publickey, keyboard-interactive") + filerSftpOptions.authMethods = cmdFiler.Flag.String("sftp.authMethods", "password,publickey", "comma-separated list of allowed auth methods: password, publickey, certificate") filerSftpOptions.maxAuthTries = cmdFiler.Flag.Int("sftp.maxAuthTries", 6, "maximum number of authentication attempts per connection") filerSftpOptions.bannerMessage = cmdFiler.Flag.String("sftp.bannerMessage", "SeaweedFS SFTP Server - Unauthorized access is prohibited", "message displayed before authentication") filerSftpOptions.loginGraceTime = cmdFiler.Flag.Duration("sftp.loginGraceTime", 2*time.Minute, "timeout for authentication") filerSftpOptions.clientAliveInterval = cmdFiler.Flag.Duration("sftp.clientAliveInterval", 5*time.Second, "interval for sending keep-alive messages") filerSftpOptions.clientAliveCountMax = cmdFiler.Flag.Int("sftp.clientAliveCountMax", 3, "maximum number of missed keep-alive messages before disconnecting") filerSftpOptions.userStoreFile = cmdFiler.Flag.String("sftp.userStoreFile", "", "path to JSON file containing user credentials and permissions") + filerSftpOptions.trustedUserCAKeysFile = cmdFiler.Flag.String("sftp.trustedUserCAKeysFile", "", "path to a file with trusted user CA public keys (OpenSSH authorized_keys format); required when 'certificate' is in -sftp.authMethods") filerSftpOptions.dataCenter = cmdFiler.Flag.String("sftp.dataCenter", "", "prefer to read and write to volumes in this data center") filerSftpOptions.bindIp = cmdFiler.Flag.String("sftp.ip.bind", "", "ip address to bind to. If empty, default to same as -ip.bind option.") filerSftpOptions.localSocket = cmdFiler.Flag.String("sftp.localSocket", "", "default to /tmp/seaweedfs-sftp-.sock") diff --git a/weed/command/server.go b/weed/command/server.go index b878c97ff..b178cde30 100644 --- a/weed/command/server.go +++ b/weed/command/server.go @@ -188,13 +188,14 @@ func init() { sftpOptions.port = cmdServer.Flag.Int("sftp.port", 2022, "SFTP server listen port") sftpOptions.sshPrivateKey = cmdServer.Flag.String("sftp.sshPrivateKey", "", "path to the SSH private key file for host authentication") sftpOptions.hostKeysFolder = cmdServer.Flag.String("sftp.hostKeysFolder", "", "path to folder containing SSH private key files for host authentication") - sftpOptions.authMethods = cmdServer.Flag.String("sftp.authMethods", "password,publickey", "comma-separated list of allowed auth methods: password, publickey, keyboard-interactive") + sftpOptions.authMethods = cmdServer.Flag.String("sftp.authMethods", "password,publickey", "comma-separated list of allowed auth methods: password, publickey, certificate") sftpOptions.maxAuthTries = cmdServer.Flag.Int("sftp.maxAuthTries", 6, "maximum number of authentication attempts per connection") sftpOptions.bannerMessage = cmdServer.Flag.String("sftp.bannerMessage", "SeaweedFS SFTP Server - Unauthorized access is prohibited", "message displayed before authentication") sftpOptions.loginGraceTime = cmdServer.Flag.Duration("sftp.loginGraceTime", 2*time.Minute, "timeout for authentication") sftpOptions.clientAliveInterval = cmdServer.Flag.Duration("sftp.clientAliveInterval", 5*time.Second, "interval for sending keep-alive messages") sftpOptions.clientAliveCountMax = cmdServer.Flag.Int("sftp.clientAliveCountMax", 3, "maximum number of missed keep-alive messages before disconnecting") sftpOptions.userStoreFile = cmdServer.Flag.String("sftp.userStoreFile", "", "path to JSON file containing user credentials and permissions") + sftpOptions.trustedUserCAKeysFile = cmdServer.Flag.String("sftp.trustedUserCAKeysFile", "", "path to a file with trusted user CA public keys (OpenSSH authorized_keys format); required when 'certificate' is in -sftp.authMethods") sftpOptions.localSocket = cmdServer.Flag.String("sftp.localSocket", "", "default to /tmp/seaweedfs-sftp-.sock") iamOptions.port = cmdServer.Flag.Int("iam.port", 8111, "iam server http listen port") diff --git a/weed/command/sftp.go b/weed/command/sftp.go index 142b1927b..e15f6ad4d 100644 --- a/weed/command/sftp.go +++ b/weed/command/sftp.go @@ -25,22 +25,23 @@ var ( // SftpOptions holds configuration options for the SFTP server. type SftpOptions struct { - filer *string - bindIp *string - port *int - sshPrivateKey *string - hostKeysFolder *string - authMethods *string - maxAuthTries *int - bannerMessage *string - loginGraceTime *time.Duration - clientAliveInterval *time.Duration - clientAliveCountMax *int - userStoreFile *string - dataCenter *string - metricsHttpPort *int - metricsHttpIp *string - localSocket *string + filer *string + bindIp *string + port *int + sshPrivateKey *string + hostKeysFolder *string + authMethods *string + maxAuthTries *int + bannerMessage *string + loginGraceTime *time.Duration + clientAliveInterval *time.Duration + clientAliveCountMax *int + userStoreFile *string + trustedUserCAKeysFile *string + dataCenter *string + metricsHttpPort *int + metricsHttpIp *string + localSocket *string } // cmdSftp defines the SFTP command similar to the S3 command. @@ -64,13 +65,14 @@ func init() { sftpOptionsStandalone.port = cmdSftp.Flag.Int("port", 2022, "SFTP server listen port") sftpOptionsStandalone.sshPrivateKey = cmdSftp.Flag.String("sshPrivateKey", "", "path to the SSH private key file for host authentication") sftpOptionsStandalone.hostKeysFolder = cmdSftp.Flag.String("hostKeysFolder", "", "path to folder containing SSH private key files for host authentication") - sftpOptionsStandalone.authMethods = cmdSftp.Flag.String("authMethods", "password,publickey", "comma-separated list of allowed auth methods: password, publickey, keyboard-interactive") + sftpOptionsStandalone.authMethods = cmdSftp.Flag.String("authMethods", "password,publickey", "comma-separated list of allowed auth methods: password, publickey, certificate") sftpOptionsStandalone.maxAuthTries = cmdSftp.Flag.Int("maxAuthTries", 6, "maximum number of authentication attempts per connection") sftpOptionsStandalone.bannerMessage = cmdSftp.Flag.String("bannerMessage", "SeaweedFS SFTP Server - Unauthorized access is prohibited", "message displayed before authentication") sftpOptionsStandalone.loginGraceTime = cmdSftp.Flag.Duration("loginGraceTime", 2*time.Minute, "timeout for authentication") sftpOptionsStandalone.clientAliveInterval = cmdSftp.Flag.Duration("clientAliveInterval", 5*time.Second, "interval for sending keep-alive messages") sftpOptionsStandalone.clientAliveCountMax = cmdSftp.Flag.Int("clientAliveCountMax", 3, "maximum number of missed keep-alive messages before disconnecting") sftpOptionsStandalone.userStoreFile = cmdSftp.Flag.String("userStoreFile", "", "path to JSON file containing user credentials and permissions") + sftpOptionsStandalone.trustedUserCAKeysFile = cmdSftp.Flag.String("trustedUserCAKeysFile", "", "path to a file with trusted user CA public keys (OpenSSH authorized_keys format); required when 'certificate' is in -authMethods. Analogous to OpenSSH TrustedUserCAKeys and MinIO --sftp=trusted-user-ca-key") sftpOptionsStandalone.dataCenter = cmdSftp.Flag.String("dataCenter", "", "prefer to read and write to volumes in this data center") sftpOptionsStandalone.metricsHttpPort = cmdSftp.Flag.Int("metricsPort", 0, "Prometheus metrics listen port") sftpOptionsStandalone.metricsHttpIp = cmdSftp.Flag.String("metricsIp", "", "metrics listen ip. If empty, default to same as -ip.bind option.") @@ -101,6 +103,7 @@ func (sftpOpt *SftpOptions) resolvePaths() { *sftpOpt.sshPrivateKey = util.ResolvePath(*sftpOpt.sshPrivateKey) *sftpOpt.hostKeysFolder = util.ResolvePath(*sftpOpt.hostKeysFolder) *sftpOpt.userStoreFile = util.ResolvePath(*sftpOpt.userStoreFile) + *sftpOpt.trustedUserCAKeysFile = util.ResolvePath(*sftpOpt.trustedUserCAKeysFile) } func (sftpOpt *SftpOptions) startSftpServer() bool { @@ -165,6 +168,7 @@ func (sftpOpt *SftpOptions) startSftpServer() bool { ClientAliveInterval: *sftpOpt.clientAliveInterval, ClientAliveCountMax: *sftpOpt.clientAliveCountMax, UserStoreFile: *sftpOpt.userStoreFile, + TrustedUserCAKeysFile: *sftpOpt.trustedUserCAKeysFile, FilerSigningKey: []byte(filerSigningKey), FilerSigningExpiresAfter: filerSigningExpiresAfter, }) diff --git a/weed/sftpd/auth/auth.go b/weed/sftpd/auth/auth.go index 5ad33999a..50ab41fea 100644 --- a/weed/sftpd/auth/auth.go +++ b/weed/sftpd/auth/auth.go @@ -2,6 +2,8 @@ package auth import ( + "fmt" + "github.com/seaweedfs/seaweedfs/weed/sftpd/user" "golang.org/x/crypto/ssh" ) @@ -17,11 +19,16 @@ type Manager struct { userStore user.Store passwordAuth *PasswordAuthenticator publicKeyAuth *PublicKeyAuthenticator + certificateAuth *CertificateAuthenticator enabledAuthMethods []string } -// NewManager creates a new authentication manager -func NewManager(userStore user.Store, enabledAuthMethods []string) *Manager { +// NewManager creates a new authentication manager. +// +// trustedUserCAKeysFile is the path to a file containing trusted CA public +// keys (OpenSSH authorized_keys format). It is required when "certificate" +// is listed in enabledAuthMethods and ignored otherwise. +func NewManager(userStore user.Store, enabledAuthMethods []string, trustedUserCAKeysFile string) (*Manager, error) { manager := &Manager{ userStore: userStore, enabledAuthMethods: enabledAuthMethods, @@ -30,6 +37,7 @@ func NewManager(userStore user.Store, enabledAuthMethods []string) *Manager { // Initialize authenticators based on enabled methods passwordEnabled := false publicKeyEnabled := false + certificateEnabled := false for _, method := range enabledAuthMethods { switch method { @@ -37,13 +45,21 @@ func NewManager(userStore user.Store, enabledAuthMethods []string) *Manager { passwordEnabled = true case "publickey": publicKeyEnabled = true + case "certificate": + certificateEnabled = true } } manager.passwordAuth = NewPasswordAuthenticator(userStore, passwordEnabled) manager.publicKeyAuth = NewPublicKeyAuthenticator(userStore, publicKeyEnabled) - return manager + certAuth, err := NewCertificateAuthenticator(userStore, certificateEnabled, trustedUserCAKeysFile) + if err != nil { + return nil, fmt.Errorf("init certificate auth: %w", err) + } + manager.certificateAuth = certAuth + + return manager, nil } // GetSSHServerConfig returns an SSH server config with the appropriate authentication methods @@ -55,8 +71,13 @@ func (m *Manager) GetSSHServerConfig() *ssh.ServerConfig { config.PasswordCallback = m.passwordAuth.Authenticate } - // Add public key authentication if enabled - if m.publicKeyAuth.Enabled() { + // Wire the public-key channel. Certificate auth, when enabled, takes + // over the channel entirely (MinIO/OpenSSH-style): plain public keys + // are rejected even if "publickey" is also listed in enabledAuthMethods. + switch { + case m.certificateAuth.Enabled(): + config.PublicKeyCallback = m.certificateAuth.Authenticate + case m.publicKeyAuth.Enabled(): config.PublicKeyCallback = m.publicKeyAuth.Authenticate } diff --git a/weed/sftpd/auth/auth_test.go b/weed/sftpd/auth/auth_test.go new file mode 100644 index 000000000..11e8abe63 --- /dev/null +++ b/weed/sftpd/auth/auth_test.go @@ -0,0 +1,79 @@ +package auth + +import ( + "strings" + "testing" + "time" + + "golang.org/x/crypto/ssh" +) + +// When both "publickey" and "certificate" are enabled, certificate auth takes +// over the public-key channel: plain keys are rejected, certs are accepted, and +// password auth stays independently wired. +func TestManager_CertificateTakesOverPublicKeyChannel(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice") + + mgr, err := NewManager(store, []string{"password", "publickey", "certificate"}, env.caKeyFile) + if err != nil { + t.Fatalf("NewManager: %v", err) + } + + cfg := mgr.GetSSHServerConfig() + if cfg.PasswordCallback == nil { + t.Fatal("expected password callback to stay wired") + } + if cfg.PublicKeyCallback == nil { + t.Fatal("expected public-key callback") + } + + // Plain public key is rejected by the certificate authenticator. + if _, err := cfg.PublicKeyCallback(&fakeConnMetadata{user: "alice"}, env.userSigner.PublicKey()); err == nil { + t.Fatal("expected plain public key to be rejected") + } else if !strings.Contains(err.Error(), "public key without certificate") { + t.Fatalf("expected certificate-channel rejection, got %v", err) + } + + // A valid cert is accepted. + cert := env.signCert(t, ssh.UserCert, []string{"alice"}, + time.Now().Add(-time.Minute), time.Now().Add(time.Hour)) + perms, err := cfg.PublicKeyCallback(&fakeConnMetadata{user: "alice"}, cert) + if err != nil { + t.Fatalf("expected cert to be accepted: %v", err) + } + if perms.Extensions["username"] != "alice" { + t.Fatalf("expected username extension alice, got %q", perms.Extensions["username"]) + } +} + +// Without "certificate", the public-key channel keeps plain public-key auth. +func TestManager_PublicKeyOnlyChannel(t *testing.T) { + mgr, err := NewManager(newStubStore("alice"), []string{"publickey"}, "") + if err != nil { + t.Fatalf("NewManager: %v", err) + } + + cfg := mgr.GetSSHServerConfig() + if cfg.PublicKeyCallback == nil { + t.Fatal("expected public-key callback") + } + + env := newTestEnv(t) + // Routes to plain public-key auth (stub store has no registered key), which + // fails with a different error than the certificate channel would. + _, err = cfg.PublicKeyCallback(&fakeConnMetadata{user: "alice"}, env.userSigner.PublicKey()) + if err == nil { + t.Fatal("expected authentication failure") + } + if strings.Contains(err.Error(), "without certificate") { + t.Fatalf("certificate auth should not be wired, got %v", err) + } +} + +// Enabling certificate auth without a CA keys file is a hard configuration error. +func TestManager_CertificateRequiresCAFile(t *testing.T) { + if _, err := NewManager(newStubStore(), []string{"certificate"}, ""); err == nil { + t.Fatal("expected error when certificate auth enabled without CA keys file") + } +} diff --git a/weed/sftpd/auth/certificate.go b/weed/sftpd/auth/certificate.go new file mode 100644 index 000000000..678e6ee82 --- /dev/null +++ b/weed/sftpd/auth/certificate.go @@ -0,0 +1,150 @@ +package auth + +import ( + "bytes" + "crypto/subtle" + "errors" + "fmt" + "os" + + "github.com/seaweedfs/seaweedfs/weed/sftpd/user" + "golang.org/x/crypto/ssh" +) + +// CertificateAuthenticator authenticates clients that present an OpenSSH +// user certificate signed by one of the configured trusted CA public keys. +// +// Behaviour mirrors MinIO's --sftp=trusted-user-ca-key option and OpenSSH's +// TrustedUserCAKeys directive: when enabled, only key blobs of type +// *ssh.Certificate are accepted on the public-key channel. Plain public +// keys are rejected. The SSH login username must appear in the cert's +// ValidPrincipals list and must resolve to an existing user in the store. +type CertificateAuthenticator struct { + userStore user.Store + enabled bool + trustedCAs []ssh.PublicKey + checker *ssh.CertChecker +} + +// NewCertificateAuthenticator constructs a CertificateAuthenticator. +// When enabled is true, caKeysFile must point to a file containing one or +// more CA public keys in OpenSSH authorized_keys format (one per line). +func NewCertificateAuthenticator(userStore user.Store, enabled bool, caKeysFile string) (*CertificateAuthenticator, error) { + a := &CertificateAuthenticator{ + userStore: userStore, + enabled: enabled, + } + + if !enabled { + return a, nil + } + + if caKeysFile == "" { + return nil, fmt.Errorf("certificate auth enabled but no trustedUserCAKeysFile provided") + } + + cas, err := loadAuthorizedKeysFile(caKeysFile) + if err != nil { + return nil, fmt.Errorf("load trusted user CA keys from %s: %w", caKeysFile, err) + } + if len(cas) == 0 { + return nil, fmt.Errorf("no trusted user CA keys found in %s", caKeysFile) + } + a.trustedCAs = cas + + // Pre-marshal trusted CA keys once. IsUserAuthority runs on every + // authentication attempt, so caching the marshaled form avoids + // repeated allocations on the hot path. + trustedCAsMarshaled := make([][]byte, len(cas)) + for i, ca := range cas { + trustedCAsMarshaled[i] = ca.Marshal() + } + + a.checker = &ssh.CertChecker{ + IsUserAuthority: func(auth ssh.PublicKey) bool { + marshaled := auth.Marshal() + for _, caBytes := range trustedCAsMarshaled { + if subtle.ConstantTimeCompare(marshaled, caBytes) == 1 { + return true + } + } + return false + }, + } + + return a, nil +} + +// Enabled reports whether certificate authentication is active. +func (a *CertificateAuthenticator) Enabled() bool { + return a.enabled +} + +// Authenticate implements ssh.ServerConfig.PublicKeyCallback. +func (a *CertificateAuthenticator) Authenticate(conn ssh.ConnMetadata, key ssh.PublicKey) (*ssh.Permissions, error) { + if !a.enabled { + return nil, fmt.Errorf("certificate authentication disabled") + } + + cert, ok := key.(*ssh.Certificate) + if !ok { + return nil, fmt.Errorf("public key without certificate not allowed") + } + if cert.CertType != ssh.UserCert { + return nil, fmt.Errorf("certificate is not a user certificate") + } + if len(cert.ValidPrincipals) == 0 { + return nil, fmt.Errorf("certificate has no valid principals") + } + + username := conn.User() + + // CertChecker.Authenticate verifies the CA signature (via IsUserAuthority), + // the ValidAfter/ValidBefore window, and that username is in ValidPrincipals. + perms, err := a.checker.Authenticate(conn, key) + if err != nil { + return nil, fmt.Errorf("certificate validation failed: %w", err) + } + + // The SSH login user must exist in the SeaweedFS user store. + if _, err := a.userStore.GetUser(username); err != nil { + var notFound *user.UserNotFoundError + if errors.As(err, ¬Found) { + return nil, fmt.Errorf("user %q not found", username) + } + return nil, fmt.Errorf("lookup user %q: %w", username, err) + } + + if perms == nil { + perms = &ssh.Permissions{} + } + if perms.Extensions == nil { + perms.Extensions = map[string]string{} + } + perms.Extensions["username"] = username + return perms, nil +} + +// loadAuthorizedKeysFile parses an authorized_keys-style file and returns +// all public keys found in it. Blank lines and comment lines starting with +// '#' are skipped. +func loadAuthorizedKeysFile(path string) ([]ssh.PublicKey, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + + var keys []ssh.PublicKey + for _, line := range bytes.Split(data, []byte("\n")) { + line = bytes.TrimSpace(line) + if len(line) == 0 || line[0] == '#' { + continue + } + pub, _, _, _, err := ssh.ParseAuthorizedKey(line) + if err != nil { + return nil, err + } + keys = append(keys, pub) + } + return keys, nil +} diff --git a/weed/sftpd/auth/certificate_test.go b/weed/sftpd/auth/certificate_test.go new file mode 100644 index 000000000..7a0827176 --- /dev/null +++ b/weed/sftpd/auth/certificate_test.go @@ -0,0 +1,312 @@ +package auth + +import ( + "crypto/ed25519" + "crypto/rand" + "errors" + "net" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/seaweedfs/seaweedfs/weed/sftpd/user" + "golang.org/x/crypto/ssh" +) + +// stubStore is a minimal user.Store for auth tests. +type stubStore struct { + users map[string]*user.User +} + +func newStubStore(usernames ...string) *stubStore { + s := &stubStore{users: map[string]*user.User{}} + for _, name := range usernames { + s.users[name] = &user.User{Username: name} + } + return s +} + +func (s *stubStore) GetUser(username string) (*user.User, error) { + if u, ok := s.users[username]; ok { + return u, nil + } + return nil, &user.UserNotFoundError{Username: username} +} +func (s *stubStore) ValidatePassword(string, []byte) bool { return false } +func (s *stubStore) ValidatePublicKey(string, string) bool { return false } +func (s *stubStore) GetUserPermissions(string, string) []string { return nil } +func (s *stubStore) SaveUser(*user.User) error { return nil } +func (s *stubStore) DeleteUser(string) error { return nil } +func (s *stubStore) ListUsers() ([]string, error) { return nil, nil } + +// fakeConnMetadata satisfies ssh.ConnMetadata for the parts CertChecker uses. +type fakeConnMetadata struct { + user string +} + +func (c *fakeConnMetadata) User() string { return c.user } +func (c *fakeConnMetadata) SessionID() []byte { return []byte("session") } +func (c *fakeConnMetadata) ClientVersion() []byte { return []byte("SSH-2.0-test") } +func (c *fakeConnMetadata) ServerVersion() []byte { return []byte("SSH-2.0-test") } +func (c *fakeConnMetadata) RemoteAddr() net.Addr { return &fakeAddr{} } +func (c *fakeConnMetadata) LocalAddr() net.Addr { return &fakeAddr{} } + +type fakeAddr struct{} + +func (fakeAddr) Network() string { return "tcp" } +func (fakeAddr) String() string { return "127.0.0.1:0" } + +// testEnv bundles a CA, a user signer, and a temp dir for a single test. +type testEnv struct { + caSigner ssh.Signer + userSigner ssh.Signer + caKeyFile string +} + +func newTestEnv(t *testing.T) *testEnv { + t.Helper() + + _, caPriv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519 CA: %v", err) + } + caSigner, err := ssh.NewSignerFromKey(caPriv) + if err != nil { + t.Fatalf("ca signer: %v", err) + } + + _, userPriv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519 user: %v", err) + } + userSigner, err := ssh.NewSignerFromKey(userPriv) + if err != nil { + t.Fatalf("user signer: %v", err) + } + + dir := t.TempDir() + caFile := filepath.Join(dir, "ca_user.pub") + if err := os.WriteFile(caFile, ssh.MarshalAuthorizedKey(caSigner.PublicKey()), 0o600); err != nil { + t.Fatalf("write ca file: %v", err) + } + + return &testEnv{caSigner: caSigner, userSigner: userSigner, caKeyFile: caFile} +} + +// signCert produces a user cert signed by the test CA. +func (e *testEnv) signCert(t *testing.T, certType uint32, principals []string, validAfter, validBefore time.Time) *ssh.Certificate { + t.Helper() + cert := &ssh.Certificate{ + Key: e.userSigner.PublicKey(), + CertType: certType, + ValidPrincipals: principals, + ValidAfter: uint64(validAfter.Unix()), + ValidBefore: uint64(validBefore.Unix()), + } + if err := cert.SignCert(rand.Reader, e.caSigner); err != nil { + t.Fatalf("sign cert: %v", err) + } + return cert +} + +func TestCertificateAuthenticator_GoldenPath(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice") + + a, err := NewCertificateAuthenticator(store, true, env.caKeyFile) + if err != nil { + t.Fatalf("new: %v", err) + } + + cert := env.signCert(t, ssh.UserCert, []string{"alice"}, + time.Now().Add(-time.Minute), time.Now().Add(time.Hour)) + + perms, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert) + if err != nil { + t.Fatalf("Authenticate: %v", err) + } + if perms.Extensions["username"] != "alice" { + t.Fatalf("expected username extension alice, got %q", perms.Extensions["username"]) + } +} + +func TestCertificateAuthenticator_RejectsPlainPublicKey(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice") + + a, err := NewCertificateAuthenticator(store, true, env.caKeyFile) + if err != nil { + t.Fatalf("new: %v", err) + } + + _, err = a.Authenticate(&fakeConnMetadata{user: "alice"}, env.userSigner.PublicKey()) + if err == nil { + t.Fatal("expected rejection of plain public key, got nil") + } + if !strings.Contains(err.Error(), "public key without certificate") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestCertificateAuthenticator_RejectsHostCert(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice") + a, _ := NewCertificateAuthenticator(store, true, env.caKeyFile) + + cert := env.signCert(t, ssh.HostCert, []string{"alice"}, + time.Now().Add(-time.Minute), time.Now().Add(time.Hour)) + + if _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert); err == nil { + t.Fatal("expected rejection of host cert, got nil") + } +} + +func TestCertificateAuthenticator_RejectsEmptyPrincipals(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice") + a, _ := NewCertificateAuthenticator(store, true, env.caKeyFile) + + cert := env.signCert(t, ssh.UserCert, nil, + time.Now().Add(-time.Minute), time.Now().Add(time.Hour)) + + _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert) + if err == nil || !strings.Contains(err.Error(), "no valid principals") { + t.Fatalf("expected empty-principals rejection, got %v", err) + } +} + +func TestCertificateAuthenticator_RejectsWrongPrincipal(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice", "bob") + a, _ := NewCertificateAuthenticator(store, true, env.caKeyFile) + + cert := env.signCert(t, ssh.UserCert, []string{"bob"}, + time.Now().Add(-time.Minute), time.Now().Add(time.Hour)) + + if _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert); err == nil { + t.Fatal("expected wrong-principal rejection, got nil") + } +} + +func TestCertificateAuthenticator_RejectsExpiredCert(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice") + a, _ := NewCertificateAuthenticator(store, true, env.caKeyFile) + + cert := env.signCert(t, ssh.UserCert, []string{"alice"}, + time.Now().Add(-2*time.Hour), time.Now().Add(-time.Hour)) + + if _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert); err == nil { + t.Fatal("expected expired-cert rejection, got nil") + } +} + +func TestCertificateAuthenticator_RejectsUnknownCA(t *testing.T) { + env := newTestEnv(t) + store := newStubStore("alice") + a, _ := NewCertificateAuthenticator(store, true, env.caKeyFile) + + // Sign with a different CA. + _, otherCAPriv, _ := ed25519.GenerateKey(rand.Reader) + otherCASigner, _ := ssh.NewSignerFromKey(otherCAPriv) + + cert := &ssh.Certificate{ + Key: env.userSigner.PublicKey(), + CertType: ssh.UserCert, + ValidPrincipals: []string{"alice"}, + ValidAfter: uint64(time.Now().Add(-time.Minute).Unix()), + ValidBefore: uint64(time.Now().Add(time.Hour).Unix()), + } + if err := cert.SignCert(rand.Reader, otherCASigner); err != nil { + t.Fatalf("sign: %v", err) + } + + if _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert); err == nil { + t.Fatal("expected unknown-CA rejection, got nil") + } +} + +func TestCertificateAuthenticator_RejectsUnknownUser(t *testing.T) { + env := newTestEnv(t) + store := newStubStore() // no users + a, _ := NewCertificateAuthenticator(store, true, env.caKeyFile) + + cert := env.signCert(t, ssh.UserCert, []string{"alice"}, + time.Now().Add(-time.Minute), time.Now().Add(time.Hour)) + + _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("expected user-not-found, got %v", err) + } +} + +func TestCertificateAuthenticator_LoadsMultipleCAKeys(t *testing.T) { + env := newTestEnv(t) + + // Append a second CA pubkey to the file. + _, ca2Priv, _ := ed25519.GenerateKey(rand.Reader) + ca2Signer, _ := ssh.NewSignerFromKey(ca2Priv) + f, err := os.OpenFile(env.caKeyFile, os.O_APPEND|os.O_WRONLY, 0) + if err != nil { + t.Fatalf("open: %v", err) + } + if _, err := f.Write(ssh.MarshalAuthorizedKey(ca2Signer.PublicKey())); err != nil { + t.Fatalf("write: %v", err) + } + f.Close() + + store := newStubStore("alice") + a, err := NewCertificateAuthenticator(store, true, env.caKeyFile) + if err != nil { + t.Fatalf("new: %v", err) + } + if len(a.trustedCAs) != 2 { + t.Fatalf("expected 2 CAs, got %d", len(a.trustedCAs)) + } + + // A cert signed by the second CA should be accepted. + cert := &ssh.Certificate{ + Key: env.userSigner.PublicKey(), + CertType: ssh.UserCert, + ValidPrincipals: []string{"alice"}, + ValidAfter: uint64(time.Now().Add(-time.Minute).Unix()), + ValidBefore: uint64(time.Now().Add(time.Hour).Unix()), + } + if err := cert.SignCert(rand.Reader, ca2Signer); err != nil { + t.Fatalf("sign: %v", err) + } + if _, err := a.Authenticate(&fakeConnMetadata{user: "alice"}, cert); err != nil { + t.Fatalf("Authenticate with second CA: %v", err) + } +} + +func TestNewCertificateAuthenticator_DisabledIgnoresFile(t *testing.T) { + a, err := NewCertificateAuthenticator(newStubStore(), false, "") + if err != nil { + t.Fatalf("new: %v", err) + } + if a.Enabled() { + t.Fatal("expected disabled") + } +} + +func TestNewCertificateAuthenticator_EnabledRequiresFile(t *testing.T) { + _, err := NewCertificateAuthenticator(newStubStore(), true, "") + if err == nil { + t.Fatal("expected error when enabled without file") + } +} + +func TestNewCertificateAuthenticator_MissingFile(t *testing.T) { + _, err := NewCertificateAuthenticator(newStubStore(), true, filepath.Join(t.TempDir(), "nope.pub")) + if err == nil { + t.Fatal("expected error for missing file") + } + // Sanity: not a not-found-user-style error. + var notFound *user.UserNotFoundError + if errors.As(err, ¬Found) { + t.Fatalf("unexpected error type: %v", err) + } +} diff --git a/weed/sftpd/sftp_service.go b/weed/sftpd/sftp_service.go index 643af99a6..19b3d39fd 100644 --- a/weed/sftpd/sftp_service.go +++ b/weed/sftpd/sftp_service.go @@ -36,7 +36,7 @@ type SFTPServiceOptions struct { // SSH Configuration SshPrivateKey string // Legacy single host key HostKeysFolder string // Multiple host keys for different algorithms - AuthMethods []string // Enabled auth methods: "password", "publickey", "keyboard-interactive" + AuthMethods []string // Enabled auth methods: "password", "publickey", "certificate" MaxAuthTries int // Limit authentication attempts BannerMessage string // Pre-auth banner message LoginGraceTime time.Duration // Timeout for authentication @@ -48,6 +48,9 @@ type SFTPServiceOptions struct { // User Management UserStoreFile string // Path to user store file + // Certificate Authentication + TrustedUserCAKeysFile string // Path to file with trusted user CA public keys (OpenSSH authorized_keys format) + // JWT Configuration for Filer FilerSigningKey []byte // JWT signing key for filer uploads FilerSigningExpiresAfter int // JWT token expiration time in seconds @@ -65,7 +68,11 @@ func NewSFTPService(options *SFTPServiceOptions) *SFTPService { service.userStore = userStore // Initialize auth manager - service.authManager = auth.NewManager(userStore, options.AuthMethods) + authManager, err := auth.NewManager(userStore, options.AuthMethods, options.TrustedUserCAKeysFile) + if err != nil { + glog.Fatalf("Failed to initialize auth manager: %v", err) + } + service.authManager = authManager return &service }