Switch to secure signing algorithm for SSH secrets engine (#14006)

* Explicitly call out SSH algorithm_signer default

Related: #11608

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Use rsa-sha2-256 as the default SSH CA hash algo

As mentioned in the OpenSSH 8.2 release notes, OpenSSH will no longer be
accepting ssh-rsa signatures by default as these use the insecure SHA-1
algorithm.

For roles in which an explicit signature type wasn't specified, we
should change the default from SHA-1 to SHA-256 for security and
compatibility with modern OpenSSH releases.

See also: https://www.openssh.com/txt/release-8.2

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Update docs mentioning new algorithm change

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Fix missing parenthesis, clarify new default value

* Add to side bar

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

builtin/logical/ssh/backend_test.go

@@ -720,7 +720,23 @@ func TestSSHBackend_CA(t *testing.T) {
 			testCAPublicKey,
 			testCAPrivateKey,
 			"",
-			true,
+			false,
+		},
+		{
+			"RSAKey_DefaultAlgoSigner_ImageSupportsRSA1",
+			dockerImageTagSupportsRSA1,
+			testCAPublicKey,
+			testCAPrivateKey,
+			"default",
+			false,
+		},
+		{
+			"RSAKey_DefaultAlgoSigner_ImageSupportsNoRSA1",
+			dockerImageTagSupportsNoRSA1,
+			testCAPublicKey,
+			testCAPrivateKey,
+			"default",
+			false,
 		},
 		{
 			"RSAKey_RSA1AlgoSigner_ImageSupportsRSA1",
@@ -730,6 +746,14 @@ func TestSSHBackend_CA(t *testing.T) {
 			ssh.SigAlgoRSA,
 			false,
 		},
+		{
+			"RSAKey_RSA1AlgoSigner_ImageSupportsNoRSA1",
+			dockerImageTagSupportsNoRSA1,
+			testCAPublicKey,
+			testCAPrivateKey,
+			ssh.SigAlgoRSA,
+			true,
+		},
 		{
 			"RSAKey_RSASHA2256AlgoSigner_ImageSupportsRSA1",
 			dockerImageTagSupportsRSA1,

builtin/logical/ssh/path_roles.go

@@ -22,10 +22,13 @@ const (
 	// KeyTypeCA is an key of type CA
 	KeyTypeCA = "ca"
 
+	// DefaultAlgorithmSigner is the default RSA signing algorithm
+	DefaultAlgorithmSigner = "default"
+
 	// Present version of the sshRole struct; when adding a new field or are
 	// needing to perform a migration, increment this struct and read the note
 	// in checkUpgrade(...).
-	roleEntryVersion = 1
+	roleEntryVersion = 2
 )
 
 // Structure that represents a role in SSH backend. This is a common role structure
@@ -354,7 +357,7 @@ func pathRoles(b *backend) *framework.Path {
 				Type: framework.TypeString,
 				Description: `
 				When supplied, this value specifies a signing algorithm for the key. Possible values:
-				ssh-rsa, rsa-sha2-256, rsa-sha2-512.
+				ssh-rsa, rsa-sha2-256, rsa-sha2-512, default, or the empty string.
 				`,
 				DisplayAttrs: &framework.DisplayAttributes{
 					Name: "Signing Algorithm",
@@ -496,15 +499,17 @@ func (b *backend) pathRoleWrite(ctx context.Context, req *logical.Request, d *fr
 			Version:         roleEntryVersion,
 		}
 	} else if keyType == KeyTypeCA {
-		algorithmSigner := ""
+		algorithmSigner := DefaultAlgorithmSigner
 		algorithmSignerRaw, ok := d.GetOk("algorithm_signer")
 		if ok {
 			algorithmSigner = algorithmSignerRaw.(string)
 			switch algorithmSigner {
 			case ssh.SigAlgoRSA, ssh.SigAlgoRSASHA2256, ssh.SigAlgoRSASHA2512:
-			case "":
+			case "", DefaultAlgorithmSigner:
 				// This case is valid, and the sign operation will use the signer's
-				// default algorithm.
+				// default algorithm. Explicitly reset the value to the default value
+				// rather than use the more vague implicit empty string.
+				algorithmSigner = DefaultAlgorithmSigner
 			default:
 				return nil, fmt.Errorf("unknown algorithm signer %q", algorithmSigner)
 			}
@@ -631,6 +636,42 @@ func (b *backend) checkUpgrade(ctx context.Context, s logical.Storage, n string,
 		modified = true
 	}
 
+	// Role version 2 migrates an empty AlgorithmSigner to an explicit ssh-rsa
+	// value WHEN the SSH CA key is a RSA key.
+	if result.Version < 2 {
+		// In order to perform the version 2 upgrade, we need knowledge of the
+		// signing key type as we want to make ssh-rsa an explicitly notated
+		// algorithm choice.
+		var publicKey ssh.PublicKey
+		publicKeyEntry, err := caKey(ctx, s, caPublicKey)
+		if err != nil {
+			b.Logger().Debug(fmt.Sprintf("failed to load public key entry while attempting to migrate: %v", err))
+			goto SKIPVERSION2
+		}
+		if publicKeyEntry == nil || publicKeyEntry.Key == "" {
+			b.Logger().Debug(fmt.Sprintf("got empty public key entry while attempting to migrate"))
+			goto SKIPVERSION2
+		}
+
+		publicKey, err = parsePublicSSHKey(publicKeyEntry.Key)
+		if err == nil {
+			// Move an empty signing algorithm to an explicit ssh-rsa (SHA-1)
+			// if this key is of type RSA. This isn't a secure default but
+			// exists for backwards compatibility with existing versions of
+			// Vault. By making it explicit, operators can see that this is
+			// the value and move it to a newer algorithm in the future.
+			if publicKey.Type() == ssh.KeyAlgoRSA && result.AlgorithmSigner == "" {
+				result.AlgorithmSigner = ssh.SigAlgoRSA
+			}
+
+			result.Version = 2
+			modified = true
+		}
+
+	SKIPVERSION2:
+		err = nil
+	}
+
 	// Add new migrations just before here.
 	//
 	// Condition copied from PKI builtin.

builtin/logical/ssh/path_sign.go

@@ -560,6 +560,14 @@ func (b *creationBundle) sign() (retCert *ssh.Certificate, retErr error) {
 	certificateBytes := out[:len(out)-4]
 
 	algo := b.Role.AlgorithmSigner
+
+	// Handle the new default algorithm selection process correctly.
+	if algo == DefaultAlgorithmSigner && sshAlgorithmSigner.PublicKey().Type() == ssh.KeyAlgoRSA {
+		algo = ssh.SigAlgoRSASHA2256
+	} else if algo == DefaultAlgorithmSigner {
+		algo = ""
+	}
+
 	sig, err := sshAlgorithmSigner.SignWithAlgorithm(rand.Reader, certificateBytes, algo)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate signed SSH key: sign error: %w", err)

changelog/14006.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/ssh: Use secure default for algorithm signer (rsa-sha2-256) with RSA SSH CA keys on new roles
+```

website/content/api-docs/secret/ssh.mdx

@@ -215,11 +215,21 @@ This endpoint creates or updates a named role.
   the CA type. To specify multiple sizes, either use a comma-separated list or an
   array of allowed key widths.
 
-- `algorithm_signer` `(string: "")` - Algorithm to sign keys with. Valid
-  values are `ssh-rsa`, `rsa-sha2-256`, and `rsa-sha2-512`. This value may be left
-  blank to use the signer's default algorithm, and must be left blank for CA key types
-  other than RSA. Note that `ssh-rsa` is now considered insecure and is not
-  supported by current OpenSSH versions.
+- `algorithm_signer` `(string: "default")` - Algorithm to sign keys with. Valid
+  values are `ssh-rsa`, `rsa-sha2-256`, `rsa-sha2-512`, or `default`. This
+  value may also be left blank to use the signer's default algorithm, and must
+  be left blank or have value `default` for CA key types other than RSA.
+
+  ~> **Note**: The value of `default` may change over time as vulnerabilities
+  in algorithms are discovered. The present value for RSA keys is equivalent
+  to `rsa-sha2-256`.
+
+  ~> **Warning**: The `algorithm_signer` value `ssh-rsa` uses the SHA-1 hash
+  algorithm. This algorithm is now considered insecure and is not supported by
+  current OpenSSH versions. As a result, Vault has made the new default
+  `rsa-sha2-256` for RSA CA keys. It is strongly encouraged for all users to
+  migrate to `rsa-sha2-256` or `default` if the role was created with an
+  explicit `algorithm_signer=rsa-sha` parameter or has been migrated to such.
 
 ### Sample Payload
 

website/content/docs/secrets/ssh/signed-ssh-certificates.mdx

@@ -254,6 +254,7 @@ accidentally SSHing into an unmanaged or malicious machine.
     ```text
     $ vault write ssh-host-signer/roles/hostrole \
         key_type=ca \
+        algorithm_signer=rsa-sha2-256 \
         ttl=87600h \
         allow_host_certificates=true \
         allowed_domains="localdomain,example.com" \

website/content/docs/upgrading/upgrade-to-1.10.x.mdx

@@ -0,0 +1,20 @@
+---
+layout: docs
+page_title: Upgrading to Vault 1.10.x - Guides
+description: |-
+  This page contains the list of deprecations and important or breaking changes
+  for Vault 1.10.x. Please read it carefully.
+---
+
+# Overview
+
+This page contains the list of deprecations and important or breaking changes
+for Vault 1.10.x compared to 1.9. Please read it carefully.
+
+## SSH Secrets Engine
+
+The new default value of `algorithm_signer` for SSH CA roles has been changed
+to `rsa-sha2-256` from `ssh-rsa`. Existing roles will be migrated to
+explicitly specify the `algorithm_signer=ssh-rsa` for RSA keys if they used
+the implicit (empty) default, but newly created roles will use the new default
+value (preferring a literal `default` which presently uses `rsa-sha2-256`).

website/data/docs-nav-data.json

@@ -1494,6 +1494,10 @@
         "title": "Upgrade Plugins",
         "path": "upgrading/plugins"
       },
+      {
+        "title": "Upgrade to 1.10.x",
+        "path": "upgrading/upgrade-to-1.10.x"
+      },
       {
         "title": "Upgrade to 1.9.x",
         "path": "upgrading/upgrade-to-1.9.x"