require at least 2048 bits for RSA certificates/keys

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

internal/dataprovider/dataprovider.go

@@ -21,6 +21,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/md5"
+	"crypto/rsa"
 	"crypto/sha1"
 	"crypto/sha256"
 	"crypto/sha512"
@@ -2877,15 +2878,32 @@ func validatePublicKeys(user *User) error {
 		user.PublicKeys = []string{}
 	}
 	var validatedKeys []string
-	for i, k := range user.PublicKeys {
+	for idx, key := range user.PublicKeys {
-		if k == "" {
+		if key == "" {
 			continue
 		}
-		_, _, _, _, err := ssh.ParseAuthorizedKey([]byte(k))
+		out, _, _, _, err := ssh.ParseAuthorizedKey([]byte(key))
 		if err != nil {
-			return util.NewValidationError(fmt.Sprintf("could not parse key nr. %d: %s", i+1, err))
+			return util.NewI18nError(
+				util.NewValidationError(fmt.Sprintf("error parsing public key at position %d: %v", idx, err)),
+				util.I18nErrorPubKeyInvalid,
+			)
 		}
-		validatedKeys = append(validatedKeys, k)
+		if k, ok := out.(ssh.CryptoPublicKey); ok {
+			cryptoKey := k.CryptoPublicKey()
+			if rsaKey, ok := cryptoKey.(*rsa.PublicKey); ok {
+				if size := rsaKey.N.BitLen(); size < 2048 {
+					providerLog(logger.LevelError, "rsa key with size %d not accepted, minimum 2048", size)
+					return util.NewI18nError(
+						util.NewValidationError(fmt.Sprintf("invalid size %d for rsa key at position %d, minimum 2048",
+							size, idx)),
+						util.I18nErrorKeySizeInvalid,
+					)
+				}
+			}
+		}
+
+		validatedKeys = append(validatedKeys, key)
 	}
 	user.PublicKeys = util.RemoveDuplicates(validatedKeys, false)
 	return nil
@@ -3047,12 +3065,25 @@ func validateTLSCerts(certs []string) error {
 				util.I18nErrorInvalidTLSCert,
 			)
 		}
-		if _, err := x509.ParseCertificate(derBlock.Bytes); err != nil {
+		cert, err := x509.ParseCertificate(derBlock.Bytes)
+		if err != nil {
 			return util.NewI18nError(
 				util.NewValidationError(fmt.Sprintf("error parsing TLS certificate %d", idx)),
 				util.I18nErrorInvalidTLSCert,
 			)
 		}
+		if cert.PublicKeyAlgorithm == x509.RSA {
+			if rsaCert, ok := cert.PublicKey.(*rsa.PublicKey); ok {
+				if size := rsaCert.N.BitLen(); size < 2048 {
+					providerLog(logger.LevelError, "rsa cert with size %d not accepted, minimum 2048", size)
+					return util.NewI18nError(
+						util.NewValidationError(fmt.Sprintf("invalid size %d for rsa cert at position %d, minimum 2048",
+							size, idx)),
+						util.I18nErrorKeySizeInvalid,
+					)
+				}
+			}
+		}
 	}
 	return nil
 }
