[v10] Revert change from PKCS1 to PKCS8

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 {

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.

lib/auth/native/native_test.go

@@ -18,21 +18,24 @@ 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"
 	"github.com/gravitational/teleport/api/utils/sshutils"
 	"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)
+}