From 42b90ff691dd1dd17142b8b279d4f95e2ffad86c Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Wed, 16 Feb 2022 13:28:57 -0600 Subject: [PATCH] Fix ed25519 generated SSH key marshalling (#14101) * Ensure we can issue against generated SSH CA keys This adds a test to ensure that we can issue leaf SSH certificates using the newly generated SSH CA keys. Presently this fails because the ed25519 key private is stored using PKIX's PKCS8 PrivateKey object format rather than using OpenSSH's desired private key format: > path_config_ca_test.go:211: bad case 12: err: failed to parse stored CA private key: ssh: invalid openssh private key format, resp: Signed-off-by: Alexander Scheel * Add dependency on edkey for OpenSSH ed25519 keys As mentioned in various terraform-provider-tls discussions, OpenSSH doesn't understand the standard OpenSSL/PKIX ed25519 key structure (as generated by PKCS8 marshalling). Instead, we need to place it into the OpenSSH RFC 8709 format. As mentioned in this dependency's README, support in golang.org/x/crypto/ssh is presently lacking for this. When the associated CL is merged, we should be able to remove this dep and rely on the (extended) standard library, however, no review progress appears to have been made since the CL was opened by the author. See also: https://go-review.googlesource.com/c/crypto/+/218620/ Signed-off-by: Alexander Scheel --- builtin/logical/ssh/path_config_ca.go | 9 ++++-- builtin/logical/ssh/path_config_ca_test.go | 36 ++++++++++++++++++++-- go.mod | 1 + go.sum | 2 ++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/builtin/logical/ssh/path_config_ca.go b/builtin/logical/ssh/path_config_ca.go index 42ae388a6d..5b75939387 100644 --- a/builtin/logical/ssh/path_config_ca.go +++ b/builtin/logical/ssh/path_config_ca.go @@ -10,6 +10,7 @@ import ( "crypto/rsa" "crypto/x509" "encoding/pem" + "errors" "fmt" "io" @@ -17,6 +18,8 @@ import ( "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" "golang.org/x/crypto/ssh" + + "github.com/mikesmitty/edkey" ) const ( @@ -357,9 +360,9 @@ func generateSSHKeyPair(randomSource io.Reader, keyType string, keyBits int) (st return "", "", err } - marshalled, err := x509.MarshalPKCS8PrivateKey(privateSeed) - if err != nil { - return "", "", err + marshalled := edkey.MarshalED25519PrivateKey(privateSeed) + if marshalled == nil { + return "", "", errors.New("unable to marshal ed25519 private key") } privateBlock = &pem.Block{ diff --git a/builtin/logical/ssh/path_config_ca_test.go b/builtin/logical/ssh/path_config_ca_test.go index d346c5710c..1a04d9dbeb 100644 --- a/builtin/logical/ssh/path_config_ca_test.go +++ b/builtin/logical/ssh/path_config_ca_test.go @@ -191,17 +191,31 @@ func createDeleteHelper(t *testing.T, b logical.Backend, config *logical.Backend } resp, err := b.HandleRequest(context.Background(), caReq) if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("bad case %v: err: %v, resp:%v", index, err, resp) + t.Fatalf("bad case %v: err: %v, resp: %v", index, err, resp) } if !strings.Contains(resp.Data["public_key"].(string), caReq.Data["key_type"].(string)) { t.Fatalf("bad case %v: expected public key of type %v but was %v", index, caReq.Data["key_type"], resp.Data["public_key"]) } + issueOptions := map[string]interface{}{ + "public_key": testCAPublicKeyEd25519, + } + issueReq := &logical.Request{ + Path: "sign/ca-issuance", + Operation: logical.UpdateOperation, + Storage: config.StorageView, + Data: issueOptions, + } + resp, err = b.HandleRequest(context.Background(), issueReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad case %v: err: %v, resp: %v", index, err, resp) + } + // Delete the configured keys caReq.Operation = logical.DeleteOperation resp, err = b.HandleRequest(context.Background(), caReq) if err != nil || (resp != nil && resp.IsError()) { - t.Fatalf("bad case %v: err: %v, resp:%v", index, err, resp) + t.Fatalf("bad case %v: err: %v, resp: %v", index, err, resp) } } @@ -235,6 +249,24 @@ func TestSSH_ConfigCAKeyTypes(t *testing.T) { {"ed25519", 0}, } + // Create a role for ssh signing. + roleOptions := map[string]interface{}{ + "allow_user_certificates": true, + "allowed_users": "*", + "key_type": "ca", + "ttl": "30s", + } + roleReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "roles/ca-issuance", + Data: roleOptions, + Storage: config.StorageView, + } + _, err = b.HandleRequest(context.Background(), roleReq) + if err != nil { + t.Fatalf("Cannot create role to issue against: %s", err) + } + for index, scenario := range cases { createDeleteHelper(t, b, config, index, scenario.keyType, scenario.keyBits) } diff --git a/go.mod b/go.mod index 73b0485278..14dae7da82 100644 --- a/go.mod +++ b/go.mod @@ -306,6 +306,7 @@ require ( github.com/mattn/go-isatty v0.0.14 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect github.com/miekg/dns v1.1.41 // indirect + github.com/mikesmitty/edkey v0.0.0-20170222072505-3356ea4e686a // indirect github.com/mitchellh/hashstructure v1.0.0 // indirect github.com/mitchellh/iochan v1.0.0 // indirect github.com/mitchellh/pointerstructure v1.2.0 // indirect diff --git a/go.sum b/go.sum index 654db47210..dc98c60a2e 100644 --- a/go.sum +++ b/go.sum @@ -1160,6 +1160,8 @@ github.com/miekg/dns v1.1.40/go.mod h1:KNUDUusw/aVsxyTYZM1oqvCicbwhgbNgztCETuNZ7 github.com/miekg/dns v1.1.41 h1:WMszZWJG0XmzbK9FEmzH2TVcqYzFesusSIB41b8KHxY= github.com/miekg/dns v1.1.41/go.mod h1:p6aan82bvRIyn+zDIv9xYNUpwa73JcSh9BKwknJysuI= github.com/miekg/pkcs11 v1.0.3/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs= +github.com/mikesmitty/edkey v0.0.0-20170222072505-3356ea4e686a h1:eU8j/ClY2Ty3qdHnn0TyW3ivFoPC/0F1gQZz8yTxbbE= +github.com/mikesmitty/edkey v0.0.0-20170222072505-3356ea4e686a/go.mod h1:v8eSC2SMp9/7FTKUncp7fH9IwPfw+ysMObcEz5FWheQ= github.com/mistifyio/go-zfs v2.1.2-0.20190413222219-f784269be439+incompatible/go.mod h1:8AuVvqP/mXw1px98n46wfvcGfQ4ci2FwoAjKYxuo3Z4= github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc= github.com/mitchellh/cli v1.1.0/go.mod h1:xcISNoH86gajksDmfB23e/pu+B+GeFRMYmoHXxx3xhI=