Switch Webhook to ed25519 and expiration of the certificate to seven …

…days (#1998)

* Switch to ed25519 and change the timeout of the certificate to seven days.

* Update comments to reflect the new grace period.

* Use oneWeek constant.

webhook/certificates/certificates.go

@@ -36,7 +36,7 @@ import (
 
 const (
 	// Time used for updating a certificate before it expires.
-	oneWeek = 7 * 24 * time.Hour
+	oneDay = 24 * time.Hour
 )
 
 type reconciler struct {
@@ -89,7 +89,7 @@ func (r *reconciler) reconcileCertificate(ctx context.Context) error {
 			certData, err := x509.ParseCertificate(cert.Certificate[0])
 			if err != nil {
 				logger.Errorw("Error parsing certificate", zap.Error(err))
-			} else if time.Now().Add(oneWeek).Before(certData.NotAfter) {
+			} else if time.Now().Add(oneDay).Before(certData.NotAfter) {
 				return nil
 			}
 		}

webhook/certificates/certificates_test.go

@@ -126,16 +126,16 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "certificate expiring soon",
 		Key:  key,
-		// 6 days falls inside of the grace period of 7 days so the secret will be updated
-		Objects: []runtime.Object{secretWithCertData(t, time.Now().Add(6*24*time.Hour))},
+		// 23 hours  falls inside of the grace period of 1 day so the secret will be updated.
+		Objects: []runtime.Object{secretWithCertData(t, time.Now().Add(23*time.Hour))},
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: secret,
 		}},
 	}, {
 		Name: "certificate not expiring soon",
 		Key:  key,
-		// 8 days falls outside of the grace period of 7 days so the secret will not be updated
-		Objects: []runtime.Object{secretWithCertData(t, time.Now().Add(8*24*time.Hour))},
+		// 25 hours falls outside of the grace period of 1 day so the secret will not be updated.
+		Objects: []runtime.Object{secretWithCertData(t, time.Now().Add(25*time.Hour))},
 	}}
 
 	table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {

webhook/certificates/resources/certs.go

@@ -18,8 +18,8 @@ package resources
 
 import (
 	"context"
+	"crypto/ed25519"
 	"crypto/rand"
-	"crypto/rsa"
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/pem"
@@ -62,7 +62,7 @@ func createCertTemplate(name, namespace string, notAfter time.Time) (*x509.Certi
 			Organization: []string{organization},
 			CommonName:   commonName,
 		},
-		SignatureAlgorithm:    x509.SHA256WithRSA,
+		SignatureAlgorithm:    x509.PureEd25519,
 		NotBefore:             time.Now(),
 		NotAfter:              notAfter,
 		BasicConstraintsValid: true,
@@ -112,9 +112,9 @@ func createCert(template, parent *x509.Certificate, pub, parentPriv interface{})
 	return
 }
 
-func createCA(ctx context.Context, name, namespace string, notAfter time.Time) (*rsa.PrivateKey, *x509.Certificate, []byte, error) {
+func createCA(ctx context.Context, name, namespace string, notAfter time.Time) (ed25519.PrivateKey, *x509.Certificate, []byte, error) {
 	logger := logging.FromContext(ctx)
-	rootKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	publicKey, privateKey, err := ed25519.GenerateKey(rand.Reader)
 	if err != nil {
 		logger.Errorw("error generating random key", zap.Error(err))
 		return nil, nil, nil, err
@@ -126,12 +126,12 @@ func createCA(ctx context.Context, name, namespace string, notAfter time.Time) (
 		return nil, nil, nil, err
 	}
 
-	rootCert, rootCertPEM, err := createCert(rootCertTmpl, rootCertTmpl, &rootKey.PublicKey, rootKey)
+	rootCert, rootCertPEM, err := createCert(rootCertTmpl, rootCertTmpl, publicKey, privateKey)
 	if err != nil {
 		logger.Errorw("error signing the CA cert", zap.Error(err))
 		return nil, nil, nil, err
 	}
-	return rootKey, rootCert, rootCertPEM, nil
+	return privateKey, rootCert, rootCertPEM, nil
 }
 
 // CreateCerts creates and returns a CA certificate and certificate and
@@ -148,7 +148,7 @@ func CreateCerts(ctx context.Context, name, namespace string, notAfter time.Time
 	}
 
 	// Then create the private key for the serving cert
-	servKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	publicKey, privateKey, err := ed25519.GenerateKey(rand.Reader)
 	if err != nil {
 		logger.Errorw("error generating random key", zap.Error(err))
 		return nil, nil, nil, err
@@ -160,13 +160,18 @@ func CreateCerts(ctx context.Context, name, namespace string, notAfter time.Time
 	}
 
 	// create a certificate which wraps the server's public key, sign it with the CA private key
-	_, servCertPEM, err := createCert(servCertTemplate, caCertificate, &servKey.PublicKey, caKey)
+	_, servCertPEM, err := createCert(servCertTemplate, caCertificate, publicKey, caKey)
 	if err != nil {
 		logger.Errorw("error signing server certificate template", zap.Error(err))
 		return nil, nil, nil, err
 	}
+	privKeyBytes, err := x509.MarshalPKCS8PrivateKey(privateKey)
+	if err != nil {
+		logger.Errorw("error marshaling private key", zap.Error(err))
+		return nil, nil, nil, err
+	}
 	servKeyPEM := pem.EncodeToMemory(&pem.Block{
-		Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(servKey),
+		Type: "PRIVATE KEY", Bytes: privKeyBytes,
 	})
 	return servKeyPEM, servCertPEM, caCertificatePEM, nil
 }

webhook/certificates/resources/certs_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package resources
 
 import (
+	"crypto/ed25519"
 	"crypto/x509"
 	"encoding/pem"
 	"fmt"
@@ -29,22 +30,22 @@ import (
 )
 
 func TestCreateCerts(t *testing.T) {
-	sKey, serverCertPEM, caCertBytes, err := CreateCerts(TestContextWithLogger(t), "got-the-hook", "knative-webhook", time.Now().AddDate(1, 0, 0))
+	sKey, serverCertPEM, caCertBytes, err := CreateCerts(TestContextWithLogger(t), "got-the-hook", "knative-webhook", time.Now().AddDate(0, 0, 7))
 	if err != nil {
 		t.Fatal("Failed to create certs", err)
 	}
 
 	// Test server private key
 	p, _ := pem.Decode(sKey)
-	if p.Type != "RSA PRIVATE KEY" {
-		t.Fatal("Expected the key to be RSA Private key type")
+	if p.Type != "PRIVATE KEY" {
+		t.Fatal("Expected the key to be Private key type")
 	}
-	key, err := x509.ParsePKCS1PrivateKey(p.Bytes)
+	key, err := x509.ParsePKCS8PrivateKey(p.Bytes)
 	if err != nil {
 		t.Fatal("Failed to parse private key", err)
 	}
-	if err := key.Validate(); err != nil {
-		t.Fatalf("Failed to validate private key")
+	if _, ok := key.(ed25519.PrivateKey); !ok {
+		t.Fatalf("Key is not ed25519 format, actually %t", key)
 	}
 
 	// Test Server Cert
@@ -98,7 +99,7 @@ func validCertificate(cert []byte, t *testing.T) (*x509.Certificate, error) {
 	if err != nil {
 		return nil, fmt.Errorf("Failed to parse cert %w", err)
 	}
-	if parsedCert.SignatureAlgorithm != x509.SHA256WithRSA {
+	if parsedCert.SignatureAlgorithm != x509.PureEd25519 {
 		return nil, fmt.Errorf("Failed to match signature. Got: %s, want: %s", parsedCert.SignatureAlgorithm, x509.SHA256WithRSA)
 	}
 	return parsedCert, nil

webhook/certificates/resources/secret.go

@@ -32,6 +32,8 @@ const (
 	// CACert is the name of the key associated with the certificate of the CA for
 	// the keypair.
 	CACert = "ca-cert.pem"
+
+	oneWeek = 7 * 24 * time.Hour
 )
 
 // MakeSecret synthesizes a Kubernetes Secret object with the keys specified by
@@ -41,7 +43,7 @@ var MakeSecret = MakeSecretInternal
 
 // MakeSecretInternal is only public so MakeSecret can be restored in testing.  Use MakeSecret.
 func MakeSecretInternal(ctx context.Context, name, namespace, serviceName string) (*corev1.Secret, error) {
-	serverKey, serverCert, caCert, err := CreateCerts(ctx, serviceName, namespace, time.Now().AddDate(1, 0, 0))
+	serverKey, serverCert, caCert, err := CreateCerts(ctx, serviceName, namespace, time.Now().Add(oneWeek))
 	if err != nil {
 		return nil, err
 	}