From fcd987ea1ebef186ee84eb1910f0fa0c586bd152 Mon Sep 17 00:00:00 2001 From: Daniel Adam Date: Tue, 22 Oct 2024 20:36:44 +0200 Subject: [PATCH] Extend signers to set CRL Distribution Points --- Makefile | 1 + client/deviceOwnershipSDK.go | 21 +- cmd/bridge-device/Dockerfile | 4 +- cmd/ocfclient/main.go | 4 +- pkg/security/generateCertificate/config.go | 19 +- .../generateCertificate/config_test.go | 57 +++++ .../generateCertificate.go | 23 +- .../generateCertificate_test.go | 236 +++++++++++++++++- .../generateIdentityCertificate.go | 20 +- .../generateIdentityCertificate_test.go | 25 +- .../generateIntermediateCA.go | 12 +- .../generateIntermediateCA_test.go | 29 +-- .../generateCertificate/generateRootCA.go | 2 +- .../generateRootCA_test.go | 25 +- pkg/security/signer/basicCertificateSigner.go | 72 ++++++ .../signer/basicCertificateSigner_test.go | 148 +++++++++++ ...signer.go => identityCertificateSigner.go} | 60 ++--- ...t.go => identityCertificateSigner_test.go} | 31 ++- pkg/security/x509/parse.go | 17 ++ pkg/security/x509/parse_test.go | 69 +++++ pkg/security/x509/validate.go | 18 ++ pkg/security/x509/validate_test.go | 50 ++++ test/signer.go | 2 +- test/test.go | 4 +- 24 files changed, 831 insertions(+), 118 deletions(-) create mode 100644 pkg/security/signer/basicCertificateSigner.go create mode 100644 pkg/security/signer/basicCertificateSigner_test.go rename pkg/security/signer/{signer.go => identityCertificateSigner.go} (63%) rename pkg/security/signer/{signer_test.go => identityCertificateSigner_test.go} (77%) create mode 100644 pkg/security/x509/validate.go create mode 100644 pkg/security/x509/validate_test.go diff --git a/Makefile b/Makefile index f3f56018..a941ab04 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,7 @@ unit-test: certificates go test -race -parallel 1 -v ./bridge/... -coverpkg=./... -covermode=atomic -coverprofile=$(TMP_PATH)/bridge.unit.coverage.txt go test -race -v ./schema/... -covermode=atomic -coverprofile=$(TMP_PATH)/schema.unit.coverage.txt ROOT_CA_CRT="$(ROOT_CA_CRT)" ROOT_CA_KEY="$(ROOT_CA_KEY)" \ + INTERMEDIATE_CA_CRT="$(INTERMEDIATE_CA_CRT)" INTERMEDIATE_CA_KEY=$(INTERMEDIATE_CA_KEY) \ go test -race -v ./pkg/... -covermode=atomic -coverprofile=$(TMP_PATH)/pkg.unit.coverage.txt test: env build-testcontainer diff --git a/client/deviceOwnershipSDK.go b/client/deviceOwnershipSDK.go index 08d2a62c..4138a5f7 100644 --- a/client/deviceOwnershipSDK.go +++ b/client/deviceOwnershipSDK.go @@ -39,13 +39,14 @@ type Signer = interface { } type DeviceOwnershipSDKConfig struct { - ID string - Cert string - CertKey string - ValidFrom string // RFC3339, or now-1m, empty means now-1m - CertExpiry *string - - CreateSignerFunc func(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore time.Time, validNotAfter time.Time) core.CertificateSigner + ID string + Cert string + CertKey string + ValidFrom string // RFC3339, or now-1m, empty means now-1m + CertExpiry *string + CRLDistributionPoints []string + + CreateSignerFunc func(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore, validNotAfter time.Time, crlDistributionPoints []string) (core.CertificateSigner, error) } type deviceOwnershipSDK struct { @@ -78,11 +79,11 @@ func newDeviceOwnershipSDKFromConfig(app ApplicationCallback, dialTLS core.DialT return nil, fmt.Errorf("invalid ID for device ownership SDK: %w", err) } - return newDeviceOwnershipSDK(app, uid.String(), dialTLS, dialDLTS, &signerCert, cfg.ValidFrom, certExpiry, cfg.CreateSignerFunc) + return newDeviceOwnershipSDK(app, uid.String(), dialTLS, dialDLTS, &signerCert, cfg.ValidFrom, certExpiry, cfg.CRLDistributionPoints, cfg.CreateSignerFunc) } func newDeviceOwnershipSDK(app ApplicationCallback, sdkDeviceID string, dialTLS core.DialTLS, - dialDTLS core.DialDTLS, signerCert *tls.Certificate, validFrom string, certExpiry time.Duration, createSigner func(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore time.Time, validNotAfter time.Time) core.CertificateSigner, + dialDTLS core.DialDTLS, signerCert *tls.Certificate, validFrom string, certExpiry time.Duration, crlDistributionPoints []string, createSigner func(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore, validNotAfter time.Time, crlDistributionPoints []string) (core.CertificateSigner, error), ) (*deviceOwnershipSDK, error) { if validFrom == "" { validFrom = "now-1m" @@ -107,7 +108,7 @@ func newDeviceOwnershipSDK(app ApplicationCallback, sdkDeviceID string, dialTLS return nil, fmt.Errorf("invalid validFrom(%v): %w", validFrom, err) } notAfter := notBefore.Add(certExpiry) - return createSigner(signerCAs, signerCert.PrivateKey, notBefore, notAfter), nil + return createSigner(signerCAs, signerCert.PrivateKey, notBefore, notAfter, crlDistributionPoints) }, app: app, dialTLS: dialTLS, diff --git a/cmd/bridge-device/Dockerfile b/cmd/bridge-device/Dockerfile index 75fe0106..b89950de 100644 --- a/cmd/bridge-device/Dockerfile +++ b/cmd/bridge-device/Dockerfile @@ -11,8 +11,8 @@ WORKDIR $GOPATH/src/github.com/plgd-dev/device RUN CGO_ENABLED=0 go build -o /go/bin/bridge-device ./cmd/bridge-device FROM alpine:3.20 AS security-provider -RUN apk add -U --no-cache ca-certificates -RUN addgroup -S nonroot \ +RUN apk add -U --no-cache ca-certificates \ + && addgroup -S nonroot \ && adduser -S nonroot -G nonroot FROM scratch AS service diff --git a/cmd/ocfclient/main.go b/cmd/ocfclient/main.go index 4a93fb30..bd8bd045 100644 --- a/cmd/ocfclient/main.go +++ b/cmd/ocfclient/main.go @@ -523,8 +523,8 @@ func NewSecureClient() (*local.Client, error) { ID: CertIdentity, Cert: string(IdentityIntermediateCA), CertKey: string(IdentityIntermediateCAKey), - CreateSignerFunc: func(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore time.Time, validNotAfter time.Time) core.CertificateSigner { - return signer.NewOCFIdentityCertificate(caCert, caKey, validNotBefore, validNotAfter) + CreateSignerFunc: func(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore, validNotAfter time.Time, crlDistributionPoints []string) (core.CertificateSigner, error) { + return signer.NewOCFIdentityCertificate(caCert, caKey, validNotBefore, validNotAfter, crlDistributionPoints) }, }, } diff --git a/pkg/security/generateCertificate/config.go b/pkg/security/generateCertificate/config.go index 1aacdf29..42cedd01 100644 --- a/pkg/security/generateCertificate/config.go +++ b/pkg/security/generateCertificate/config.go @@ -9,9 +9,12 @@ import ( "encoding/asn1" "fmt" "net" + "slices" "strconv" "strings" "time" + + pkgX509 "github.com/plgd-dev/device/v2/pkg/security/x509" ) type ( @@ -56,9 +59,10 @@ type Configuration struct { //nolint:staticcheck KeyUsages []string `yaml:"keyUsages" long:"ku" default:"digitalSignature" default:"keyAgreement" description:"to set more values repeat option with parameter"` //nolint:staticcheck - ExtensionKeyUsages []string `yaml:"extensionKeyUsages" long:"eku" default:"client" default:"server" description:"to set more values repeat option with parameter"` - EllipticCurve EllipticCurve `yaml:"ellipticCurve" long:"ellipticCurve" default:"P256" description:"supported values:P256, P384, P521"` - SignatureAlgorithm SignatureAlgorithm `yaml:"signatureAlgorithm" long:"signatureAlgorithm" default:"ECDSA-SHA256" description:"supported values:ECDSA-SHA256, ECDSA-SHA384, ECDSA-SHA512"` + ExtensionKeyUsages []string `yaml:"extensionKeyUsages" long:"eku" default:"client" default:"server" description:"to set more values repeat option with parameter"` + EllipticCurve EllipticCurve `yaml:"ellipticCurve" long:"ellipticCurve" default:"P256" description:"supported values:P256, P384, P521"` + SignatureAlgorithm SignatureAlgorithm `yaml:"signatureAlgorithm" long:"signatureAlgorithm" default:"ECDSA-SHA256" description:"supported values:ECDSA-SHA256, ECDSA-SHA384, ECDSA-SHA512"` + CRLDistributionPoints []string `yaml:"crlDistributionPoints" long:"crl" description:"to set more values repeat option with parameter"` } func (cfg Configuration) ToPkixName() pkix.Name { @@ -303,3 +307,12 @@ func (cfg Configuration) ToIPAddresses() ([]net.IP, error) { } return ips, nil } + +func (cfg Configuration) ToCRLDistributionPoints() ([]string, error) { + if err := pkgX509.ValidateCRLDistributionPoints(cfg.CRLDistributionPoints); err != nil { + return nil, err + } + cdp := slices.Clone(cfg.CRLDistributionPoints) + slices.Sort(cdp) + return slices.Compact(cdp), nil +} diff --git a/pkg/security/generateCertificate/config_test.go b/pkg/security/generateCertificate/config_test.go index 6502fae3..0bd8101c 100644 --- a/pkg/security/generateCertificate/config_test.go +++ b/pkg/security/generateCertificate/config_test.go @@ -112,3 +112,60 @@ func TestToIPAddresses(t *testing.T) { expected := []net.IP{net.ParseIP("192.168.0.1"), net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334")} require.Equal(t, expected, ips) } + +func TestToCRLDistributionPoints(t *testing.T) { + tests := []struct { + name string + cfg generateCertificate.Configuration + want []string + wantErr bool + }{ + { + name: "Valid CRL URLs", + cfg: generateCertificate.Configuration{ + CRLDistributionPoints: []string{ + "http://example.com/crl1", + "http://example.com/crl2", + }, + }, + want: []string{"http://example.com/crl1", "http://example.com/crl2"}, + }, + { + name: "Duplicate CRL URLs", + cfg: generateCertificate.Configuration{ + CRLDistributionPoints: []string{ + "http://example.com/crl1", + "http://example.com/crl1", // duplicate + }, + }, + want: []string{"http://example.com/crl1"}, + }, + { + name: "Invalid CRL URL", + cfg: generateCertificate.Configuration{ + CRLDistributionPoints: []string{ + "invalid-url", + }, + }, + wantErr: true, + }, + { + name: "Empty CRL list", + cfg: generateCertificate.Configuration{ + CRLDistributionPoints: []string{}, + }, + want: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + crls, err := tt.cfg.ToCRLDistributionPoints() + if tt.wantErr { + require.Error(t, err) + return + } + require.ElementsMatch(t, tt.want, crls) + }) + } +} diff --git a/pkg/security/generateCertificate/generateCertificate.go b/pkg/security/generateCertificate/generateCertificate.go index bc4d5474..652a5a2d 100644 --- a/pkg/security/generateCertificate/generateCertificate.go +++ b/pkg/security/generateCertificate/generateCertificate.go @@ -9,7 +9,13 @@ import ( "encoding/asn1" "encoding/pem" - ocfSigner "github.com/plgd-dev/kit/v2/security/signer" + ocfSigner "github.com/plgd-dev/device/v2/pkg/security/signer" +) + +var ( + ASN1KeyUsage = asn1.ObjectIdentifier{2, 5, 29, 15} + ASN1BasicConstraints = asn1.ObjectIdentifier{2, 5, 29, 19} + ASN1ExtKeyUsage = asn1.ObjectIdentifier{2, 5, 29, 37} ) // GenerateCSR creates CSR according to configuration. @@ -28,12 +34,12 @@ func GenerateCSR(cfg Configuration, privateKey *ecdsa.PrivateKey) ([]byte, error extraExtensions := make([]pkix.Extension, 0, 3) if !cfg.BasicConstraints.Ignore { - bcVal, errM := asn1.Marshal(basicConstraints{false}) + bcVal, errM := asn1.Marshal(BasicConstraints{false}) if errM != nil { return nil, errM } extraExtensions = append(extraExtensions, pkix.Extension{ - Id: asn1.ObjectIdentifier{2, 5, 29, 19}, // basic constraints + Id: ASN1BasicConstraints, Value: bcVal, Critical: false, }) @@ -49,7 +55,7 @@ func GenerateCSR(cfg Configuration, privateKey *ecdsa.PrivateKey) ([]byte, error return nil, errM } extraExtensions = append(extraExtensions, pkix.Extension{ - Id: asn1.ObjectIdentifier{2, 5, 29, 15}, // key usage + Id: ASN1KeyUsage, Value: val, Critical: false, }) @@ -65,7 +71,7 @@ func GenerateCSR(cfg Configuration, privateKey *ecdsa.PrivateKey) ([]byte, error return nil, errM } extraExtensions = append(extraExtensions, pkix.Extension{ - Id: asn1.ObjectIdentifier{2, 5, 29, 37}, // EKU + Id: ASN1ExtKeyUsage, Value: val, Critical: false, }) @@ -100,8 +106,11 @@ func GenerateCert(cfg Configuration, privateKey *ecdsa.PrivateKey, signerCA []*x if err != nil { return nil, err } - notAfter := notBefore.Add(cfg.ValidFor) - s := ocfSigner.NewBasicCertificateSigner(signerCA, signerCAKey, notBefore, notAfter) + crlDistributionPoints, err := cfg.ToCRLDistributionPoints() + if err != nil { + return nil, err + } + s := ocfSigner.NewBasicCertificateSigner(signerCA, signerCAKey, notBefore, notAfter, crlDistributionPoints) return s.Sign(context.Background(), csr) } diff --git a/pkg/security/generateCertificate/generateCertificate_test.go b/pkg/security/generateCertificate/generateCertificate_test.go index 00220282..e0eeae5d 100644 --- a/pkg/security/generateCertificate/generateCertificate_test.go +++ b/pkg/security/generateCertificate/generateCertificate_test.go @@ -1,15 +1,227 @@ -package generateCertificate +package generateCertificate_test import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "encoding/pem" + "slices" "testing" "time" + "github.com/plgd-dev/device/v2/pkg/security/generateCertificate" "github.com/stretchr/testify/require" ) +func TestGenerateCSR(t *testing.T) { + privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + validCfg := func() generateCertificate.Configuration { + cfg := generateCertificate.Configuration{} + cfg.Subject.Country = []string{"US"} + cfg.Subject.Organization = []string{"TestOrg"} + cfg.Subject.CommonName = "test.example.com" + cfg.SubjectAlternativeName.DNSNames = []string{"example.com"} + cfg.BasicConstraints.Ignore = true + cfg.BasicConstraints.MaxPathLen = -1 + cfg.ValidFor = time.Hour * 24 + return cfg + }() + + type args struct { + cfg generateCertificate.Configuration + } + tests := []struct { + name string + args args + verify func(t *testing.T, cfg generateCertificate.Configuration, parsedCSR *x509.CertificateRequest) + wantErr bool + }{ + { + name: "valid", + args: args{ + cfg: validCfg, + }, + verify: func(t *testing.T, cfg generateCertificate.Configuration, parsedCSR *x509.CertificateRequest) { + require.Equal(t, cfg.Subject.CommonName, parsedCSR.Subject.CommonName) + require.ElementsMatch(t, cfg.Subject.Country, parsedCSR.Subject.Country) + require.ElementsMatch(t, cfg.Subject.Organization, parsedCSR.Subject.Organization) + require.ElementsMatch(t, cfg.SubjectAlternativeName.DNSNames, parsedCSR.DNSNames) + }, + }, + { + name: "valid with basic constraint", + args: args{ + cfg: func() generateCertificate.Configuration { + cfg := validCfg + cfg.BasicConstraints.Ignore = false + cfg.BasicConstraints.MaxPathLen = 42 + return cfg + }(), + }, + verify: func(t *testing.T, cfg generateCertificate.Configuration, parsedCSR *x509.CertificateRequest) { + require.Equal(t, cfg.Subject.CommonName, parsedCSR.Subject.CommonName) + require.ElementsMatch(t, cfg.Subject.Country, parsedCSR.Subject.Country) + require.ElementsMatch(t, cfg.Subject.Organization, parsedCSR.Subject.Organization) + require.ElementsMatch(t, cfg.SubjectAlternativeName.DNSNames, parsedCSR.DNSNames) + var bcExt *pkix.Extension + for _, ext := range parsedCSR.Extensions { + if slices.Equal(ext.Id, generateCertificate.ASN1BasicConstraints) { + bcExt = &ext + } + } + require.NotNil(t, bcExt) + require.False(t, bcExt.Critical) + var bc generateCertificate.BasicConstraints + _, err := asn1.Unmarshal(bcExt.Value, &bc) + require.NoError(t, err) + require.False(t, bc.CA) + }, + }, + { + name: "valid with key usages", + args: args{ + cfg: func() generateCertificate.Configuration { + cfg := validCfg + cfg.KeyUsages = append(cfg.KeyUsages, "digitalSignature", "certSign") + return cfg + }(), + }, + verify: func(t *testing.T, cfg generateCertificate.Configuration, parsedCSR *x509.CertificateRequest) { + require.Equal(t, cfg.Subject.CommonName, parsedCSR.Subject.CommonName) + require.ElementsMatch(t, cfg.Subject.Country, parsedCSR.Subject.Country) + require.ElementsMatch(t, cfg.Subject.Organization, parsedCSR.Subject.Organization) + require.ElementsMatch(t, cfg.SubjectAlternativeName.DNSNames, parsedCSR.DNSNames) + var kuExt *pkix.Extension + for _, ext := range parsedCSR.Extensions { + if slices.Equal(ext.Id, generateCertificate.ASN1KeyUsage) { + kuExt = &ext + } + } + require.NotNil(t, kuExt) + require.False(t, kuExt.Critical) + var ku asn1.BitString + _, err := asn1.Unmarshal(kuExt.Value, &ku) + require.NoError(t, err) + keyUsages, err := cfg.AsnKeyUsages() + require.NoError(t, err) + require.Equal(t, keyUsages, ku) + }, + }, + { + name: "valid with extended key usages", + args: args{ + cfg: func() generateCertificate.Configuration { + cfg := validCfg + cfg.ExtensionKeyUsages = append(cfg.ExtensionKeyUsages, + asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 3, 3}.String(), + asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 3, 9}.String()) + return cfg + }(), + }, + verify: func(t *testing.T, cfg generateCertificate.Configuration, parsedCSR *x509.CertificateRequest) { + require.Equal(t, cfg.Subject.CommonName, parsedCSR.Subject.CommonName) + require.ElementsMatch(t, cfg.Subject.Country, parsedCSR.Subject.Country) + require.ElementsMatch(t, cfg.Subject.Organization, parsedCSR.Subject.Organization) + require.ElementsMatch(t, cfg.SubjectAlternativeName.DNSNames, parsedCSR.DNSNames) + var ekuExt *pkix.Extension + for _, ext := range parsedCSR.Extensions { + if slices.Equal(ext.Id, generateCertificate.ASN1ExtKeyUsage) { + ekuExt = &ext + } + } + require.NotNil(t, ekuExt) + require.False(t, ekuExt.Critical) + var ekus []asn1.ObjectIdentifier + _, err := asn1.Unmarshal(ekuExt.Value, &ekus) + require.NoError(t, err) + x509ExtKeyUsages, unknownKeyUsages, err := cfg.X509ExtKeyUsages() + require.NoError(t, err) + var extKeyUsages []asn1.ObjectIdentifier + for _, e := range x509ExtKeyUsages { + eku, ok := generateCertificate.OidFromExtKeyUsage(e) + require.True(t, ok) + extKeyUsages = append(extKeyUsages, eku) + } + extKeyUsages = append(extKeyUsages, unknownKeyUsages...) + require.ElementsMatch(t, extKeyUsages, ekus) + }, + }, + { + name: "invalid IP address", + args: args{ + cfg: func() generateCertificate.Configuration { + cfg := validCfg + cfg.SubjectAlternativeName.IPAddresses = []string{"invalid-ip"} + return cfg + }(), + }, + wantErr: true, + }, + { + name: "invalid signature algorithm", + args: args{ + cfg: func() generateCertificate.Configuration { + cfg := validCfg + cfg.SignatureAlgorithm = "invalid-algorithm" + return cfg + }(), + }, + wantErr: true, + }, + { + name: "invalid key usages", + args: args{ + cfg: func() generateCertificate.Configuration { + cfg := validCfg + cfg.KeyUsages = []string{"invalid-usage"} + return cfg + }(), + }, + wantErr: true, + }, + { + name: "invalid extended key usages", + args: args{ + cfg: func() generateCertificate.Configuration { + cfg := validCfg + cfg.ExtensionKeyUsages = []string{"invalid-eku"} + return cfg + }(), + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + csr, err := generateCertificate.GenerateCSR(tt.args.cfg, privateKey) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.NotNil(t, csr) + + // Parse the PEM-encoded CSR and verify it's well-formed + block, _ := pem.Decode(csr) + require.NotNil(t, block) + require.Equal(t, "CERTIFICATE REQUEST", block.Type) + + parsedCSR, err := x509.ParseCertificateRequest(block.Bytes) + require.NoError(t, err) + tt.verify(t, tt.args.cfg, parsedCSR) + }) + } +} + func TestGenerateCertificate(t *testing.T) { type args struct { - cfg Configuration + cfg generateCertificate.Configuration } tests := []struct { name string @@ -19,7 +231,7 @@ func TestGenerateCertificate(t *testing.T) { { name: "valid - default", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, }, }, @@ -27,36 +239,36 @@ func TestGenerateCertificate(t *testing.T) { { name: "valid - sha384", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA384, + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA384, }, }, }, { name: "valid - sha512", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA512, + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA512, }, }, }, { name: "valid - p384", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - EllipticCurve: EllipticCurveP384, + EllipticCurve: generateCertificate.EllipticCurveP384, }, }, }, { name: "valid - p521", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - EllipticCurve: EllipticCurveP521, + EllipticCurve: generateCertificate.EllipticCurveP521, }, }, }, @@ -66,7 +278,7 @@ func TestGenerateCertificate(t *testing.T) { caCrt, caKey := generateRootCA(t, tt.args.cfg) privateKey, err := tt.args.cfg.GenerateKey() require.NoError(t, err) - got, err := GenerateCert(tt.args.cfg, privateKey, caCrt, caKey) + got, err := generateCertificate.GenerateCert(tt.args.cfg, privateKey, caCrt, caKey) if tt.wantErr { require.Error(t, err) return diff --git a/pkg/security/generateCertificate/generateIdentityCertificate.go b/pkg/security/generateCertificate/generateIdentityCertificate.go index 5afd55c2..80f5fc0a 100644 --- a/pkg/security/generateCertificate/generateIdentityCertificate.go +++ b/pkg/security/generateCertificate/generateIdentityCertificate.go @@ -13,7 +13,7 @@ import ( ocfSigner "github.com/plgd-dev/device/v2/pkg/security/signer" ) -type basicConstraints struct { +type BasicConstraints struct { CA bool } @@ -27,7 +27,7 @@ func NewIdentityCSRTemplate(deviceID string) (*x509.CertificateRequest, error) { return nil, err } - bcVal, err := asn1.Marshal(basicConstraints{false}) + bcVal, err := asn1.Marshal(BasicConstraints{false}) if err != nil { return nil, err } @@ -41,17 +41,17 @@ func NewIdentityCSRTemplate(deviceID string) (*x509.CertificateRequest, error) { Subject: subj, ExtraExtensions: []pkix.Extension{ { - Id: asn1.ObjectIdentifier{2, 5, 29, 19}, // basic constraints + Id: ASN1BasicConstraints, Value: bcVal, Critical: false, }, { - Id: asn1.ObjectIdentifier{2, 5, 29, 15}, // key usage + Id: ASN1KeyUsage, Value: kuVal, Critical: false, }, { - Id: asn1.ObjectIdentifier{2, 5, 29, 37}, // EKU + Id: ASN1ExtKeyUsage, Value: val, Critical: false, }, @@ -93,7 +93,13 @@ func GenerateIdentityCert(cfg Configuration, deviceID string, privateKey *ecdsa. return nil, err } notAfter := notBefore.Add(cfg.ValidFor) - - s := ocfSigner.NewOCFIdentityCertificate(signerCA, signerCAKey, notBefore, notAfter) + crlDistributionPoints, err := cfg.ToCRLDistributionPoints() + if err != nil { + return nil, err + } + s, err := ocfSigner.NewOCFIdentityCertificate(signerCA, signerCAKey, notBefore, notAfter, crlDistributionPoints) + if err != nil { + return nil, err + } return s.Sign(context.Background(), csr) } diff --git a/pkg/security/generateCertificate/generateIdentityCertificate_test.go b/pkg/security/generateCertificate/generateIdentityCertificate_test.go index 57bf4915..270a5d12 100644 --- a/pkg/security/generateCertificate/generateIdentityCertificate_test.go +++ b/pkg/security/generateCertificate/generateIdentityCertificate_test.go @@ -1,15 +1,16 @@ -package generateCertificate +package generateCertificate_test import ( "testing" "time" + "github.com/plgd-dev/device/v2/pkg/security/generateCertificate" "github.com/stretchr/testify/require" ) func TestGenerateIdentityCertificate(t *testing.T) { type args struct { - cfg Configuration + cfg generateCertificate.Configuration } tests := []struct { name string @@ -19,7 +20,7 @@ func TestGenerateIdentityCertificate(t *testing.T) { { name: "valid - default", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, }, }, @@ -27,36 +28,36 @@ func TestGenerateIdentityCertificate(t *testing.T) { { name: "valid - sha384", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA384, + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA384, }, }, }, { name: "valid - sha512", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA512, + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA512, }, }, }, { name: "valid - p384", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - EllipticCurve: EllipticCurveP384, + EllipticCurve: generateCertificate.EllipticCurveP384, }, }, }, { name: "valid - p521", args: args{ - cfg: Configuration{ + cfg: generateCertificate.Configuration{ ValidFor: time.Minute, - EllipticCurve: EllipticCurveP521, + EllipticCurve: generateCertificate.EllipticCurveP521, }, }, }, @@ -66,7 +67,7 @@ func TestGenerateIdentityCertificate(t *testing.T) { caCrt, caKey := generateRootCA(t, tt.args.cfg) privateKey, err := tt.args.cfg.GenerateKey() require.NoError(t, err) - got, err := GenerateIdentityCert(tt.args.cfg, "deviceID", privateKey, caCrt, caKey) + got, err := generateCertificate.GenerateIdentityCert(tt.args.cfg, "deviceID", privateKey, caCrt, caKey) if tt.wantErr { require.Error(t, err) return diff --git a/pkg/security/generateCertificate/generateIntermediateCA.go b/pkg/security/generateCertificate/generateIntermediateCA.go index 1b06d3f2..03f62943 100644 --- a/pkg/security/generateCertificate/generateIntermediateCA.go +++ b/pkg/security/generateCertificate/generateIntermediateCA.go @@ -10,7 +10,7 @@ import ( pkgX509 "github.com/plgd-dev/device/v2/pkg/security/x509" ) -func newCert(cfg Configuration) (*x509.Certificate, error) { +func newCert(cfg Configuration, isRootCA bool) (*x509.Certificate, error) { notBefore, err := cfg.ToValidFrom() if err != nil { return nil, err @@ -36,7 +36,13 @@ func newCert(cfg Configuration) (*x509.Certificate, error) { IsCA: true, SignatureAlgorithm: signatureAlgorithm, } - + if !isRootCA { + crlDistributionPoints, err := cfg.ToCRLDistributionPoints() + if err != nil { + return nil, err + } + template.CRLDistributionPoints = crlDistributionPoints + } if cfg.BasicConstraints.MaxPathLen >= 0 { if cfg.BasicConstraints.MaxPathLen == 0 { template.MaxPathLenZero = true @@ -48,7 +54,7 @@ func newCert(cfg Configuration) (*x509.Certificate, error) { } func GenerateIntermediateCA(cfg Configuration, privateKey *ecdsa.PrivateKey, signerCA []*x509.Certificate, signerCAKey *ecdsa.PrivateKey) ([]byte, error) { - cacert, err := newCert(cfg) + cacert, err := newCert(cfg, false) if err != nil { return nil, err } diff --git a/pkg/security/generateCertificate/generateIntermediateCA_test.go b/pkg/security/generateCertificate/generateIntermediateCA_test.go index 36fb5e4b..b63f730d 100644 --- a/pkg/security/generateCertificate/generateIntermediateCA_test.go +++ b/pkg/security/generateCertificate/generateIntermediateCA_test.go @@ -1,18 +1,19 @@ -package generateCertificate +package generateCertificate_test import ( "crypto/ecdsa" "crypto/x509" "testing" + "github.com/plgd-dev/device/v2/pkg/security/generateCertificate" pkgX509 "github.com/plgd-dev/device/v2/pkg/security/x509" "github.com/stretchr/testify/require" ) -func generateRootCA(t *testing.T, cfg Configuration) ([]*x509.Certificate, *ecdsa.PrivateKey) { +func generateRootCA(t *testing.T, cfg generateCertificate.Configuration) ([]*x509.Certificate, *ecdsa.PrivateKey) { privateKey, err := cfg.GenerateKey() require.NoError(t, err) - cert, err := GenerateRootCA(cfg, privateKey) + cert, err := generateCertificate.GenerateRootCA(cfg, privateKey) require.NoError(t, err) crt, err := pkgX509.ParsePemCertificates(cert) require.NoError(t, err) @@ -21,7 +22,7 @@ func generateRootCA(t *testing.T, cfg Configuration) ([]*x509.Certificate, *ecds func TestGenerateIntermediateCA(t *testing.T) { type args struct { - cfg Configuration + cfg generateCertificate.Configuration } tests := []struct { name string @@ -31,38 +32,38 @@ func TestGenerateIntermediateCA(t *testing.T) { { name: "valid - default", args: args{ - cfg: Configuration{}, + cfg: generateCertificate.Configuration{}, }, }, { name: "valid - sha384", args: args{ - cfg: Configuration{ - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA384, + cfg: generateCertificate.Configuration{ + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA384, }, }, }, { name: "valid - sha512", args: args{ - cfg: Configuration{ - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA512, + cfg: generateCertificate.Configuration{ + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA512, }, }, }, { name: "valid - p384", args: args{ - cfg: Configuration{ - EllipticCurve: EllipticCurveP384, + cfg: generateCertificate.Configuration{ + EllipticCurve: generateCertificate.EllipticCurveP384, }, }, }, { name: "valid - p521", args: args{ - cfg: Configuration{ - EllipticCurve: EllipticCurveP521, + cfg: generateCertificate.Configuration{ + EllipticCurve: generateCertificate.EllipticCurveP521, }, }, }, @@ -72,7 +73,7 @@ func TestGenerateIntermediateCA(t *testing.T) { caCrt, caKey := generateRootCA(t, tt.args.cfg) privateKey, err := tt.args.cfg.GenerateKey() require.NoError(t, err) - got, err := GenerateIntermediateCA(tt.args.cfg, privateKey, caCrt, caKey) + got, err := generateCertificate.GenerateIntermediateCA(tt.args.cfg, privateKey, caCrt, caKey) if tt.wantErr { require.Error(t, err) return diff --git a/pkg/security/generateCertificate/generateRootCA.go b/pkg/security/generateCertificate/generateRootCA.go index 416bfc25..c57d0ee2 100644 --- a/pkg/security/generateCertificate/generateRootCA.go +++ b/pkg/security/generateCertificate/generateRootCA.go @@ -8,7 +8,7 @@ import ( ) func GenerateRootCA(cfg Configuration, privateKey *ecdsa.PrivateKey) ([]byte, error) { - cacert, err := newCert(cfg) + cacert, err := newCert(cfg, true) if err != nil { return nil, err } diff --git a/pkg/security/generateCertificate/generateRootCA_test.go b/pkg/security/generateCertificate/generateRootCA_test.go index 0323a2ba..bf791cca 100644 --- a/pkg/security/generateCertificate/generateRootCA_test.go +++ b/pkg/security/generateCertificate/generateRootCA_test.go @@ -1,14 +1,15 @@ -package generateCertificate +package generateCertificate_test import ( "testing" + "github.com/plgd-dev/device/v2/pkg/security/generateCertificate" "github.com/stretchr/testify/require" ) func TestGenerateRootCA(t *testing.T) { type args struct { - cfg Configuration + cfg generateCertificate.Configuration } tests := []struct { name string @@ -18,38 +19,38 @@ func TestGenerateRootCA(t *testing.T) { { name: "valid - default", args: args{ - cfg: Configuration{}, + cfg: generateCertificate.Configuration{}, }, }, { name: "valid - sha384", args: args{ - cfg: Configuration{ - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA384, + cfg: generateCertificate.Configuration{ + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA384, }, }, }, { name: "valid - sha512", args: args{ - cfg: Configuration{ - SignatureAlgorithm: SignatureAlgorithmECDSAWithSHA512, + cfg: generateCertificate.Configuration{ + SignatureAlgorithm: generateCertificate.SignatureAlgorithmECDSAWithSHA512, }, }, }, { name: "valid - p384", args: args{ - cfg: Configuration{ - EllipticCurve: EllipticCurveP384, + cfg: generateCertificate.Configuration{ + EllipticCurve: generateCertificate.EllipticCurveP384, }, }, }, { name: "valid - p521", args: args{ - cfg: Configuration{ - EllipticCurve: EllipticCurveP521, + cfg: generateCertificate.Configuration{ + EllipticCurve: generateCertificate.EllipticCurveP521, }, }, }, @@ -58,7 +59,7 @@ func TestGenerateRootCA(t *testing.T) { t.Run(tt.name, func(t *testing.T) { privateKey, err := tt.args.cfg.GenerateKey() require.NoError(t, err) - got, err := GenerateRootCA(tt.args.cfg, privateKey) + got, err := generateCertificate.GenerateRootCA(tt.args.cfg, privateKey) if tt.wantErr { require.Error(t, err) return diff --git a/pkg/security/signer/basicCertificateSigner.go b/pkg/security/signer/basicCertificateSigner.go new file mode 100644 index 00000000..de1852ca --- /dev/null +++ b/pkg/security/signer/basicCertificateSigner.go @@ -0,0 +1,72 @@ +package signer + +import ( + "context" + "crypto" + "crypto/rand" + "crypto/x509" + "errors" + "math/big" + "time" + + pkgX509 "github.com/plgd-dev/device/v2/pkg/security/x509" +) + +type BasicCertificateSigner struct { + caCert []*x509.Certificate + caKey crypto.PrivateKey + validNotBefore time.Time + validNotAfter time.Time + crlDistributionPoints []string +} + +func NewBasicCertificateSigner(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore, validNotAfter time.Time, crlDistributionPoints []string) *BasicCertificateSigner { + return &BasicCertificateSigner{ + caCert: caCert, + caKey: caKey, + validNotBefore: validNotBefore, + validNotAfter: validNotAfter, + crlDistributionPoints: crlDistributionPoints, + } +} + +func (s *BasicCertificateSigner) Sign(_ context.Context, csr []byte) ([]byte, error) { + certificateRequest, err := pkgX509.ParseAndCheckCertificateRequest(csr) + if err != nil { + return nil, err + } + + notBefore := s.validNotBefore + notAfter := s.validNotAfter + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + return nil, err + } + if err = pkgX509.ValidateCRLDistributionPoints(s.crlDistributionPoints); err != nil { + return nil, err + } + template := x509.Certificate{ + SerialNumber: serialNumber, + NotBefore: notBefore, + NotAfter: notAfter, + Subject: certificateRequest.Subject, + PublicKeyAlgorithm: certificateRequest.PublicKeyAlgorithm, + PublicKey: certificateRequest.PublicKey, + SignatureAlgorithm: s.caCert[0].SignatureAlgorithm, + DNSNames: certificateRequest.DNSNames, + IPAddresses: certificateRequest.IPAddresses, + URIs: certificateRequest.URIs, + EmailAddresses: certificateRequest.EmailAddresses, + ExtraExtensions: certificateRequest.Extensions, + CRLDistributionPoints: s.crlDistributionPoints, + } + if len(s.caCert) == 0 { + return nil, errors.New("cannot sign with empty signer CA certificates") + } + signedCsr, err := x509.CreateCertificate(rand.Reader, &template, s.caCert[0], certificateRequest.PublicKey, s.caKey) + if err != nil { + return nil, err + } + return pkgX509.CreatePemChain(s.caCert, signedCsr) +} diff --git a/pkg/security/signer/basicCertificateSigner_test.go b/pkg/security/signer/basicCertificateSigner_test.go new file mode 100644 index 00000000..46844e3f --- /dev/null +++ b/pkg/security/signer/basicCertificateSigner_test.go @@ -0,0 +1,148 @@ +package signer_test + +import ( + "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "encoding/pem" + "os" + "testing" + "time" + + "github.com/plgd-dev/device/v2/pkg/net/coap" + "github.com/plgd-dev/device/v2/pkg/security/generateCertificate" + "github.com/plgd-dev/device/v2/pkg/security/signer" + pkgX509 "github.com/plgd-dev/device/v2/pkg/security/x509" + "github.com/stretchr/testify/require" +) + +func TestBasicCertificateSignerSign(t *testing.T) { + caCert, err := pkgX509.ReadPemCertificates(os.Getenv("INTERMEDIATE_CA_CRT")) + require.NoError(t, err) + caKey, err := pkgX509.ReadPemEcdsaPrivateKey(os.Getenv("INTERMEDIATE_CA_KEY")) + require.NoError(t, err) + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + csrCfg := generateCertificate.Configuration{} + csrCfg.Subject.CommonName = "uuid:00000000-0000-0000-0000-000000000001" + csrCfg.ExtensionKeyUsages = []string{"server", "client", coap.ExtendedKeyUsage_IDENTITY_CERTIFICATE.String()} + 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", + 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, + }, + wantErr: false, + 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}, + }, + { + Subject: pkix.Name{ + CommonName: "intermediateCA", + }, + }, + }, + }, + { + 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", + }, + }, + }, + }, + } + 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 + } + require.NoError(t, err) + for i := 0; i < len(tt.want); i++ { + block, rest := pem.Decode(got) + require.NotEmpty(t, block.Bytes) + cert, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err) + 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 + } + }) + } +} diff --git a/pkg/security/signer/signer.go b/pkg/security/signer/identityCertificateSigner.go similarity index 63% rename from pkg/security/signer/signer.go rename to pkg/security/signer/identityCertificateSigner.go index 2479b94f..6149e4d4 100644 --- a/pkg/security/signer/signer.go +++ b/pkg/security/signer/identityCertificateSigner.go @@ -22,7 +22,6 @@ import ( "crypto/rand" "crypto/x509" "encoding/asn1" - "encoding/pem" "errors" "fmt" "math/big" @@ -33,14 +32,24 @@ import ( ) type OCFIdentityCertificate struct { - caCert []*x509.Certificate - caKey crypto.PrivateKey - validNotBefore time.Time - validNotAfter time.Time + caCert []*x509.Certificate + caKey crypto.PrivateKey + validNotBefore time.Time + validNotAfter time.Time + crlDistributionPoints []string } -func NewOCFIdentityCertificate(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore time.Time, validNotAfter time.Time) *OCFIdentityCertificate { - return &OCFIdentityCertificate{caCert: caCert, caKey: caKey, validNotBefore: validNotBefore, validNotAfter: validNotAfter} +func NewOCFIdentityCertificate(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore, validNotAfter time.Time, crlDistributionPoints []string) (*OCFIdentityCertificate, error) { + if err := pkgX509.ValidateCRLDistributionPoints(crlDistributionPoints); err != nil { + return nil, err + } + return &OCFIdentityCertificate{ + caCert: caCert, + caKey: caKey, + validNotBefore: validNotBefore, + validNotAfter: validNotAfter, + crlDistributionPoints: crlDistributionPoints, + }, nil } func (s *OCFIdentityCertificate) Sign(_ context.Context, csr []byte) ([]byte, error) { @@ -65,17 +74,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 } @@ -85,18 +84,21 @@ 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, - NotAfter: notAfter, - Subject: certificateRequest.Subject, - PublicKeyAlgorithm: certificateRequest.PublicKeyAlgorithm, - PublicKey: certificateRequest.PublicKey, - SignatureAlgorithm: s.caCert[0].SignatureAlgorithm, - KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyAgreement, - UnknownExtKeyUsage: []asn1.ObjectIdentifier{coap.ExtendedKeyUsage_IDENTITY_CERTIFICATE}, - ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + SerialNumber: serialNumber, + NotBefore: notBefore, + NotAfter: notAfter, + Subject: certificateRequest.Subject, + PublicKeyAlgorithm: certificateRequest.PublicKeyAlgorithm, + PublicKey: certificateRequest.PublicKey, + SignatureAlgorithm: s.caCert[0].SignatureAlgorithm, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyAgreement, + UnknownExtKeyUsage: []asn1.ObjectIdentifier{coap.ExtendedKeyUsage_IDENTITY_CERTIFICATE}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth}, + CRLDistributionPoints: s.crlDistributionPoints, } if len(s.caCert) == 0 { return nil, errors.New("cannot sign with empty signer CA certificates") diff --git a/pkg/security/signer/signer_test.go b/pkg/security/signer/identityCertificateSigner_test.go similarity index 77% rename from pkg/security/signer/signer_test.go rename to pkg/security/signer/identityCertificateSigner_test.go index 2d6065b9..3b8d47da 100644 --- a/pkg/security/signer/signer_test.go +++ b/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,38 @@ 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) + s, err := signer.NewOCFIdentityCertificate(tt.fields.caCert, tt.fields.caKey, tt.fields.validNotBefore, tt.fields.validNotAfter, tt.fields.crlPoints) + require.NoError(t, err) gotSignedCsr, err := s.Sign(context.Background(), tt.args.csr) if tt.wantErr { require.Error(t, err) diff --git a/pkg/security/x509/parse.go b/pkg/security/x509/parse.go index 85d8c46b..b3dd952b 100644 --- a/pkg/security/x509/parse.go +++ b/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 +} diff --git a/pkg/security/x509/parse_test.go b/pkg/security/x509/parse_test.go index 0442610b..b45ed5b3 100644 --- a/pkg/security/x509/parse_test.go +++ b/pkg/security/x509/parse_test.go @@ -166,3 +166,72 @@ func TestParseCertificates_Fail(t *testing.T) { _, err := pkgX509.ParseCertificates(&tls.Certificate{}) require.Error(t, err) } + +func generateValidCSR(t *testing.T) []byte { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err, "failed to generate private key") + + template := &x509.CertificateRequest{ + SignatureAlgorithm: x509.ECDSAWithSHA256, + } + csrBytes, err := x509.CreateCertificateRequest(rand.Reader, template, priv) + require.NoError(t, err, "failed to create certificate request") + + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes}) +} + +func generateCSRWithInvalidSignature(t *testing.T) []byte { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err, "failed to generate private key") + + template := &x509.CertificateRequest{ + SignatureAlgorithm: x509.ECDSAWithSHA256, + } + csrBytes, err := x509.CreateCertificateRequest(rand.Reader, template, priv) + require.NoError(t, err, "failed to create certificate request") + + // Tamper with the CSR bytes to invalidate the signature + csrBytes[len(csrBytes)-1] ^= 0xFF + + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes}) +} + +func TestParseAndCheckCertificateRequest(t *testing.T) { + tests := []struct { + name string + csr []byte + wantErr bool + }{ + { + name: "Valid CSR", + csr: generateValidCSR(t), + }, + { + name: "Invalid PEM", + csr: []byte("invalid-pem-data"), + wantErr: true, + }, + { + name: "Invalid CSR", + csr: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: []byte("invalid-csr-data")}), + wantErr: true, + }, + { + name: "Invalid Signature", + csr: generateCSRWithInvalidSignature(t), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := pkgX509.ParseAndCheckCertificateRequest(tt.csr) + + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} diff --git a/pkg/security/x509/validate.go b/pkg/security/x509/validate.go new file mode 100644 index 00000000..3d25d983 --- /dev/null +++ b/pkg/security/x509/validate.go @@ -0,0 +1,18 @@ +package x509 + +import ( + "fmt" + "net/url" +) + +// ValidateCRLDistributionPoints validates a slice of CRL distribution point URLs. +// It ensures each URL is properly formatted and returns an error if any URL is invalid. +// Returns nil if all URLs are valid. +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 +} diff --git a/pkg/security/x509/validate_test.go b/pkg/security/x509/validate_test.go new file mode 100644 index 00000000..afd5b8d2 --- /dev/null +++ b/pkg/security/x509/validate_test.go @@ -0,0 +1,50 @@ +package x509_test + +import ( + "testing" + + "github.com/plgd-dev/device/v2/pkg/security/x509" + "github.com/stretchr/testify/require" +) + +func TestValidateCRLDistributionPoints(t *testing.T) { + tests := []struct { + name string + crlPoints []string + wantErr bool + }{ + { + name: "Valid CRL distribution points", + crlPoints: []string{"http://example.com/crl1", "https://example.com/crl2"}, + }, + { + name: "Invalid CRL distribution point", + crlPoints: []string{"http://valid-crl.com", "invalid-url"}, + wantErr: true, + }, + { + name: "Empty CRL distribution point", + crlPoints: []string{"http://example.com/crl1", ""}, + wantErr: true, + }, + { + name: "Valid - No CRL distribution points", + crlPoints: []string{}, + }, + { + name: "Valid - Nil CRL distribution points", + crlPoints: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := x509.ValidateCRLDistributionPoints(tt.crlPoints) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} diff --git a/test/signer.go b/test/signer.go index b42ac847..0f9f700e 100644 --- a/test/signer.go +++ b/test/signer.go @@ -26,5 +26,5 @@ func NewTestSigner() (core.CertificateSigner, error) { notBefore := time.Now() notAfter := notBefore.Add(time.Hour * 86400) - return NewIdentityCertificateSigner(identityIntermediateCA, identityIntermediateCAKey, notBefore, notAfter), nil + return NewIdentityCertificateSigner(identityIntermediateCA, identityIntermediateCAKey, notBefore, notAfter, nil) } diff --git a/test/test.go b/test/test.go index b8cc6033..2f3d66a3 100644 --- a/test/test.go +++ b/test/test.go @@ -122,8 +122,8 @@ func FindDeviceByName(ctx context.Context, name string) (deviceID string, _ erro return id, nil } -func NewIdentityCertificateSigner(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore time.Time, validNotAfter time.Time) core.CertificateSigner { - return signer.NewOCFIdentityCertificate(caCert, caKey, validNotBefore, validNotAfter) +func NewIdentityCertificateSigner(caCert []*x509.Certificate, caKey crypto.PrivateKey, validNotBefore, validNotAfter time.Time, crlDistributionPoints []string) (core.CertificateSigner, error) { + return signer.NewOCFIdentityCertificate(caCert, caKey, validNotBefore, validNotAfter, crlDistributionPoints) } type IPType int