@@ -3275,7 +3306,7 @@ func ValidateUser(user *User) error {
 		return err
 	}
 	if err := validatePublicKeys(user); err != nil {
-		return util.NewI18nError(err, util.I18nErrorPubKeyInvalid)
+		return err
 	}
 	if err := validateBaseFilters(&user.Filters.BaseUserFilters); err != nil {
 		return err
@@ -3477,14 +3508,14 @@ func checkUserAndPubKey(user *User, pubKey []byte, isSSHCert bool) (User, string
 	if len(user.PublicKeys) == 0 {
 		return *user, "", ErrInvalidCredentials
 	}
-	for i, k := range user.PublicKeys {
+	for idx, key := range user.PublicKeys {
-		storedPubKey, comment, _, _, err := ssh.ParseAuthorizedKey([]byte(k))
+		storedKey, comment, _, _, err := ssh.ParseAuthorizedKey([]byte(key))
 		if err != nil {
-			providerLog(logger.LevelError, "error parsing stored public key %d for user %s: %v", i, user.Username, err)
+			providerLog(logger.LevelError, "error parsing stored public key %d for user %s: %v", idx, user.Username, err)
 			return *user, "", err
 		}
-		if bytes.Equal(storedPubKey.Marshal(), pubKey) {
+		if bytes.Equal(storedKey.Marshal(), pubKey) {
-			return *user, fmt.Sprintf("%s:%s", ssh.FingerprintSHA256(storedPubKey), comment), nil
+			return *user, fmt.Sprintf("%s:%s", ssh.FingerprintSHA256(storedKey), comment), nil
 		}
 	}
 	return *user, "", ErrInvalidCredentials

internal/httpd/httpd_test.go

@@ -241,6 +241,23 @@ qwlk5iw/jQekxThg==
 `
 	testPubKeyPwd  = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAILqltfCL7IPuIQ2q+8w23flfgskjIlKViEwMfjJR4mrb"
 	privateKeyPwd  = "password"
+	rsa1024PrivKey = `-----BEGIN OPENSSH PRIVATE KEY-----
+b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAlwAAAAdzc2gtcn
+NhAAAAAwEAAQAAAIEAxgrZ84gJyU7Qz8JbYuYh0fgTN29h4qVkqDkEE0lWZe7L4QRcQHrB
+vycJO5vjfitY5JTojV3nbDNHN6XGVX8QNurwXmxv0EmEbqPoNO/rTf1t7qqwMBBAfSJJ5H
+TXsO37vqcWSOt1Ki5yjRm232UfPo3AYXaZdOKDWKpzI12FfqkAAAIAondFqKJ3RagAAAAH
+c3NoLXJzYQAAAIEAxgrZ84gJyU7Qz8JbYuYh0fgTN29h4qVkqDkEE0lWZe7L4QRcQHrBvy
+cJO5vjfitY5JTojV3nbDNHN6XGVX8QNurwXmxv0EmEbqPoNO/rTf1t7qqwMBBAfSJJ5HTX
+sO37vqcWSOt1Ki5yjRm232UfPo3AYXaZdOKDWKpzI12FfqkAAAADAQABAAAAgC7V5COG+a
+GFJTbtJQWnnTn17D2A9upN6RcrnL4e6vLiXY8So+qP3YAicDmLrWpqP/SXDsRX/+ID4oTT
+jKstiJy5jTvXAozwBbFCvNDk1qifs8p/HKzel3t0172j6gLOa2h9+clJ4BYyCk6ue4f8fV
+yKTIc9chdJSpeINNY60CJxAAAAQQDhYpGXljD2Xy/CzqRXyoF+iMtOImLlbgQYswTXegk3
+7JoCNvwqg8xP+JxGpvUGpX23VWh0nBhzcAKHGlssiYQuAAAAQQDwB6s7s1WIRZ2Jsz8f6l
+7/ebpPrAMyKmWkXc7KyvR53zuMkMIdvujM5NkOWh1ON8jtNumArey2dWuGVh+pXbdVAAAA
+QQDTOAaMcyTfXMH/oSMsp+5obvT/RuewaRLHdBiCy0y1Jw0ykOcOCkswr/btDL26hImaHF
+SheorO+2We7dnFuUIFAAAACW5pY29sYUBwMQE=
+-----END OPENSSH PRIVATE KEY-----`
+	rsa1024PubKey  = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDGCtnziAnJTtDPwlti5iHR+BM3b2HipWSoOQQTSVZl7svhBFxAesG/Jwk7m+N+K1jklOiNXedsM0c3pcZVfxA26vBebG/QSYRuo+g07+tN/W3uqrAwEEB9IknkdNew7fu+pxZI63UqLnKNGbbfZR8+jcBhdpl04oNYqnMjXYV+qQ=="
 	redactedSecret = "[**redacted**]"
 	osWindows      = "windows"
 	oidcMockAddr   = "127.0.0.1:11111"
@@ -823,6 +840,20 @@ func TestRoleRelations(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestRSAKeyInvalidSize(t *testing.T) {
+	u := getTestUser()
+	u.PublicKeys = append(u.PublicKeys, rsa1024PubKey)
+	_, resp, err := httpdtest.AddUser(u, http.StatusBadRequest)
+	assert.NoError(t, err, string(resp))
+	assert.Contains(t, string(resp), "invalid size")
+	u = getTestSFTPUser()
+	u.FsConfig.SFTPConfig.Password = nil
+	u.FsConfig.SFTPConfig.PrivateKey = kms.NewPlainSecret(rsa1024PrivKey)
+	_, resp, err = httpdtest.AddUser(u, http.StatusBadRequest)
+	assert.NoError(t, err, string(resp))
+	assert.Contains(t, string(resp), "rsa key with size 1024 not accepted")
+}
+
 func TestTLSCert(t *testing.T) {
 	u := getTestUser()
 	u.Filters.TLSCerts = []string{"not a cert"}
@@ -11065,7 +11096,7 @@ func TestWebAPIChangeUserProfileMock(t *testing.T) {
 	setBearerForReq(req, token)
 	rr = executeRequest(req)
 	checkResponseCode(t, http.StatusBadRequest, rr)
-	assert.Contains(t, rr.Body.String(), "Validation error: could not parse key")
+	assert.Contains(t, rr.Body.String(), "Validation error: error parsing public key")
 
 	user, _, err = httpdtest.GetUserByUsername(user.Username, http.StatusOK)
 	assert.NoError(t, err)
@@ -22409,8 +22440,8 @@ func TestWebUserSFTPFsMock(t *testing.T) {
 	user.FsConfig.SFTPConfig.Endpoint = "127.0.0.1:22"
 	user.FsConfig.SFTPConfig.Username = "sftpuser"
 	user.FsConfig.SFTPConfig.Password = kms.NewPlainSecret("pwd")
-	user.FsConfig.SFTPConfig.PrivateKey = kms.NewPlainSecret(sftpPrivateKey)
+	user.FsConfig.SFTPConfig.PrivateKey = kms.NewPlainSecret(testPrivateKeyPwd)
-	user.FsConfig.SFTPConfig.KeyPassphrase = kms.NewPlainSecret(testPrivateKeyPwd)
+	user.FsConfig.SFTPConfig.KeyPassphrase = kms.NewPlainSecret(privateKeyPwd)
 	user.FsConfig.SFTPConfig.Fingerprints = []string{sftpPkeyFingerprint}
 	user.FsConfig.SFTPConfig.Prefix = "/home/sftpuser"
 	user.FsConfig.SFTPConfig.DisableCouncurrentReads = true

internal/util/i18n.go

@@ -119,6 +119,8 @@ const (
 	I18nErrorHomeRequired              = "user.home_required"
 	I18nErrorHomeInvalid               = "user.home_invalid"
 	I18nErrorPubKeyInvalid             = "user.pub_key_invalid"
+	I18nErrorPrivKeyInvalid            = "user.priv_key_invalid"
+	I18nErrorKeySizeInvalid            = "user.key_invalid_size"
 	I18nErrorPrimaryGroup              = "user.err_primary_group"
 	I18nErrorDuplicateGroup            = "user.err_duplicate_group"
 	I18nErrorNoPermission              = "user.no_permissions"

internal/vfs/sftpfs.go

@@ -17,6 +17,7 @@ package vfs
 import (
 	"bufio"
 	"bytes"
+	"crypto/rsa"
 	"errors"
 	"fmt"
 	"hash/fnv"
@@ -179,6 +180,34 @@ func (c *SFTPFsConfig) validate() error {
 	} else {
 		c.Prefix = "/"
 	}
+	return c.validatePrivateKey()
+}
+
+func (c *SFTPFsConfig) validatePrivateKey() error {
+	if c.PrivateKey.IsPlain() {
+		var signer ssh.Signer
+		var err error
+		if c.KeyPassphrase.IsPlain() {
+			signer, err = ssh.ParsePrivateKeyWithPassphrase([]byte(c.PrivateKey.GetPayload()),
+				[]byte(c.KeyPassphrase.GetPayload()))
+		} else {
+			signer, err = ssh.ParsePrivateKey([]byte(c.PrivateKey.GetPayload()))
+		}
+		if err != nil {
+			return util.NewI18nError(fmt.Errorf("invalid private key: %w", err), util.I18nErrorPrivKeyInvalid)
+		}
+		if key, ok := signer.PublicKey().(ssh.CryptoPublicKey); ok {
+			cryptoKey := key.CryptoPublicKey()
+			if rsaKey, ok := cryptoKey.(*rsa.PublicKey); ok {
+				if size := rsaKey.N.BitLen(); size < 2048 {
+					return util.NewI18nError(
+						fmt.Errorf("rsa key with size %d not accepted, minimum 2048", size),
+						util.I18nErrorKeySizeInvalid,
+					)
+				}
+			}
+		}
+	}
 	return nil
 }
 
@@ -902,6 +931,14 @@ func (c *sftpConnection) OpenConnection() error {
 	return c.openConnNoLock()
 }
 
+func (c *sftpConnection) getSigner() (ssh.Signer, error) {
+	if c.config.KeyPassphrase.GetPayload() != "" {
+		return ssh.ParsePrivateKeyWithPassphrase([]byte(c.config.PrivateKey.GetPayload()),
+			[]byte(c.config.KeyPassphrase.GetPayload()))
+	}
+	return ssh.ParsePrivateKey([]byte(c.config.PrivateKey.GetPayload()))
+}
+
 func (c *sftpConnection) openConnNoLock() error {
 	if c.isConnected {
 		logger.Debug(c.logSender, "", "reusing connection")
@@ -940,14 +977,7 @@ func (c *sftpConnection) openConnNoLock() error {
 		ClientVersion: fmt.Sprintf("SSH-2.0-SFTPGo_%v", version.Get().Version),
 	}
 	if c.config.PrivateKey.GetPayload() != "" {
-		var signer ssh.Signer
+		signer, err := c.getSigner()
-		var err error
-		if c.config.KeyPassphrase.GetPayload() != "" {
-			signer, err = ssh.ParsePrivateKeyWithPassphrase([]byte(c.config.PrivateKey.GetPayload()),
-				[]byte(c.config.KeyPassphrase.GetPayload()))
-		} else {
-			signer, err = ssh.ParsePrivateKey([]byte(c.config.PrivateKey.GetPayload()))
-		}
 		if err != nil {
 			return fmt.Errorf("sftpfs: unable to parse the private key: %w", err)
 		}

static/locales/en/translation.json

@@ -489,6 +489,8 @@
         "home_required": "The home directory is mandatory",
         "home_invalid": "The home directory must be an absolute path",
         "pub_key_invalid": "Invalid public key",
+        "priv_key_invalid": "Invalid private key",
+        "key_invalid_size": "Invalid RSA key: the minimum supported size is 2048",
         "err_primary_group": "Only one primary group is allowed",
         "err_duplicate_group": "Duplicate groups detected",
         "no_permissions": "Directories permissions are mandatory",

static/locales/it/translation.json

@@ -489,6 +489,8 @@
         "home_required": "La directory principale è obbligatoria",
         "home_invalid": "La directory principale deve essere un path assoluto",
         "pub_key_invalid": "Chiave pubblica non valida",
+        "priv_key_invalid": "Chiave privata non valida",
+        "key_invalid_size": "Chiave RSA non valida: la dimensione minima supportata è 2048",
         "err_primary_group": "È consentito un solo gruppo primario",
         "err_duplicate_group": "Rilevati gruppi duplicati",
         "no_permissions": "I permessi per le directory sono obbligatori",