fix: implement NewKeyPair

This function was completely broken. This implements it properly.

BREAKING CHANGE:

The NewCSRAndIdentity function has been removed in favor of explicit
NewRSACSRAndIdentity, and NewEd25519CSRAndIdentity functions. This aligns with
the existing pattern in this package. To resolve this breaking change, use
NewEd25519CSRAndIdentity instead of NewCSRAndIdentity.

Signed-off-by: Andrew Rynhard <andrew@rynhard.io>

tls/provider.go

@@ -81,7 +81,7 @@ func (p *certificateProvider) update() (ca []byte, cert tls.Certificate, err err
 		identity *talosx509.PEMEncodedCertificateAndKey
 	)
 
-	csr, identity, err = talosx509.NewCSRAndIdentity(p.dnsNames, p.ips)
+	csr, identity, err = talosx509.NewEd25519CSRAndIdentity(p.dnsNames, p.ips)
 	if err != nil {
 		return nil, cert, err
 	}

x509/x509.go

@@ -451,25 +451,34 @@ func NewCertificateFromCSRBytes(ca, key, csr []byte, setters ...Option) (crt *Ce
 }
 
 // NewKeyPair generates a certificate signed by the provided CA, and a private
-// key. The certifcate and private key are then used to create an
+// key. The certifcate and private key are then used to create a
 // tls.X509KeyPair.
-func NewKeyPair(ca *x509.Certificate, key interface{}, setters ...Option) (keypair *KeyPair, err error) {
-	csr, err := NewCertificateSigningRequest(key, setters...)
-	if err != nil {
-		return
-	}
+func NewKeyPair(ca *CertificateAuthority, setters ...Option) (keypair *KeyPair, err error) {
+	opts := NewDefaultOptions(setters...)
 
-	k, err := NewEd25519Key()
-	if err != nil {
-		return
+	var (
+		csr      *CertificateSigningRequest
+		identity *PEMEncodedCertificateAndKey
+	)
+
+	if opts.RSA {
+		csr, identity, err = NewRSACSRAndIdentity(opts.DNSNames, opts.IPAddresses)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create RSA CSR and identity: %w", err)
+		}
+	} else {
+		csr, identity, err = NewEd25519CSRAndIdentity(opts.DNSNames, opts.IPAddresses)
+		if err != nil {
+			return nil, fmt.Errorf("failed to create Ed25519 CSR and identity: %w", err)
+		}
 	}
 
-	crt, err := NewCertificateFromCSR(ca, key, csr.X509CertificateRequest, setters...)
+	crt, err := NewCertificateFromCSRBytes(ca.CrtPEM, ca.KeyPEM, csr.X509CertificateRequestPEM, setters...)
 	if err != nil {
-		return
+		return nil, fmt.Errorf("failed to create new certificate: %w", err)
 	}
 
-	x509KeyPair, err := tls.X509KeyPair(crt.X509CertificatePEM, k.PublicKeyPEM)
+	x509KeyPair, err := tls.X509KeyPair(crt.X509CertificatePEM, identity.Key)
 	if err != nil {
 		return
 	}
@@ -503,9 +512,9 @@ func NewCertificateAndKeyFromFiles(crt, key string) (p *PEMEncodedCertificateAnd
 	return p, nil
 }
 
-// NewCSRAndIdentity generates and PEM encoded certificate and key, along with a
+// NewEd25519CSRAndIdentity generates and PEM encoded certificate and key, along with a
 // CSR for the generated key.
-func NewCSRAndIdentity(dnsNames []string, ips []net.IP) (csr *CertificateSigningRequest, identity *PEMEncodedCertificateAndKey, err error) {
+func NewEd25519CSRAndIdentity(dnsNames []string, ips []net.IP) (csr *CertificateSigningRequest, identity *PEMEncodedCertificateAndKey, err error) {
 	var key *Ed25519Key
 
 	key, err = NewEd25519Key()
@@ -539,6 +548,43 @@ func NewCSRAndIdentity(dnsNames []string, ips []net.IP) (csr *CertificateSigning
 	return csr, identity, nil
 }
 
+// NewRSACSRAndIdentity generates and PEM encoded certificate and key, along with a
+// CSR for the generated key.
+func NewRSACSRAndIdentity(dnsNames []string, ips []net.IP) (csr *CertificateSigningRequest, identity *PEMEncodedCertificateAndKey, err error) {
+	var key *RSAKey
+
+	key, err = NewRSAKey()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	identity = &PEMEncodedCertificateAndKey{
+		Key: key.KeyPEM,
+	}
+
+	pemBlock, _ := pem.Decode(key.KeyPEM)
+	if pemBlock == nil {
+		return nil, nil, fmt.Errorf("failed to decode key")
+	}
+
+	priv, err := x509.ParsePKCS1PrivateKey(pemBlock.Bytes)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	opts := []Option{}
+	opts = append(opts, DNSNames(dnsNames))
+	opts = append(opts, IPAddresses(ips))
+	opts = append(opts, RSA(true))
+
+	csr, err = NewCertificateSigningRequest(priv, opts...)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return csr, identity, nil
+}
+
 // UnmarshalYAML implements the yaml.Unmarshaler interface for
 // PEMEncodedCertificateAndKey. It is expected that the Crt and Key are a base64
 // encoded string in the YAML file. This function decodes the strings into byte

x509/x509_test.go

@@ -2,13 +2,62 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+//nolint: scopelint
 package x509_test
 
-import "testing"
+import (
+	"testing"
 
-func TestEmpty(t *testing.T) {
-	// added for accurate coverage estimation
-	//
-	// please remove it once any unit-test is added
-	// for this package
+	"github.com/talos-systems/crypto/x509"
+)
+
+func TestNewKeyPair(t *testing.T) {
+	rsaCA, err := x509.NewSelfSignedCertificateAuthority(x509.RSA(true))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ed25519ca, err := x509.NewSelfSignedCertificateAuthority(x509.RSA(false))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	type args struct {
+		ca      *x509.CertificateAuthority
+		setters []x509.Option
+	}
+
+	tests := []struct {
+		name    string
+		args    args
+		wantErr bool
+	}{
+		{
+			name: "valid RSA",
+			args: args{
+				ca:      rsaCA,
+				setters: []x509.Option{x509.RSA(true)},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid Ed25519",
+			args: args{
+				ca:      ed25519ca,
+				setters: []x509.Option{x509.RSA(false)},
+			},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			_, err := x509.NewKeyPair(tt.args.ca, tt.args.setters...)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("NewKeyPair() error = %v, wantErr %v", err, tt.wantErr)
+
+				return
+			}
+		})
+	}
 }