fixup! Extend signers to set CRL Distribution Points

pkg/security/generateCertificate/config.go

@@ -9,11 +9,12 @@ import (
 	"encoding/asn1"
 	"fmt"
 	"net"
-	"net/url"
 	"slices"
 	"strconv"
 	"strings"
 	"time"
+
+	pkgX509 "github.com/plgd-dev/device/v2/pkg/security/x509"
 )
 
 type (
@@ -308,13 +309,10 @@ func (cfg Configuration) ToIPAddresses() ([]net.IP, error) {
 }
 
 func (cfg Configuration) ToCRLDistributionPoints() ([]string, error) {
-	cdp := make([]string, 0, len(cfg.CRLDistributionPoints))
-	for _, crl := range cfg.CRLDistributionPoints {
-		if _, err := url.ParseRequestURI(crl); err != nil {
-			return nil, fmt.Errorf("invalid CRL distribution point URL %q: %w", crl, err)
-		}
-		cdp = append(cdp, crl)
+	if err := pkgX509.ValidateCRLDistributionPoints(cfg.CRLDistributionPoints); err != nil {
+		return nil, err
 	}
+	cdp := slices.Clone(cfg.CRLDistributionPoints)
 	slices.Sort(cdp)
 	return slices.Compact(cdp), nil
 }

pkg/security/signer/basicCertificateSigner.go

@@ -5,7 +5,6 @@ import (
 	"crypto"
 	"crypto/rand"
 	"crypto/x509"
-	"encoding/pem"
 	"errors"
 	"math/big"
 	"time"
@@ -32,17 +31,7 @@ func NewBasicCertificateSigner(caCert []*x509.Certificate, caKey crypto.PrivateK
 }
 
 func (s *BasicCertificateSigner) Sign(_ context.Context, csr []byte) ([]byte, error) {
-	csrBlock, _ := pem.Decode(csr)
-	if csrBlock == nil {
-		return nil, errors.New("pem not found")
-	}
-
-	certificateRequest, err := x509.ParseCertificateRequest(csrBlock.Bytes)
-	if err != nil {
-		return nil, err
-	}
-
-	err = certificateRequest.CheckSignature()
+	certificateRequest, err := pkgX509.ParseAndCheckCertificateRequest(csr)
 	if err != nil {
 		return nil, err
 	}
@@ -54,7 +43,9 @@ func (s *BasicCertificateSigner) Sign(_ context.Context, csr []byte) ([]byte, er
 	if err != nil {
 		return nil, err
 	}
-
+	if err = pkgX509.ValidateCRLDistributionPoints(s.crlDistributionPoints); err != nil {
+		return nil, err
+	}
 	template := x509.Certificate{
 		SerialNumber:          serialNumber,
 		NotBefore:             notBefore,

pkg/security/signer/basicCertificateSigner_test.go

@@ -2,6 +2,7 @@ package signer_test
 
 import (
 	"context"
+	"crypto"
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
@@ -33,21 +34,41 @@ func TestBasicCertificateSignerSign(t *testing.T) {
 	csr, err := generateCertificate.GenerateCSR(csrCfg, priv)
 	require.NoError(t, err)
 
+	type fields struct {
+		caCert         []*x509.Certificate
+		caKey          crypto.PrivateKey
+		validNotBefore time.Time
+		validNotAfter  time.Time
+		crlPoints      []string
+	}
 	type args struct {
 		csr []byte
 	}
 	tests := []struct {
 		name    string
+		fields  fields
 		args    args
 		want    []*x509.Certificate
 		wantErr bool
 	}{
 		{
-			name:    "invalid",
+			name: "invalid",
+			fields: fields{
+				caCert:         caCert,
+				caKey:          caKey,
+				validNotBefore: time.Now(),
+				validNotAfter:  time.Now().Add(time.Hour * 86400),
+			},
 			wantErr: true,
 		},
 		{
 			name: "valid",
+			fields: fields{
+				caCert:         caCert,
+				caKey:          caKey,
+				validNotBefore: time.Now(),
+				validNotAfter:  time.Now().Add(time.Hour * 86400),
+			},
 			args: args{
 				csr: csr,
 			},
@@ -70,14 +91,42 @@ func TestBasicCertificateSignerSign(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "valid with CRL points",
+			fields: fields{
+				caCert:         caCert,
+				caKey:          caKey,
+				validNotBefore: time.Now().Add(-time.Second),
+				validNotAfter:  time.Now().Add(time.Hour),
+				crlPoints:      []string{"http://example.com/crl"},
+			},
+			args: args{
+				csr: csr,
+			},
+			want: []*x509.Certificate{
+				{
+					Subject: pkix.Name{
+						CommonName: "uuid:00000000-0000-0000-0000-000000000001",
+					},
+					ExtKeyUsage: []x509.ExtKeyUsage{
+						x509.ExtKeyUsageServerAuth,
+						x509.ExtKeyUsageClientAuth,
+					},
+					UnknownExtKeyUsage:    []asn1.ObjectIdentifier{coap.ExtendedKeyUsage_IDENTITY_CERTIFICATE},
+					CRLDistributionPoints: []string{"http://example.com/crl"},
+				},
+				{
+					Subject: pkix.Name{
+						CommonName: "intermediateCA",
+					},
+				},
+			},
+		},
 	}
-
-	s := signer.NewBasicCertificateSigner(caCert, caKey, time.Now(), time.Now().Add(time.Hour*86400), nil)
-
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			s := signer.NewBasicCertificateSigner(tt.fields.caCert, tt.fields.caKey, tt.fields.validNotBefore, tt.fields.validNotAfter, tt.fields.crlPoints)
 			got, err := s.Sign(context.Background(), tt.args.csr)
-
 			if tt.wantErr {
 				require.Error(t, err)
 				return
@@ -91,6 +140,7 @@ func TestBasicCertificateSignerSign(t *testing.T) {
 				require.Equal(t, tt.want[i].Subject.CommonName, cert.Subject.CommonName)
 				require.Equal(t, tt.want[i].ExtKeyUsage, cert.ExtKeyUsage)
 				require.Equal(t, tt.want[i].UnknownExtKeyUsage, cert.UnknownExtKeyUsage)
+				require.Equal(t, tt.want[i].CRLDistributionPoints, cert.CRLDistributionPoints)
 				got = rest
 			}
 		})

pkg/security/signer/identityCertificateSigner.go

@@ -22,7 +22,6 @@ import (
 	"crypto/rand"
 	"crypto/x509"
 	"encoding/asn1"
-	"encoding/pem"
 	"errors"
 	"fmt"
 	"math/big"
@@ -72,17 +71,7 @@ func (s *OCFIdentityCertificate) Sign(_ context.Context, csr []byte) ([]byte, er
 		return nil, fmt.Errorf("expired: current time %v is out of time range: %v <-> %v", now, notBefore.Format(time.RFC3339), notAfter.Format(time.RFC3339))
 	}
 
-	csrBlock, _ := pem.Decode(csr)
-	if csrBlock == nil {
-		return nil, errors.New("pem not found")
-	}
-
-	certificateRequest, err := x509.ParseCertificateRequest(csrBlock.Bytes)
-	if err != nil {
-		return nil, err
-	}
-
-	err = certificateRequest.CheckSignature()
+	certificateRequest, err := pkgX509.ParseAndCheckCertificateRequest(csr)
 	if err != nil {
 		return nil, err
 	}
@@ -92,7 +81,9 @@ func (s *OCFIdentityCertificate) Sign(_ context.Context, csr []byte) ([]byte, er
 	if err != nil {
 		return nil, err
 	}
-
+	if err = pkgX509.ValidateCRLDistributionPoints(s.crlDistributionPoints); err != nil {
+		return nil, err
+	}
 	template := x509.Certificate{
 		SerialNumber:          serialNumber,
 		NotBefore:             notBefore,

pkg/security/signer/identityCertificateSigner_test.go

@@ -24,6 +24,7 @@ func TestOCFIdentityCertificateSign(t *testing.T) {
 		caKey          crypto.PrivateKey
 		validNotBefore time.Time
 		validNotAfter  time.Time
+		crlPoints      []string
 	}
 	type args struct {
 		csr []byte
@@ -107,10 +108,37 @@ func TestOCFIdentityCertificateSign(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "valid with CRL points",
+			fields: fields{
+				caCert:         caCert,
+				caKey:          caKey,
+				validNotBefore: time.Now().Add(-time.Second),
+				validNotAfter:  time.Now().Add(time.Hour),
+				crlPoints:      []string{"http://example.com/crl"},
+			},
+			args: args{
+				csr: csr,
+			},
+		},
+		{
+			name: "invalid CRL point format",
+			fields: fields{
+				caCert:         caCert,
+				caKey:          caKey,
+				validNotBefore: time.Now().Add(-time.Second),
+				validNotAfter:  time.Now().Add(time.Hour),
+				crlPoints:      []string{"invalid-url"},
+			},
+			args: args{
+				csr: csr,
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			s := signer.NewOCFIdentityCertificate(tt.fields.caCert, tt.fields.caKey, tt.fields.validNotBefore, tt.fields.validNotAfter, nil)
+			s := signer.NewOCFIdentityCertificate(tt.fields.caCert, tt.fields.caKey, tt.fields.validNotBefore, tt.fields.validNotAfter, tt.fields.crlPoints)
 			gotSignedCsr, err := s.Sign(context.Background(), tt.args.csr)
 			if tt.wantErr {
 				require.Error(t, err)

pkg/security/x509/parse.go

@@ -106,3 +106,20 @@ func ParseCertificates(cert *tls.Certificate) ([]*x509.Certificate, error) {
 	}
 	return caChain, nil
 }
+
+func ParseAndCheckCertificateRequest(csr []byte) (*x509.CertificateRequest, error) {
+	csrBlock, _ := pem.Decode(csr)
+	if csrBlock == nil {
+		return nil, errors.New("pem not found")
+	}
+
+	certificateRequest, err := x509.ParseCertificateRequest(csrBlock.Bytes)
+	if err != nil {
+		return nil, err
+	}
+	err = certificateRequest.CheckSignature()
+	if err != nil {
+		return nil, err
+	}
+	return certificateRequest, nil
+}

pkg/security/x509/validate.go

@@ -0,0 +1,15 @@
+package x509
+
+import (
+	"fmt"
+	"net/url"
+)
+
+func ValidateCRLDistributionPoints(crlDistributionPoints []string) error {
+	for _, crl := range crlDistributionPoints {
+		if _, err := url.ParseRequestURI(crl); err != nil {
+			return fmt.Errorf("invalid CRL distribution point URL %q: %w", crl, err)
+		}
+	}
+	return nil
+}