diff --git a/builtin/logical/ssh/backend_test.go b/builtin/logical/ssh/backend_test.go index dc6038f99..1287dceda 100644 --- a/builtin/logical/ssh/backend_test.go +++ b/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, diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index 73aa09b50..d46ad622e 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/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. diff --git a/builtin/logical/ssh/path_sign.go b/builtin/logical/ssh/path_sign.go index 7fb5f25ec..0f150710d 100644 --- a/builtin/logical/ssh/path_sign.go +++ b/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) diff --git a/changelog/14006.txt b/changelog/14006.txt new file mode 100644 index 000000000..30b578061 --- /dev/null +++ b/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 +``` diff --git a/website/content/api-docs/secret/ssh.mdx b/website/content/api-docs/secret/ssh.mdx index 2398f1745..e58c742d0 100644 --- a/website/content/api-docs/secret/ssh.mdx +++ b/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 diff --git a/website/content/docs/secrets/ssh/signed-ssh-certificates.mdx b/website/content/docs/secrets/ssh/signed-ssh-certificates.mdx index 4ee36981f..82d0b666b 100644 --- a/website/content/docs/secrets/ssh/signed-ssh-certificates.mdx +++ b/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" \ diff --git a/website/content/docs/upgrading/upgrade-to-1.10.x.mdx b/website/content/docs/upgrading/upgrade-to-1.10.x.mdx new file mode 100644 index 000000000..9ffc0d54b --- /dev/null +++ b/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`). diff --git a/website/data/docs-nav-data.json b/website/data/docs-nav-data.json index 10ba45421..160694ebf 100644 --- a/website/data/docs-nav-data.json +++ b/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"