From 8c783087addbd5c1346e9ce21551a842325d8bfd Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Wed, 19 Jun 2024 16:33:27 -0700 Subject: [PATCH 1/3] consolidate (un)marshalling of key PEMs This commit consolidates various functions we have for marshalling private and public keys to and from the PEM format, mostly by replacing functions from lib/utils and lib/tlsca with equivalents in api/utils/keys. The new functions also support ECDSA and Ed25519 keys, which is necessary for the implementation of RFD136. --- api/utils/keys/publickey.go | 89 +++++++++++++++++ integration/kube/fixtures.go | 3 +- lib/auth/auth.go | 2 +- .../integration/integrationv1/awsoidc_test.go | 4 +- lib/auth/keystore/aws_kms_test.go | 4 +- lib/auth/keystore/gcp_kms_test.go | 6 +- lib/auth/keystore/keystore_test.go | 5 +- lib/auth/keystore/manager.go | 6 +- lib/auth/keystore/software.go | 4 +- lib/auth/rotate.go | 17 ++-- lib/auth/tls_test.go | 7 +- lib/client/db/oracle/oracle.go | 5 +- lib/client/identityfile/identity.go | 4 +- lib/cloud/gcp/sql.go | 7 +- lib/jwt/jwk.go | 4 +- lib/jwt/jwt.go | 8 +- lib/jwt/jwt_test.go | 18 ++-- lib/kube/proxy/server_test.go | 4 +- lib/kube/proxy/utils_testing.go | 3 +- lib/multiplexer/multiplexer_test.go | 9 +- lib/service/service.go | 3 +- lib/services/authority.go | 8 +- .../alpnproxy/azure_msi_middleware_test.go | 4 +- lib/srv/alpnproxy/listener.go | 7 +- lib/tbot/config/bot_test.go | 3 +- lib/tlsca/ca.go | 2 +- lib/tlsca/ca_test.go | 3 +- lib/tlsca/parsegen.go | 84 ++++------------ lib/utils/certs.go | 40 ++------ lib/utils/chconn_test.go | 3 +- lib/utils/keys.go | 98 ------------------- lib/web/app/transport.go | 3 +- tool/tbot/kube_test.go | 4 +- 33 files changed, 209 insertions(+), 262 deletions(-) create mode 100644 api/utils/keys/publickey.go delete mode 100644 lib/utils/keys.go diff --git a/api/utils/keys/publickey.go b/api/utils/keys/publickey.go new file mode 100644 index 0000000000000..a5db3f13b9382 --- /dev/null +++ b/api/utils/keys/publickey.go @@ -0,0 +1,89 @@ +// Copyright 2024 Gravitational, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package keys + +import ( + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/rsa" + "crypto/x509" + "encoding/pem" + + "github.com/gravitational/trace" +) + +const ( + // PKCS1PublicKeyType is the PEM encoding type commonly used for PKCS#1, ASN.1 DER form public keys. + PKCS1PublicKeyType = "RSA PUBLIC KEY" + // PKIXPublicKeyType is the PEM encoding type commonly used for PKIX, ASN.1 DER form public keys. + PKIXPublicKeyType = "PUBLIC KEY" +) + +// MarshalPublicKey returns a PEM encoding of the given public key. Encodes RSA keys in PKCS1 format for +// backward compatibility. Only supports *rsa.PublicKey, *ecdsa.PublicKey, and ed25519.PublicKey. +func MarshalPublicKey(pub crypto.PublicKey) ([]byte, error) { + switch pubKey := pub.(type) { + case *rsa.PublicKey: + pubPEM := pem.EncodeToMemory(&pem.Block{ + Type: PKCS1PublicKeyType, + Bytes: x509.MarshalPKCS1PublicKey(pubKey), + }) + return pubPEM, nil + case *ecdsa.PublicKey, ed25519.PublicKey: + der, err := x509.MarshalPKIXPublicKey(pubKey) + if err != nil { + return nil, trace.Wrap(err) + } + pubPEM := pem.EncodeToMemory(&pem.Block{ + Type: PKIXPublicKeyType, + Bytes: der, + }) + return pubPEM, nil + default: + return nil, trace.BadParameter("unsupported public key type %T", pub) + } +} + +// ParsePublicKey parses a PEM-encoded public key. Supports PEM encodings of PKCS#1 or PKIX ASN.1 DER form +// public keys. +func ParsePublicKey(keyPEM []byte) (crypto.PublicKey, error) { + block, _ := pem.Decode(keyPEM) + if block == nil { + return nil, trace.BadParameter("failed to decode public key PEM block") + } + + switch block.Type { + case PKCS1PublicKeyType: + pub, pkcs1Err := x509.ParsePKCS1PublicKey(block.Bytes) + if pkcs1Err != nil { + // Failed to parse as PKCS#1. We have been known to stuff PKIX DER encoded RSA public keys into + // "RSA PUBLIC KEY" PEM blocks, so try to parse as PKIX. + pub, pkixErr := x509.ParsePKIXPublicKey(block.Bytes) + if pkixErr != nil { + // Parsing as both formats failed. We really should expect PKCS#1 in this PEM block, so return + // that error. + return nil, trace.Wrap(pkcs1Err) + } + return pub, nil + } + return pub, nil + case PKIXPublicKeyType: + pub, err := x509.ParsePKIXPublicKey(block.Bytes) + return pub, trace.Wrap(err) + default: + return nil, trace.BadParameter("unsupported public key type %q", block.Type) + } +} diff --git a/integration/kube/fixtures.go b/integration/kube/fixtures.go index ee051466f57e2..42228789a0f1b 100644 --- a/integration/kube/fixtures.go +++ b/integration/kube/fixtures.go @@ -27,6 +27,7 @@ import ( "k8s.io/client-go/rest" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/integration/helpers" "github.com/gravitational/teleport/lib/auth/native" "github.com/gravitational/teleport/lib/services" @@ -91,7 +92,7 @@ func ProxyClient(cfg ProxyConfig) (*kubernetes.Clientset, *rest.Config, error) { if err != nil { return nil, nil, trace.Wrap(err) } - priv, err := tlsca.ParsePrivateKeyPEM(privPEM) + priv, err := keys.ParsePrivateKey(privPEM) if err != nil { return nil, nil, trace.Wrap(err) } diff --git a/lib/auth/auth.go b/lib/auth/auth.go index cdf7daf730330..f22504f71f52f 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -4252,7 +4252,7 @@ func (a *Server) GenerateHostCerts(ctx context.Context, req *proto.HostCertsRequ if _, _, _, _, err := ssh.ParseAuthorizedKey(req.PublicSSHKey); err != nil { return nil, trace.BadParameter("failed to parse SSH public key") } - cryptoPubKey, err := tlsca.ParsePublicKeyPEM(req.PublicTLSKey) + cryptoPubKey, err := keys.ParsePublicKey(req.PublicTLSKey) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/integration/integrationv1/awsoidc_test.go b/lib/auth/integration/integrationv1/awsoidc_test.go index 3bcd19b14868f..ba55cfd542764 100644 --- a/lib/auth/integration/integrationv1/awsoidc_test.go +++ b/lib/auth/integration/integrationv1/awsoidc_test.go @@ -26,12 +26,12 @@ import ( integrationv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/integration/v1" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/integrations/awsoidc" "github.com/gravitational/teleport/lib/jwt" "github.com/gravitational/teleport/lib/tlsca" - "github.com/gravitational/teleport/lib/utils" ) func TestGenerateAWSOIDCToken(t *testing.T) { @@ -114,7 +114,7 @@ func TestGenerateAWSOIDCToken(t *testing.T) { require.NotEmpty(t, ca.GetActiveKeys().JWT) jwtPubKey := ca.GetActiveKeys().JWT[0].PublicKey - publicKey, err := utils.ParsePublicKey(jwtPubKey) + publicKey, err := keys.ParsePublicKey(jwtPubKey) require.NoError(t, err) // Validate JWT against public key diff --git a/lib/auth/keystore/aws_kms_test.go b/lib/auth/keystore/aws_kms_test.go index fce5b6996d7e0..31056f9cd45a4 100644 --- a/lib/auth/keystore/aws_kms_test.go +++ b/lib/auth/keystore/aws_kms_test.go @@ -40,10 +40,10 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/cloud" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/services" - "github.com/gravitational/teleport/lib/utils" ) // TestAWSKMS_deleteUnusedKeys tests the AWS KMS keystore's deleteUnusedKeys @@ -298,7 +298,7 @@ func (f *fakeAWSKMSService) Sign(input *kms.SignInput) (*kms.SignOutput, error) default: return nil, trace.BadParameter("unsupported SigningAlgorithm %q", aws.StringValue(input.SigningAlgorithm)) } - signer, err := utils.ParsePrivateKeyPEM(testRawPrivateKey) + signer, err := keys.ParsePrivateKey(testRawPrivateKey) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/keystore/gcp_kms_test.go b/lib/auth/keystore/gcp_kms_test.go index 60a33612732e0..3405c7b65c550 100644 --- a/lib/auth/keystore/gcp_kms_test.go +++ b/lib/auth/keystore/gcp_kms_test.go @@ -46,6 +46,7 @@ import ( "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/grpc/interceptors" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/keystore/internal/faketime" "github.com/gravitational/teleport/lib/auth/testauthority" "github.com/gravitational/teleport/lib/cloud" @@ -53,7 +54,6 @@ import ( "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/services" "github.com/gravitational/teleport/lib/tlsca" - "github.com/gravitational/teleport/lib/utils" ) const ( @@ -160,7 +160,7 @@ func (f *fakeGCPKMSServer) GetPublicKey(ctx context.Context, req *kmspb.GetPubli return nil, trace.BadParameter("cannot fetch public key, state has value %s", keyState.cryptoKeyVersion.State) } - signer, err := utils.ParsePrivateKeyPEM([]byte(keyState.pem)) + signer, err := keys.ParsePrivateKey([]byte(keyState.pem)) if err != nil { return nil, trace.Wrap(err) } @@ -192,7 +192,7 @@ func (f *fakeGCPKMSServer) AsymmetricSign(ctx context.Context, req *kmspb.Asymme return nil, trace.BadParameter("cannot fetch key, state has value %s", keyState.cryptoKeyVersion.State) } - signer, err := utils.ParsePrivateKeyPEM([]byte(keyState.pem)) + signer, err := keys.ParsePrivateKey([]byte(keyState.pem)) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/keystore/keystore_test.go b/lib/auth/keystore/keystore_test.go index 09ad4016ffc87..a289b3a16942b 100644 --- a/lib/auth/keystore/keystore_test.go +++ b/lib/auth/keystore/keystore_test.go @@ -35,6 +35,7 @@ import ( "golang.org/x/crypto/ssh" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/cloud" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/services" @@ -188,7 +189,7 @@ func TestBackends(t *testing.T) { var err error rawPrivateKeys[i], signer, err = backend.generateRSA(ctx) require.NoError(t, err) - rawPublicKeys[i], err = utils.MarshalPublicKey(signer) + rawPublicKeys[i], err = keys.MarshalPublicKey(signer.Public()) require.NoError(t, err) } @@ -307,7 +308,7 @@ func TestManager(t *testing.T) { jwtSigner, err := manager.GetJWTSigner(ctx, ca) require.NoError(t, err, trace.DebugReport(err)) - pubkeyPem, err := utils.MarshalPublicKey(jwtSigner) + pubkeyPem, err := keys.MarshalPublicKey(jwtSigner.Public()) require.NoError(t, err) require.Equal(t, jwtKeyPair.PublicKey, pubkeyPem) diff --git a/lib/auth/keystore/manager.go b/lib/auth/keystore/manager.go index 4cd820f7edac7..654fa0f80dfb5 100644 --- a/lib/auth/keystore/manager.go +++ b/lib/auth/keystore/manager.go @@ -35,11 +35,11 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/keystore/internal/faketime" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/tlsca" - "github.com/gravitational/teleport/lib/utils" ) // Manager provides an interface to interact with teleport CA private keys, @@ -309,7 +309,7 @@ func (m *Manager) GetJWTSigner(ctx context.Context, ca types.CertAuthority) (cry if !canSign { continue } - pub, err := utils.ParsePublicKey(keyPair.PublicKey) + pub, err := keys.ParsePublicKey(keyPair.PublicKey) if err != nil { return nil, trace.Wrap(err) } @@ -368,7 +368,7 @@ func (m *Manager) NewJWTKeyPair(ctx context.Context) (*types.JWTKeyPair, error) if err != nil { return nil, trace.Wrap(err) } - publicKey, err := utils.MarshalPublicKey(signer) + publicKey, err := keys.MarshalPublicKey(signer.Public()) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/keystore/software.go b/lib/auth/keystore/software.go index 7c326ffe6d2e1..08ee777cd98b0 100644 --- a/lib/auth/keystore/software.go +++ b/lib/auth/keystore/software.go @@ -25,8 +25,8 @@ import ( "github.com/gravitational/trace" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/native" - "github.com/gravitational/teleport/lib/utils" ) type softwareKeyStore struct { @@ -81,7 +81,7 @@ func (s *softwareKeyStore) getSigner(ctx context.Context, rawKey []byte, publicK } func (s *softwareKeyStore) getSignerWithoutPublicKey(ctx context.Context, rawKey []byte) (crypto.Signer, error) { - signer, err := utils.ParsePrivateKeyPEM(rawKey) + signer, err := keys.ParsePrivateKey(rawKey) return signer, trace.Wrap(err) } diff --git a/lib/auth/rotate.go b/lib/auth/rotate.go index 702cab85bc615..896cbcab334b0 100644 --- a/lib/auth/rotate.go +++ b/lib/auth/rotate.go @@ -20,7 +20,6 @@ package auth import ( "context" - "crypto/rsa" "crypto/x509/pkix" "fmt" "time" @@ -32,11 +31,11 @@ import ( "golang.org/x/crypto/ssh" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/keystore" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/tlsca" - "github.com/gravitational/teleport/lib/utils" ) // RotateRequest is a request to start rotation of the certificate authority. @@ -373,17 +372,17 @@ func (a *Server) startNewRotation(ctx context.Context, req rotationReq, ca types if len(req.privateKey) != 0 { log.Infof("Generating CA, using pregenerated test private key.") - rsaKey, err := ssh.ParseRawPrivateKey(req.privateKey) + signer, err := keys.ParsePrivateKey(req.privateKey) if err != nil { return trace.Wrap(err) } if len(activeKeys.SSH) > 0 { - signer, err := ssh.NewSignerFromKey(rsaKey) + sshSigner, err := ssh.NewSignerFromKey(signer) if err != nil { return trace.Wrap(err) } - sshPublicKey := ssh.MarshalAuthorizedKey(signer.PublicKey()) + sshPublicKey := ssh.MarshalAuthorizedKey(sshSigner.PublicKey()) newKeys.SSH = append(newKeys.SSH, &types.SSHKeyPair{ PublicKey: sshPublicKey, PrivateKey: req.privateKey, @@ -393,7 +392,7 @@ func (a *Server) startNewRotation(ctx context.Context, req rotationReq, ca types if len(activeKeys.TLS) > 0 { tlsCert, err := tlsca.GenerateSelfSignedCAWithConfig(tlsca.GenerateCAConfig{ - Signer: rsaKey.(*rsa.PrivateKey), + Signer: signer, Entity: pkix.Name{ CommonName: ca.GetClusterName(), Organization: []string{ca.GetClusterName()}, @@ -412,7 +411,11 @@ func (a *Server) startNewRotation(ctx context.Context, req rotationReq, ca types } if len(activeKeys.JWT) > 0 { - jwtPublicKey, jwtPrivateKey, err := utils.MarshalPrivateKey(rsaKey.(*rsa.PrivateKey)) + jwtPublicKey, err := keys.MarshalPublicKey(signer.Public()) + if err != nil { + return trace.Wrap(err) + } + jwtPrivateKey, err := keys.MarshalPrivateKey(signer) if err != nil { return trace.Wrap(err) } diff --git a/lib/auth/tls_test.go b/lib/auth/tls_test.go index ff4f7819b94ce..58b73d83c5a34 100644 --- a/lib/auth/tls_test.go +++ b/lib/auth/tls_test.go @@ -55,6 +55,7 @@ import ( eventtypes "github.com/gravitational/teleport/api/types/events" "github.com/gravitational/teleport/api/types/wrappers" apiutils "github.com/gravitational/teleport/api/utils" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/auth/join" @@ -2612,7 +2613,7 @@ func TestGenerateCerts(t *testing.T) { require.Error(t, err) require.True(t, trace.IsAccessDenied(err), "trace.IsAccessDenied failed: err=%v (%T)", err, trace.Unwrap(err)) - _, privateKeyPEM, err := utils.MarshalPrivateKey(privateKey.(crypto.Signer)) + privateKeyPEM, err := keys.MarshalPrivateKey(privateKey.(crypto.Signer)) require.NoError(t, err) clientCert, err := tls.X509KeyPair(userCerts.TLS, privateKeyPEM) @@ -4855,7 +4856,7 @@ func TestGRPCServer_DeleteToken(t *testing.T) { func verifyJWT(clock clockwork.Clock, clusterName string, pairs []*types.JWTKeyPair, token string) (*jwt.Claims, error) { errs := []error{} for _, pair := range pairs { - publicKey, err := utils.ParsePublicKey(pair.PublicKey) + publicKey, err := keys.ParsePublicKey(pair.PublicKey) if err != nil { errs = append(errs, trace.Wrap(err)) continue @@ -4889,7 +4890,7 @@ func verifyJWT(clock clockwork.Clock, clusterName string, pairs []*types.JWTKeyP func verifyJWTAWSOIDC(clock clockwork.Clock, clusterName string, pairs []*types.JWTKeyPair, token, issuer string) (*jwt.Claims, error) { errs := []error{} for _, pair := range pairs { - publicKey, err := utils.ParsePublicKey(pair.PublicKey) + publicKey, err := keys.ParsePublicKey(pair.PublicKey) if err != nil { errs = append(errs, trace.Wrap(err)) continue diff --git a/lib/client/db/oracle/oracle.go b/lib/client/db/oracle/oracle.go index 929592c701718..b7f8976262b21 100644 --- a/lib/client/db/oracle/oracle.go +++ b/lib/client/db/oracle/oracle.go @@ -32,6 +32,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" @@ -85,11 +86,11 @@ func createClientWallet(key *client.Key, certPem []byte, password string, wallet } func createJKSWallet(keyPEM, certPEM, caPEM []byte, password string) ([]byte, error) { - key, err := utils.ParsePrivateKey(keyPEM) + key, err := keys.ParsePrivateKey(keyPEM) if err != nil { return nil, trace.Wrap(err) } - privateKey, err := x509.MarshalPKCS8PrivateKey(key) + privateKey, err := x509.MarshalPKCS8PrivateKey(key.Signer) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/client/identityfile/identity.go b/lib/client/identityfile/identity.go index c9e669190bb98..59760cddd575b 100644 --- a/lib/client/identityfile/identity.go +++ b/lib/client/identityfile/identity.go @@ -517,7 +517,7 @@ func writeOracleFormat(cfg WriteConfig, writer ConfigWriter) ([]string, error) { if err != nil { return nil, trace.Wrap(err) } - keyK, err := utils.ParsePrivateKeyPEM(cfg.Key.PrivateKeyPEM()) + keyK, err := keys.ParsePrivateKey(cfg.Key.PrivateKeyPEM()) if err != nil { return nil, trace.Wrap(err) } @@ -527,7 +527,7 @@ func writeOracleFormat(cfg WriteConfig, writer ConfigWriter) ([]string, error) { // issuer for an oracle wallet user_cert, and the server cert we create // is not signed by the DB Client CA, so don't pass trusted certs // (DB Client CA) here. - pf, err := pkcs12.LegacyRC2.WithRand(rand.Reader).Encode(keyK, certBlock, nil, cfg.Password) + pf, err := pkcs12.LegacyRC2.WithRand(rand.Reader).Encode(keyK.Signer, certBlock, nil, cfg.Password) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/cloud/gcp/sql.go b/lib/cloud/gcp/sql.go index 594d19eacb2d7..65350e982347e 100644 --- a/lib/cloud/gcp/sql.go +++ b/lib/cloud/gcp/sql.go @@ -33,6 +33,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/tlsca" ) @@ -128,7 +129,11 @@ func (g *gcpSQLAdminClient) GenerateEphemeralCert(ctx context.Context, db types. } // Create TLS certificate from returned ephemeral certificate and private key. - cert, err := tls.X509KeyPair([]byte(resp.EphemeralCert.Cert), tlsca.MarshalPrivateKeyPEM(pkey)) + keyPEM, err := keys.MarshalPrivateKey(pkey) + if err != nil { + return nil, trace.Wrap(err) + } + cert, err := tls.X509KeyPair([]byte(resp.EphemeralCert.Cert), keyPEM) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/jwt/jwk.go b/lib/jwt/jwk.go index 68d575b86eb66..d8a5c7409f57f 100644 --- a/lib/jwt/jwk.go +++ b/lib/jwt/jwk.go @@ -28,8 +28,8 @@ import ( "github.com/gravitational/trace" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/utils" ) // JWK is a JSON Web Key, described in detail in RFC 7517. @@ -61,7 +61,7 @@ func KeyID(pub *rsa.PublicKey) string { // MarshalJWK will marshal a supported public key into JWK format. func MarshalJWK(bytes []byte) (JWK, error) { // Parse the public key and validate type. - p, err := utils.ParsePublicKey(bytes) + p, err := keys.ParsePublicKey(bytes) if err != nil { return JWK{}, trace.Wrap(err) } diff --git a/lib/jwt/jwt.go b/lib/jwt/jwt.go index 083d3b4139107..9f6f82c49fadd 100644 --- a/lib/jwt/jwt.go +++ b/lib/jwt/jwt.go @@ -42,7 +42,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/wrappers" - "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/api/utils/keys" ) // Config defines the clock and PEM encoded bytes of a public and private @@ -559,7 +559,11 @@ func GenerateKeyPair() ([]byte, []byte, error) { return nil, nil, trace.Wrap(err) } - public, private, err := utils.MarshalPrivateKey(privateKey) + public, err := keys.MarshalPublicKey(privateKey.Public()) + if err != nil { + return nil, nil, trace.Wrap(err) + } + private, err := keys.MarshalPrivateKey(privateKey) if err != nil { return nil, nil, trace.Wrap(err) } diff --git a/lib/jwt/jwt_test.go b/lib/jwt/jwt_test.go index 1ca69146057a1..1c30979f8d406 100644 --- a/lib/jwt/jwt_test.go +++ b/lib/jwt/jwt_test.go @@ -27,14 +27,14 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types/wrappers" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/utils" ) func TestSignAndVerify(t *testing.T) { _, privateBytes, err := GenerateKeyPair() require.NoError(t, err) - privateKey, err := utils.ParsePrivateKey(privateBytes) + privateKey, err := keys.ParsePrivateKey(privateBytes) require.NoError(t, err) clock := clockwork.NewFakeClockAt(time.Now()) @@ -73,9 +73,9 @@ func TestSignAndVerify(t *testing.T) { func TestPublicOnlyVerifyAzure(t *testing.T) { publicBytes, privateBytes, err := GenerateKeyPair() require.NoError(t, err) - privateKey, err := utils.ParsePrivateKey(privateBytes) + privateKey, err := keys.ParsePrivateKey(privateBytes) require.NoError(t, err) - publicKey, err := utils.ParsePublicKey(publicBytes) + publicKey, err := keys.ParsePublicKey(publicBytes) require.NoError(t, err) // Create a new key that can sign and verify tokens. @@ -118,9 +118,9 @@ func TestPublicOnlyVerifyAzure(t *testing.T) { func TestPublicOnlyVerify(t *testing.T) { publicBytes, privateBytes, err := GenerateKeyPair() require.NoError(t, err) - privateKey, err := utils.ParsePrivateKey(privateBytes) + privateKey, err := keys.ParsePrivateKey(privateBytes) require.NoError(t, err) - publicKey, err := utils.ParsePublicKey(publicBytes) + publicKey, err := keys.ParsePublicKey(publicBytes) require.NoError(t, err) clock := clockwork.NewFakeClockAt(time.Now()) @@ -175,7 +175,7 @@ func TestPublicOnlyVerify(t *testing.T) { func TestKey_SignAndVerifyPROXY(t *testing.T) { _, privateBytes, err := GenerateKeyPair() require.NoError(t, err) - privateKey, err := utils.ParsePrivateKey(privateBytes) + privateKey, err := keys.ParsePrivateKey(privateBytes) require.NoError(t, err) clock := clockwork.NewFakeClockAt(time.Now()) @@ -250,7 +250,7 @@ func TestKey_SignAndVerifyPROXY(t *testing.T) { func TestKey_SignAndVerifyAWSOIDC(t *testing.T) { _, privateBytes, err := GenerateKeyPair() require.NoError(t, err) - privateKey, err := utils.ParsePrivateKey(privateBytes) + privateKey, err := keys.ParsePrivateKey(privateBytes) require.NoError(t, err) clock := clockwork.NewFakeClockAt(time.Now()) @@ -314,7 +314,7 @@ func TestKey_SignAndVerifyAWSOIDC(t *testing.T) { func TestExpiry(t *testing.T) { _, privateBytes, err := GenerateKeyPair() require.NoError(t, err) - privateKey, err := utils.ParsePrivateKey(privateBytes) + privateKey, err := keys.ParsePrivateKey(privateBytes) require.NoError(t, err) clock := clockwork.NewFakeClockAt(time.Now()) diff --git a/lib/kube/proxy/server_test.go b/lib/kube/proxy/server_test.go index b5870eaebb3cd..6f494978761c5 100644 --- a/lib/kube/proxy/server_test.go +++ b/lib/kube/proxy/server_test.go @@ -42,11 +42,11 @@ import ( "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/authclient" testingkubemock "github.com/gravitational/teleport/lib/kube/proxy/testing/kube_server" "github.com/gravitational/teleport/lib/reversetunnel" "github.com/gravitational/teleport/lib/tlsca" - "github.com/gravitational/teleport/lib/utils" ) func TestServeConfigureError(t *testing.T) { @@ -72,7 +72,7 @@ func TestMTLSClientCAs(t *testing.T) { addCA := func(t *testing.T, name string) (key, cert []byte) { cert, err := tlsca.GenerateSelfSignedCAWithSigner(caKey, pkix.Name{CommonName: name}, nil, time.Minute) require.NoError(t, err) - _, key, err = utils.MarshalPrivateKey(caKey) + key, err = keys.MarshalPrivateKey(caKey) require.NoError(t, err) ca, err := types.NewCertAuthority(types.CertAuthoritySpecV2{ Type: types.HostCA, diff --git a/lib/kube/proxy/utils_testing.go b/lib/kube/proxy/utils_testing.go index dda4a92dc51b6..626fd4889adeb 100644 --- a/lib/kube/proxy/utils_testing.go +++ b/lib/kube/proxy/utils_testing.go @@ -46,6 +46,7 @@ import ( apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" apievents "github.com/gravitational/teleport/api/types/events" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/auth/keygen" @@ -507,7 +508,7 @@ func (c *TestContext) GenTestKubeClientTLSCert(t *testing.T, userName, kubeClust privPEM, _, err := testauthority.New().GenerateKeyPair() require.NoError(t, err) - priv, err := tlsca.ParsePrivateKeyPEM(privPEM) + priv, err := keys.ParsePrivateKey(privPEM) require.NoError(t, err) id := tlsca.Identity{ diff --git a/lib/multiplexer/multiplexer_test.go b/lib/multiplexer/multiplexer_test.go index dbdb5698a30d8..c7301c970bc33 100644 --- a/lib/multiplexer/multiplexer_test.go +++ b/lib/multiplexer/multiplexer_test.go @@ -47,6 +47,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/native" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/fixtures" @@ -745,7 +746,9 @@ func TestMux(t *testing.T) { DNSNames: []string{"127.0.0.1"}, }) require.NoError(t, err) - serverCert, err := tls.X509KeyPair(serverPEM, tlsca.MarshalPrivateKeyPEM(serverRSAKey)) + serverKeyPEM, err := keys.MarshalPrivateKey(serverRSAKey) + require.NoError(t, err) + serverCert, err := tls.X509KeyPair(serverPEM, serverKeyPEM) require.NoError(t, err) // Sign client certificate with database access identity. @@ -765,7 +768,9 @@ func TestMux(t *testing.T) { NotAfter: time.Now().Add(time.Hour), }) require.NoError(t, err) - clientCert, err := tls.X509KeyPair(clientPEM, tlsca.MarshalPrivateKeyPEM(clientRSAKey)) + clientKeyPEM, err := keys.MarshalPrivateKey(clientRSAKey) + require.NoError(t, err) + clientCert, err := tls.X509KeyPair(clientPEM, clientKeyPEM) require.NoError(t, err) webLis, err := NewWebListener(WebListenerConfig{ diff --git a/lib/service/service.go b/lib/service/service.go index 6ccd5aeae4dde..b1c1f225fb4fe 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -79,6 +79,7 @@ import ( apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/api/utils/aws" "github.com/gravitational/teleport/api/utils/grpc/interceptors" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib" "github.com/gravitational/teleport/lib/agentless" "github.com/gravitational/teleport/lib/auditd" @@ -5106,7 +5107,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { } func (process *TeleportProcess) getPROXYSigner(ident *state.Identity) (multiplexer.PROXYHeaderSigner, error) { - signer, err := utils.ParsePrivateKeyPEM(ident.KeyBytes) + signer, err := keys.ParsePrivateKey(ident.KeyBytes) if err != nil { return nil, trace.Wrap(err, "could not parse identity's private key") } diff --git a/lib/services/authority.go b/lib/services/authority.go index 08d864051e90a..88b9821d2f478 100644 --- a/lib/services/authority.go +++ b/lib/services/authority.go @@ -143,7 +143,7 @@ func checkDatabaseCA(cai types.CertAuthority) error { if len(pair.Cert) > 0 { _, err = tls.X509KeyPair(pair.Cert, pair.Key) } else { - _, err = utils.ParsePrivateKey(pair.Key) + _, err = keys.ParsePrivateKey(pair.Key) } if err != nil { return trace.Wrap(err) @@ -199,12 +199,12 @@ func checkJWTKeys(cai types.CertAuthority) error { for _, pair := range ca.GetTrustedJWTKeyPairs() { // TODO(nic): validate PKCS11 private keys if len(pair.PrivateKey) > 0 && pair.PrivateKeyType == types.PrivateKeyType_RAW { - privateKey, err = utils.ParsePrivateKey(pair.PrivateKey) + privateKey, err = keys.ParsePrivateKey(pair.PrivateKey) if err != nil { return trace.Wrap(err) } } - publicKey, err := utils.ParsePublicKey(pair.PublicKey) + publicKey, err := keys.ParsePublicKey(pair.PublicKey) if err != nil { return trace.Wrap(err) } @@ -240,7 +240,7 @@ func checkSAMLIDPCA(cai types.CertAuthority) error { if len(pair.Cert) > 0 { _, err = tls.X509KeyPair(pair.Cert, pair.Key) } else { - _, err = utils.ParsePrivateKey(pair.Key) + _, err = keys.ParsePrivateKey(pair.Key) } if err != nil { return trace.Wrap(err) diff --git a/lib/srv/alpnproxy/azure_msi_middleware_test.go b/lib/srv/alpnproxy/azure_msi_middleware_test.go index 41c8f0b1c6184..47efd9232025b 100644 --- a/lib/srv/alpnproxy/azure_msi_middleware_test.go +++ b/lib/srv/alpnproxy/azure_msi_middleware_test.go @@ -34,16 +34,16 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/jwt" - "github.com/gravitational/teleport/lib/utils" ) func TestAzureMSIMiddlewareHandleRequest(t *testing.T) { newPrivateKey := func() crypto.Signer { _, privateBytes, err := jwt.GenerateKeyPair() require.NoError(t, err) - privateKey, err := utils.ParsePrivateKey(privateBytes) + privateKey, err := keys.ParsePrivateKey(privateBytes) require.NoError(t, err) return privateKey } diff --git a/lib/srv/alpnproxy/listener.go b/lib/srv/alpnproxy/listener.go index f664df8ba258c..20495af150537 100644 --- a/lib/srv/alpnproxy/listener.go +++ b/lib/srv/alpnproxy/listener.go @@ -29,6 +29,7 @@ import ( "github.com/gravitational/trace" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" ) @@ -245,7 +246,11 @@ func (r *CertGenListener) generateCertFor(host string) (*tls.Certificate, error) return nil, trace.Wrap(err) } - cert, err := tls.X509KeyPair(certPem, tlsca.MarshalPrivateKeyPEM(certKey)) + keyPem, err := keys.MarshalPrivateKey(certKey) + if err != nil { + return nil, trace.Wrap(err) + } + cert, err := tls.X509KeyPair(certPem, keyPem) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/tbot/config/bot_test.go b/lib/tbot/config/bot_test.go index 5afa7dd843eed..2aed70903edb0 100644 --- a/lib/tbot/config/bot_test.go +++ b/lib/tbot/config/bot_test.go @@ -36,6 +36,7 @@ import ( machineidv1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/machineid/v1" trustpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/trust/v1" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/testauthority" "github.com/gravitational/teleport/lib/fixtures" "github.com/gravitational/teleport/lib/services" @@ -207,7 +208,7 @@ func getTestIdent(t *testing.T, username string, reqs ...identRequest) *identity tlsPublicKeyPEM, err := tlsca.MarshalPublicKeyFromPrivateKeyPEM(sshPrivateKey) require.NoError(t, err) - tlsPublicKey, err := tlsca.ParsePublicKeyPEM(tlsPublicKeyPEM) + tlsPublicKey, err := keys.ParsePublicKey(tlsPublicKeyPEM) require.NoError(t, err) // Note: it'd be nice to make this more universally useful in our tests at diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index 0c6bf7266325c..ff67923ae8ae6 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -71,7 +71,7 @@ func FromKeys(certPEM, keyPEM []byte) (*CertAuthority, error) { return nil, trace.Wrap(err) } if len(keyPEM) != 0 { - ca.Signer, err = ParsePrivateKeyPEM(keyPEM) + ca.Signer, err = keys.ParsePrivateKey(keyPEM) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/tlsca/ca_test.go b/lib/tlsca/ca_test.go index a2129769b5663..e38af89fdb8bf 100644 --- a/lib/tlsca/ca_test.go +++ b/lib/tlsca/ca_test.go @@ -38,6 +38,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/constants" apievents "github.com/gravitational/teleport/api/types/events" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/fixtures" ) @@ -58,7 +59,7 @@ func TestPrincipals(t *testing.T) { { name: "FromCertAndSigner", createFunc: func() (*CertAuthority, error) { - signer, err := ParsePrivateKeyPEM([]byte(fixtures.TLSCAKeyPEM)) + signer, err := keys.ParsePrivateKey([]byte(fixtures.TLSCAKeyPEM)) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/tlsca/parsegen.go b/lib/tlsca/parsegen.go index 6353b2d0d10a7..4bdce3ca8bac7 100644 --- a/lib/tlsca/parsegen.go +++ b/lib/tlsca/parsegen.go @@ -21,7 +21,6 @@ package tlsca import ( "bytes" "crypto" - "crypto/ecdsa" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -35,6 +34,7 @@ import ( "github.com/jonboulle/clockwork" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/utils" ) @@ -194,77 +194,27 @@ func ParseCertificatePEMs(bytes []byte) ([]*x509.Certificate, error) { return certs, nil } -// ParsePrivateKeyPEM parses PEM-encoded private key -func ParsePrivateKeyPEM(bytes []byte) (crypto.Signer, error) { - block, _ := pem.Decode(bytes) - if block == nil { - return nil, trace.BadParameter("expected PEM-encoded block") - } - return ParsePrivateKeyDER(block.Bytes) -} - -// ParsePrivateKeyDER parses unencrypted DER-encoded private key -func ParsePrivateKeyDER(der []byte) (crypto.Signer, error) { - generalKey, err := x509.ParsePKCS8PrivateKey(der) - if err != nil { - generalKey, err = x509.ParsePKCS1PrivateKey(der) - if err != nil { - generalKey, err = x509.ParseECPrivateKey(der) - if err != nil { - return nil, trace.BadParameter("failed parsing private key") - } - } - } - - switch k := generalKey.(type) { - case *rsa.PrivateKey: - return k, nil - case *ecdsa.PrivateKey: - return k, nil - } - - return nil, trace.BadParameter("unsupported private key type") -} - -// ParsePublicKeyPEM parses public key PEM -func ParsePublicKeyPEM(bytes []byte) (interface{}, error) { - block, _ := pem.Decode(bytes) - if block == nil { - return nil, trace.BadParameter("expected PEM-encoded block") - } - return ParsePublicKeyDER(block.Bytes) -} - -// ParsePublicKeyDER parses unencrypted DER-encoded publice key -func ParsePublicKeyDER(der []byte) (crypto.PublicKey, error) { - generalKey, err := x509.ParsePKIXPublicKey(der) - if err != nil { - return nil, trace.Wrap(err) - } - return generalKey, nil -} - // MarshalPublicKeyFromPrivateKeyPEM extracts public key from private key // and returns PEM marshaled key func MarshalPublicKeyFromPrivateKeyPEM(privateKey crypto.PrivateKey) ([]byte, error) { - rsaPrivateKey, ok := privateKey.(*rsa.PrivateKey) - if !ok { - return nil, trace.BadParameter("expected RSA key") + // TODO(nklaassen): DELETE IN 18.0.0 when this quirk is no longer necessary because all parsers can handle + // either format. + if rsaPrivateKey, ok := privateKey.(*rsa.PrivateKey); ok { + // This is weird and historical: we're marshaling an RSA public key into PKIX DER format and then + // putting it into an "RSA PUBLIC KEY" PEM block. Normally RSA keys should either be: + // - PKCS#1 DER format in an "RSA PUBLIC KEY" PEM block + // - PKIX DER format in a "PUBLIC KEY" PEM block + derBytes, err := x509.MarshalPKIXPublicKey(rsaPrivateKey.Public()) + if err != nil { + return nil, trace.Wrap(err) + } + return pem.EncodeToMemory(&pem.Block{Type: keys.PKCS1PublicKeyType, Bytes: derBytes}), nil } - rsaPublicKey := rsaPrivateKey.Public() - derBytes, err := x509.MarshalPKIXPublicKey(rsaPublicKey) - if err != nil { - return nil, trace.Wrap(err) + // All private keys in the standard library implement crypto.Signer, which gives access to the public key. + if signer, ok := privateKey.(crypto.Signer); ok { + return keys.MarshalPublicKey(signer.Public()) } - return pem.EncodeToMemory(&pem.Block{Type: "RSA PUBLIC KEY", Bytes: derBytes}), nil -} - -// MarshalPrivateKeyPEM marshals provided rsa.PrivateKey into PEM format. -func MarshalPrivateKeyPEM(privateKey *rsa.PrivateKey) []byte { - return pem.EncodeToMemory(&pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(privateKey), - }) + return nil, trace.BadParameter("unsupported key type %T", privateKey) } // MarshalCertificatePEM takes a *x509.Certificate and returns the PEM diff --git a/lib/utils/certs.go b/lib/utils/certs.go index 53c9f2049ac0a..c2f3b96c5b482 100644 --- a/lib/utils/certs.go +++ b/lib/utils/certs.go @@ -16,7 +16,6 @@ package utils import ( "bytes" "crypto" - "crypto/ecdsa" "crypto/rand" "crypto/rsa" "crypto/tls" @@ -30,9 +29,9 @@ import ( "github.com/gravitational/trace" "github.com/jonboulle/clockwork" - "github.com/sirupsen/logrus" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/tlsutils" ) @@ -42,11 +41,11 @@ func ParseKeyStorePEM(keyPEM, certPEM string) (*KeyStore, error) { if err != nil { return nil, trace.Wrap(err) } - key, err := ParsePrivateKeyPEM([]byte(keyPEM)) + key, err := keys.ParsePrivateKey([]byte(keyPEM)) if err != nil { return nil, trace.Wrap(err) } - rsaKey, ok := key.(*rsa.PrivateKey) + rsaKey, ok := key.Signer.(*rsa.PrivateKey) if !ok { return nil, trace.BadParameter("key of type %T is not supported, only RSA keys are supported", key) } @@ -105,37 +104,10 @@ func GenerateSelfSignedSigningCert(entity pkix.Name, dnsNames []string, ttl time return keyPEM, certPEM, nil } -// ParsePrivateKeyPEM parses PEM-encoded private key +// ParsePrivateKeyPEM parses PEM-encoded private key. +// Prefer [keys.ParsePrivateKey], this will be deleted after references are removed from teleport.e. func ParsePrivateKeyPEM(bytes []byte) (crypto.Signer, error) { - block, _ := pem.Decode(bytes) - if block == nil { - return nil, trace.BadParameter("expected PEM-encoded block") - } - return ParsePrivateKeyDER(block.Bytes) -} - -// ParsePrivateKeyDER parses unencrypted DER-encoded private key -func ParsePrivateKeyDER(der []byte) (crypto.Signer, error) { - generalKey, err := x509.ParsePKCS8PrivateKey(der) - if err != nil { - generalKey, err = x509.ParsePKCS1PrivateKey(der) - if err != nil { - generalKey, err = x509.ParseECPrivateKey(der) - if err != nil { - logrus.Errorf("Failed to parse key: %v.", err) - return nil, trace.BadParameter("failed parsing private key") - } - } - } - - switch k := generalKey.(type) { - case *rsa.PrivateKey: - return k, nil - case *ecdsa.PrivateKey: - return k, nil - } - - return nil, trace.BadParameter("unsupported private key type") + return keys.ParsePrivateKey(bytes) } // VerifyCertificateExpiry checks the certificate's expiration status. diff --git a/lib/utils/chconn_test.go b/lib/utils/chconn_test.go index 712a4b6d83946..7a42fe0f663d2 100644 --- a/lib/utils/chconn_test.go +++ b/lib/utils/chconn_test.go @@ -31,6 +31,7 @@ import ( "golang.org/x/crypto/ssh" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/sshutils" ) @@ -94,7 +95,7 @@ func startSSHServer(t *testing.T, listener net.Listener, sshConnCh chan<- sshCon privateKey, err := rsa.GenerateKey(rand.Reader, constants.RSAKeySize) require.NoError(t, err) - _, private, err := MarshalPrivateKey(privateKey) + private, err := keys.MarshalPrivateKey(privateKey) require.NoError(t, err) signer, err := ssh.ParsePrivateKey(private) diff --git a/lib/utils/keys.go b/lib/utils/keys.go deleted file mode 100644 index b811161570ac7..0000000000000 --- a/lib/utils/keys.go +++ /dev/null @@ -1,98 +0,0 @@ -/* - * Teleport - * Copyright (C) 2023 Gravitational, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -package utils - -import ( - "crypto" - "crypto/rsa" - "crypto/x509" - "encoding/pem" - - "github.com/gravitational/trace" -) - -// MarshalPrivateKey will return a PEM encoded crypto.Signer. Only supports -// RSA private keys. -func MarshalPrivateKey(key crypto.Signer) ([]byte, []byte, error) { - switch privateKey := key.(type) { - case *rsa.PrivateKey: - privateBytes := pem.EncodeToMemory(&pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(privateKey), - }) - - publicKey, ok := key.Public().(*rsa.PublicKey) - if !ok { - return nil, nil, trace.BadParameter("invalid key type: %T", publicKey) - } - publicBytes := pem.EncodeToMemory(&pem.Block{ - Type: "RSA PUBLIC KEY", - Bytes: x509.MarshalPKCS1PublicKey(publicKey), - }) - - return publicBytes, privateBytes, nil - default: - return nil, nil, trace.BadParameter("unsupported private key type %T", key) - } -} - -// MarshalPublicKey returns a PEM encoded public key for a given crypto.Signer -func MarshalPublicKey(signer crypto.Signer) ([]byte, error) { - switch publicKey := signer.Public().(type) { - case *rsa.PublicKey: - return pem.EncodeToMemory(&pem.Block{ - Type: "RSA PUBLIC KEY", - Bytes: x509.MarshalPKCS1PublicKey(publicKey), - }), nil - default: - return nil, trace.BadParameter("unsupported public key type %T", publicKey) - } -} - -// ParsePrivateKey parses a PEM encoded private key and returns a -// crypto.Signer. Only supports RSA private keys. -func ParsePrivateKey(bytes []byte) (crypto.Signer, error) { - block, _ := pem.Decode(bytes) - if block == nil { - return nil, trace.BadParameter("failed to decode private key PEM block") - } - - switch block.Type { - case "RSA PRIVATE KEY": - return x509.ParsePKCS1PrivateKey(block.Bytes) - default: - return nil, trace.BadParameter("unsupported private key type %q", block.Type) - } -} - -// ParsePublicKey parses a PEM encoded public key and returns a -// crypto.PublicKey. Only support RSA public keys. -func ParsePublicKey(bytes []byte) (crypto.PublicKey, error) { - block, _ := pem.Decode(bytes) - if block == nil { - return nil, trace.BadParameter("failed to decode public key PEM block") - } - - switch block.Type { - case "RSA PUBLIC KEY": - return x509.ParsePKCS1PublicKey(block.Bytes) - default: - return nil, trace.BadParameter("unsupported public key type %q", block.Type) - } -} diff --git a/lib/web/app/transport.go b/lib/web/app/transport.go index 408cf0a24c8da..93a768c3238f8 100644 --- a/lib/web/app/transport.go +++ b/lib/web/app/transport.go @@ -35,6 +35,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/types" apiutils "github.com/gravitational/teleport/api/utils" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/authz" "github.com/gravitational/teleport/lib/defaults" @@ -296,7 +297,7 @@ func (t *transport) resignAzureJWTCookie(r *http.Request) error { } // Create a new jwt key using the web session private key to sign a new token. - wsPrivateKey, err := utils.ParsePrivateKey(t.c.ws.GetPriv()) + wsPrivateKey, err := keys.ParsePrivateKey(t.c.ws.GetPriv()) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/kube_test.go b/tool/tbot/kube_test.go index 3ce19e50d35e3..09a29f6c38c9e 100644 --- a/tool/tbot/kube_test.go +++ b/tool/tbot/kube_test.go @@ -32,6 +32,7 @@ import ( "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/identityfile" + "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/lib/fixtures" "github.com/gravitational/teleport/lib/tlsca" ) @@ -54,7 +55,8 @@ func TestGetKubeCredentialData(t *testing.T) { }) require.NoError(t, err) - privateKeyBytes := tlsca.MarshalPrivateKeyPEM(privateKey) + privateKeyBytes, err := keys.MarshalPrivateKey(privateKey) + require.NoError(t, err) idFile := &identityfile.IdentityFile{ PrivateKey: privateKeyBytes, Certs: identityfile.Certs{ From 1e4bba36c169e222a4ba98af473f4f06511e6cae Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Wed, 19 Jun 2024 18:02:32 -0700 Subject: [PATCH 2/3] speed up TestMTLSClientCAs --- lib/kube/proxy/server_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/kube/proxy/server_test.go b/lib/kube/proxy/server_test.go index 6f494978761c5..3da3cb3ba9fa7 100644 --- a/lib/kube/proxy/server_test.go +++ b/lib/kube/proxy/server_test.go @@ -23,11 +23,9 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rand" - "crypto/rsa" "crypto/tls" "crypto/x509" "crypto/x509/pkix" - "encoding/pem" "errors" "fmt" "net" @@ -66,7 +64,7 @@ func TestMTLSClientCAs(t *testing.T) { cas: make(map[string]types.CertAuthority), } // Reuse the same CA private key for performance. - caKey, err := rsa.GenerateKey(rand.Reader, 2048) + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) addCA := func(t *testing.T, name string) (key, cert []byte) { @@ -105,9 +103,8 @@ func TestMTLSClientCAs(t *testing.T) { DNSNames: sans, }) require.NoError(t, err) - keyRaw, err := x509.MarshalECPrivateKey(userHostKey) + keyPEM, err := keys.MarshalPrivateKey(userHostKey) require.NoError(t, err) - keyPEM := pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyRaw}) cert, err := tls.X509KeyPair(certRaw, keyPEM) require.NoError(t, err) return cert From d9a94f74ba772803157c4e47624f072eb5c8d6ab Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Fri, 21 Jun 2024 12:13:07 -0700 Subject: [PATCH 3/3] fix new use in local proxy middleware --- lib/client/local_proxy_middleware.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/client/local_proxy_middleware.go b/lib/client/local_proxy_middleware.go index e093cdb6c6060..f3251f62b284a 100644 --- a/lib/client/local_proxy_middleware.go +++ b/lib/client/local_proxy_middleware.go @@ -378,7 +378,11 @@ func (r *LocalCertGenerator) generateCert(host string) (*tls.Certificate, error) return nil, trace.Wrap(err) } - cert, err := tls.X509KeyPair(certPem, tlsca.MarshalPrivateKeyPEM(certKey)) + keyPEM, err := keys.MarshalPrivateKey(certKey) + if err != nil { + return nil, trace.Wrap(err) + } + cert, err := tls.X509KeyPair(certPem, keyPEM) if err != nil { return nil, trace.Wrap(err) }