From b0370aa9b8d3967d8577f00bf0d06ab496d702ba Mon Sep 17 00:00:00 2001 From: joerger Date: Tue, 4 Oct 2022 11:35:57 -0700 Subject: [PATCH] Revert change from PKCS1 to PKCS8. --- api/utils/keys/signer.go | 19 +++++++++--------- lib/auth/native/native.go | 25 +++++------------------- lib/auth/native/native_test.go | 35 ++++++++++++++++++++++++++-------- 3 files changed, 41 insertions(+), 38 deletions(-) diff --git a/api/utils/keys/signer.go b/api/utils/keys/signer.go index 15705a9016a3f..8e7b5dfb8367f 100644 --- a/api/utils/keys/signer.go +++ b/api/utils/keys/signer.go @@ -18,6 +18,7 @@ package keys import ( "crypto" + "crypto/rsa" "crypto/tls" "crypto/x509" "encoding/pem" @@ -51,20 +52,18 @@ type StandardSigner struct { keyPEM []byte } -// NewStandardSigner creates a new StandardSigner from the given *rsa.PrivateKey, *ecdsa.PrivateKey or ed25519.PrivateKey. -func NewStandardSigner(signer crypto.Signer) (*StandardSigner, error) { - keyDER, err := x509.MarshalPKCS8PrivateKey(signer) - if err != nil { - return nil, trace.Wrap(err) - } - +// NewStandardSigner creates a new StandardSigner from the given *rsa.PrivateKey. +func NewRSASigner(rsaKey *rsa.PrivateKey) (*StandardSigner, error) { + // We encode the private key in PKCS #1, ASN.1 DER form + // instead of PKCS #8 to maintain compatibility with some + // third party clients. keyPEM := pem.EncodeToMemory(&pem.Block{ - Type: PKCS8PrivateKeyType, + Type: PKCS1PrivateKeyType, Headers: nil, - Bytes: keyDER, + Bytes: x509.MarshalPKCS1PrivateKey(rsaKey), }) - return newStandardSigner(signer, keyPEM), nil + return newStandardSigner(rsaKey, keyPEM), nil } func newStandardSigner(signer crypto.Signer, keyPEM []byte) *StandardSigner { diff --git a/lib/auth/native/native.go b/lib/auth/native/native.go index 710fcb573eae2..1ea024a0c851f 100644 --- a/lib/auth/native/native.go +++ b/lib/auth/native/native.go @@ -20,8 +20,6 @@ import ( "context" "crypto/rand" "crypto/rsa" - "crypto/x509" - "encoding/pem" "fmt" "strings" "sync" @@ -57,24 +55,11 @@ var startPrecomputeOnce sync.Once // GenerateKeyPair generates a new RSA key pair. func GenerateKeyPair() ([]byte, []byte, error) { - priv, err := getOrGenerateRSAPrivateKey() + priv, err := GeneratePrivateKey() if err != nil { return nil, nil, trace.Wrap(err) } - - privPEM := pem.EncodeToMemory(&pem.Block{ - Type: "RSA PRIVATE KEY", - Headers: nil, - Bytes: x509.MarshalPKCS1PrivateKey(priv), - }) - - pub, err := ssh.NewPublicKey(&priv.PublicKey) - if err != nil { - return nil, nil, trace.Wrap(err) - } - pubPEM := ssh.MarshalAuthorizedKey(pub) - - return privPEM, pubPEM, nil + return priv.PrivateKeyPEM(), priv.MarshalSSHPublicKey(), nil } // GeneratePrivateKey generates a new RSA private key. @@ -83,7 +68,7 @@ func GeneratePrivateKey() (*keys.PrivateKey, error) { if err != nil { return nil, trace.Wrap(err) } - rsaSigner, err := keys.NewStandardSigner(rsaKey) + rsaSigner, err := keys.NewRSASigner(rsaKey) if err != nil { return nil, trace.Wrap(err) } @@ -368,8 +353,8 @@ func (k *Keygen) GenerateUserCertWithoutValidation(c services.UserCertParams) ([ // BuildPrincipals takes a hostID, nodeName, clusterName, and role and builds a list of // principals to insert into a certificate. This function is backward compatible with // older clients which means: -// * If RoleAdmin is in the list of roles, only a single principal is returned: hostID -// * If nodename is empty, it is not included in the list of principals. +// - If RoleAdmin is in the list of roles, only a single principal is returned: hostID +// - If nodename is empty, it is not included in the list of principals. func BuildPrincipals(hostID string, nodeName string, clusterName string, roles types.SystemRoles) []string { // TODO(russjones): This should probably be clusterName, but we need to // verify changing this won't break older clients. diff --git a/lib/auth/native/native_test.go b/lib/auth/native/native_test.go index 99193a7576f72..1c45c63ede48c 100644 --- a/lib/auth/native/native_test.go +++ b/lib/auth/native/native_test.go @@ -18,10 +18,17 @@ package native import ( "context" + "crypto/x509" + "encoding/pem" "os" "testing" "time" + "github.com/jonboulle/clockwork" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/ssh" + "gopkg.in/check.v1" + "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" @@ -29,10 +36,6 @@ import ( "github.com/gravitational/teleport/lib/auth/test" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/utils" - - "github.com/jonboulle/clockwork" - "golang.org/x/crypto/ssh" - "gopkg.in/check.v1" ) // TestPrecomputeMode verifies that package enters precompute mode when @@ -89,13 +92,13 @@ func (s *NativeSuite) TestGenerateUserCert(c *check.C) { // TestBuildPrincipals makes sure that the list of principals for a host // certificate is correctly built. -// * If the node has role admin, then only the host ID should be listed +// - If the node has role admin, then only the host ID should be listed // in the principals field. -// * If only a host ID is provided, don't include a empty node name +// - If only a host ID is provided, don't include a empty node name // this is for backward compatibility. -// * If both host ID and node name are given, then both should be included +// - If both host ID and node name are given, then both should be included // on the certificate. -// * If the host ID and node name are the same, only list one. +// - If the host ID and node name are the same, only list one. func (s *NativeSuite) TestBuildPrincipals(c *check.C) { caPrivateKey, _, err := GenerateKeyPair() c.Assert(err, check.IsNil) @@ -249,3 +252,19 @@ func (s *NativeSuite) TestUserCertCompatibility(c *check.C) { c.Assert(extVal, check.Equals, "hello") } } + +// TestGenerateRSAPKSC1Keypair tests that GeneratePrivateKey generates +// a valid PKCS1 rsa key. +func TestGeneratePKSC1RSAKey(t *testing.T) { + t.Parallel() + + priv, err := GeneratePrivateKey() + require.NoError(t, err) + + block, rest := pem.Decode(priv.PrivateKeyPEM()) + require.NoError(t, err) + require.Empty(t, rest) + + _, err = x509.ParsePKCS1PrivateKey(block.Bytes) + require.NoError(t, err) +}