From 20580cda52c0dfee8a8316b570143edcdaef397e Mon Sep 17 00:00:00 2001 From: Jan Dittberner Date: Sat, 23 Apr 2022 19:37:42 +0200 Subject: [PATCH] Use standard library types for certificates and revocations --- x509/openssl/repository.go | 89 +++++++++-------------- x509/openssl/repository_test.go | 15 +--- x509/revoking/repository.go | 7 +- x509/revoking/revoking.go | 125 ++++++++++++++++++++++++++------ x509/revoking/revoking_test.go | 46 ++++++++++-- x509/signing/repository.go | 4 +- x509/signing/signer.go | 54 ++++---------- x509/signing/signing.go | 9 +-- x509/signing/signing_test.go | 29 +++----- 9 files changed, 214 insertions(+), 164 deletions(-) diff --git a/x509/openssl/repository.go b/x509/openssl/repository.go index b06b418..85a1273 100644 --- a/x509/openssl/repository.go +++ b/x509/openssl/repository.go @@ -13,58 +13,10 @@ import ( "strings" "sync" "time" + + "git.cacert.org/cacert-gosigner/x509/revoking" ) -var OidCRLReason = asn1.ObjectIdentifier{2, 5, 29, 21} - -type CRLReason int - -// CRL reason codes as defined in RFC 5280 section 5.3.1 -const ( - CRLReasonUnspecified CRLReason = 0 - CRLReasonKeyCompromise CRLReason = 1 - CRLReasonCACompromise CRLReason = 2 - CRLReasonAffiliationChanged CRLReason = 3 - CRLReasonSuperseded CRLReason = 4 - CRLReasonCessationOfOperation CRLReason = 5 - CRLReasonCertificateHold CRLReason = 6 - CRLReasonRemoveFromCRL CRLReason = 8 - CRLReasonPrivilegeWithdrawn CRLReason = 9 - CRLReasonAACompromise CRLReason = 10 -) - -var crlReasonNames = map[CRLReason]string{ - CRLReasonUnspecified: "unspecified", - CRLReasonKeyCompromise: "keyCompromise", - CRLReasonCACompromise: "CACompromise", - CRLReasonAffiliationChanged: "affiliationChanged", - CRLReasonSuperseded: "superseded", - CRLReasonCessationOfOperation: "cessationOfOperation", - CRLReasonCertificateHold: "certificateHold", - CRLReasonRemoveFromCRL: "removeFromCRL", - CRLReasonPrivilegeWithdrawn: "privilegeWithdrawn", - CRLReasonAACompromise: "AACompromise", -} - -func (r CRLReason) String() string { - if reason, ok := crlReasonNames[r]; ok { - return reason - } - - return crlReasonNames[CRLReasonUnspecified] -} - -// ParseReason takes a reason string and performs a case-insensitive match to a reason code -func ParseReason(rs string) CRLReason { - for key, name := range crlReasonNames { - if strings.EqualFold(name, rs) { - return key - } - } - - return CRLReasonUnspecified -} - const TimeSpec = "060102030405Z" type indexStatus string @@ -81,7 +33,7 @@ type indexEntry struct { statusFlag indexStatus expiresAt time.Time revokedAt *time.Time - revocationReason CRLReason + revocationReason revoking.CRLReason serialNumber *big.Int fileName string certificateSubjectDN string @@ -119,7 +71,7 @@ type Repository struct { entries []indexEntry } -func (ie *indexEntry) markRevoked(revocationTime time.Time, reason CRLReason) { +func (ie *indexEntry) markRevoked(revocationTime time.Time, reason revoking.CRLReason) { if ie.statusFlag == certificateValid { ie.statusFlag = certificateRevoked ie.revokedAt = &revocationTime @@ -168,10 +120,10 @@ func (r *Repository) StoreRevocation(revoked *pkix.RevokedCertificate) error { return CannotRevokeUnknown{Serial: revoked.SerialNumber} } - reason := CRLReasonUnspecified + reason := revoking.CRLReasonUnspecified for _, ext := range revoked.Extensions { - if ext.Id.Equal(OidCRLReason) { + if ext.Id.Equal(revoking.OidCRLReason) { _, err := asn1.Unmarshal(ext.Value, &reason) if err != nil { return fmt.Errorf("could not unmarshal ") @@ -225,6 +177,31 @@ func (r *Repository) StoreCertificate(signed *x509.Certificate) error { return nil } +func (r *Repository) RevokedCertificates() ([]pkix.RevokedCertificate, error) { + var err error + + r.lock.Lock() + defer r.lock.Unlock() + + err = r.loadIndex() + if err != nil { + return nil, err + } + + result := make([]pkix.RevokedCertificate, 0) + for _, entry := range r.entries { + if entry.revokedAt != nil { + result = append(result, pkix.RevokedCertificate{ + SerialNumber: entry.serialNumber, + RevocationTime: *entry.revokedAt, + Extensions: []pkix.Extension{entry.revocationReason.BuildExtension()}, + }) + } + } + + return result, nil +} + func (r *Repository) loadIndex() error { entries := make([]indexEntry, 0, 100) @@ -320,14 +297,14 @@ func (r *Repository) newIndexEntryFromLine(text string) (*indexEntry, error) { } var revocationTimeParsed time.Time - var revocationReason CRLReason + var revocationReason revoking.CRLReason if fields[2] != "" { var timeString string if strings.Contains(fields[2], ",") { parts := strings.SplitN(fields[2], ",", 2) timeString = parts[0] - revocationReason = ParseReason(parts[1]) + revocationReason = revoking.ParseReason(parts[1]) } else { timeString = fields[2] } diff --git a/x509/openssl/repository_test.go b/x509/openssl/repository_test.go index 48fa186..c8269e4 100644 --- a/x509/openssl/repository_test.go +++ b/x509/openssl/repository_test.go @@ -4,7 +4,6 @@ import ( "crypto/rand" "crypto/x509" "crypto/x509/pkix" - "encoding/asn1" "math/big" "os" "path" @@ -13,6 +12,7 @@ import ( "time" "git.cacert.org/cacert-gosigner/x509/openssl" + "git.cacert.org/cacert-gosigner/x509/revoking" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -28,19 +28,12 @@ func TestStoreRevocation(t *testing.T) { t.Errorf("could not create random serial: %v", err) } - extBytes, err := asn1.Marshal(openssl.CRLReasonKeyCompromise) - if err != nil { - t.Errorf("could not marshal revocation reason: %v", err) - } - notAfter := time.Now().UTC().Add(24 * time.Hour).UTC() err = fr.StoreRevocation(&pkix.RevokedCertificate{ SerialNumber: serial, RevocationTime: notAfter, - Extensions: []pkix.Extension{ - {Id: openssl.OidCRLReason, Value: extBytes}, - }, + Extensions: []pkix.Extension{revoking.CRLReasonKeyCompromise.BuildExtension()}, }) assert.ErrorIs(t, err, openssl.CannotRevokeUnknown{Serial: serial}) @@ -63,9 +56,7 @@ func TestStoreRevocation(t *testing.T) { err = fr.StoreRevocation(&pkix.RevokedCertificate{ SerialNumber: serial, RevocationTime: time.Now(), - Extensions: []pkix.Extension{ - {Id: openssl.OidCRLReason, Value: extBytes}, - }, + Extensions: []pkix.Extension{revoking.CRLReasonKeyCompromise.BuildExtension()}, }) assert.NoError(t, err) diff --git a/x509/revoking/repository.go b/x509/revoking/repository.go index 3aa593e..637b676 100644 --- a/x509/revoking/repository.go +++ b/x509/revoking/repository.go @@ -1,7 +1,12 @@ package revoking +import ( + "crypto/x509/pkix" +) + // A Repository for storing certificate status information type Repository interface { // StoreRevocation stores information about a revoked certificate. - StoreRevocation(*CertificateRevoked) error + StoreRevocation(*pkix.RevokedCertificate) error + RevokedCertificates() ([]pkix.RevokedCertificate, error) } diff --git a/x509/revoking/revoking.go b/x509/revoking/revoking.go index b3b61c5..de3a394 100644 --- a/x509/revoking/revoking.go +++ b/x509/revoking/revoking.go @@ -1,17 +1,88 @@ package revoking import ( + "crypto" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "fmt" + "log" "math/big" + "strings" "time" ) +var OidCRLReason = asn1.ObjectIdentifier{2, 5, 29, 21} + +type CRLReason int + +// CRL reason codes as defined in RFC 5280 section 5.3.1 +const ( + CRLReasonUnspecified CRLReason = 0 + CRLReasonKeyCompromise CRLReason = 1 + CRLReasonCACompromise CRLReason = 2 + CRLReasonAffiliationChanged CRLReason = 3 + CRLReasonSuperseded CRLReason = 4 + CRLReasonCessationOfOperation CRLReason = 5 + CRLReasonCertificateHold CRLReason = 6 + CRLReasonRemoveFromCRL CRLReason = 8 + CRLReasonPrivilegeWithdrawn CRLReason = 9 + CRLReasonAACompromise CRLReason = 10 +) + +var crlReasonNames = map[CRLReason]string{ + CRLReasonUnspecified: "unspecified", + CRLReasonKeyCompromise: "keyCompromise", + CRLReasonCACompromise: "CACompromise", + CRLReasonAffiliationChanged: "affiliationChanged", + CRLReasonSuperseded: "superseded", + CRLReasonCessationOfOperation: "cessationOfOperation", + CRLReasonCertificateHold: "certificateHold", + CRLReasonRemoveFromCRL: "removeFromCRL", + CRLReasonPrivilegeWithdrawn: "privilegeWithdrawn", + CRLReasonAACompromise: "AACompromise", +} + +func (r CRLReason) String() string { + if reason, ok := crlReasonNames[r]; ok { + return reason + } + + return crlReasonNames[CRLReasonUnspecified] +} + +func (r CRLReason) BuildExtension() pkix.Extension { + extBytes, err := asn1.Marshal(r) + if err != nil { + // we can panic here because all values of CRLReason must be ASN.1 marshal-able + log.Panicf("could not marshal revocation reason: %v", err) + } + + return pkix.Extension{Id: OidCRLReason, Value: extBytes} +} + +// ParseReason takes a reason string and performs a case-insensitive match to a reason code +func ParseReason(rs string) CRLReason { + for key, name := range crlReasonNames { + if strings.EqualFold(name, rs) { + return key + } + } + + return CRLReasonUnspecified +} + type X509Revoking struct { - repository Repository + repository Repository + crlAlgorithm x509.SignatureAlgorithm + crlIssuer *x509.Certificate + signer crypto.Signer } type RevokeCertificate struct { serialNumber *big.Int - reason string + reason CRLReason } type CertificateRevoked struct { @@ -20,13 +91,15 @@ type CertificateRevoked struct { reason string } -type CRLInformation struct{} +type CRLInformation struct { + CRL []byte // DER encoded CRL +} -func (r *X509Revoking) Revoke(revokeCertificate *RevokeCertificate) (*CertificateRevoked, error) { - revoked := &CertificateRevoked{ - serialNumber: revokeCertificate.serialNumber, - revocationTime: time.Now(), - reason: revokeCertificate.reason, +func (r *X509Revoking) Revoke(revokeCertificate *RevokeCertificate) (*pkix.RevokedCertificate, error) { + revoked := &pkix.RevokedCertificate{ + SerialNumber: revokeCertificate.serialNumber, + RevocationTime: time.Now(), + Extensions: []pkix.Extension{revokeCertificate.reason.BuildExtension()}, } if err := r.repository.StoreRevocation(revoked); err != nil { @@ -37,21 +110,27 @@ func (r *X509Revoking) Revoke(revokeCertificate *RevokeCertificate) (*Certificat } func (r *X509Revoking) CreateCRL() (*CRLInformation, error) { - return &CRLInformation{}, nil + revoked, err := r.repository.RevokedCertificates() + if err != nil { + return nil, fmt.Errorf("could not get revocation information: %w", err) + } + + list, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{ + SignatureAlgorithm: r.crlAlgorithm, + RevokedCertificates: revoked, + }, r.crlIssuer, r.signer) + if err != nil { + return nil, fmt.Errorf("could not sign revocation list: %w", err) + } + + return &CRLInformation{CRL: list}, nil } -func (r *CertificateRevoked) SerialNumber() *big.Int { - return r.serialNumber -} - -func (r *CertificateRevoked) RevocationTime() time.Time { - return r.revocationTime -} - -func (r *CertificateRevoked) Reason() string { - return r.reason -} - -func NewX509Revoking(repo Repository) *X509Revoking { - return &X509Revoking{repository: repo} +func NewX509Revoking( + repo Repository, + crlAlgorithm x509.SignatureAlgorithm, + issuer *x509.Certificate, + signer crypto.Signer, +) *X509Revoking { + return &X509Revoking{repository: repo, crlAlgorithm: crlAlgorithm, crlIssuer: issuer, signer: signer} } diff --git a/x509/revoking/revoking_test.go b/x509/revoking/revoking_test.go index ea8159f..4937239 100644 --- a/x509/revoking/revoking_test.go +++ b/x509/revoking/revoking_test.go @@ -2,8 +2,13 @@ package revoking import ( "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" "math/big" + rand2 "math/rand" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -12,25 +17,54 @@ type testRepo struct { revoked []big.Int } -func (t *testRepo) StoreRevocation(revoked *CertificateRevoked) error { - t.revoked = append(t.revoked, *revoked.serialNumber) +func (t *testRepo) RevokedCertificates() ([]pkix.RevokedCertificate, error) { + result := make([]pkix.RevokedCertificate, len(t.revoked)) + + for i, s := range t.revoked { + result[i] = pkix.RevokedCertificate{ + SerialNumber: &s, + RevocationTime: time.Now(), + } + } + + return result, nil +} + +func (t *testRepo) StoreRevocation(revoked *pkix.RevokedCertificate) error { + t.revoked = append(t.revoked, *revoked.SerialNumber) return nil } func TestRevoking(t *testing.T) { testRepository := testRepo{revoked: make([]big.Int, 0)} - r := NewX509Revoking(&testRepository) + + caKey, err := rsa.GenerateKey(rand.Reader, 3072) + if err != nil { + t.Fatalf("could not generate key pair: %v", err) + } + caTemplate := &x509.Certificate{Subject: pkix.Name{CommonName: "Test CA"}, SerialNumber: big.NewInt(rand2.Int63())} + + certificateBytes, err := x509.CreateCertificate(rand.Reader, caTemplate, caTemplate, caKey.Public(), caKey) + if err != nil { + t.Fatalf("could not self-sign CA certificate: %v", err) + } + caCertificate, err := x509.ParseCertificate(certificateBytes) + if err != nil { + t.Fatalf("could not create test CA certificate: %v", err) + } + + r := NewX509Revoking(&testRepository, x509.ECDSAWithSHA256, caCertificate, caKey) serial, err := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128)) if err != nil { t.Errorf("could not create random serial: %v", err) } - revoke, err := r.Revoke(&RevokeCertificate{serialNumber: serial, reason: "for testing"}) + revoke, err := r.Revoke(&RevokeCertificate{serialNumber: serial, reason: CRLReasonKeyCompromise}) assert.NoError(t, err) - assert.Equal(t, "for testing", revoke.reason) - assert.Equal(t, serial, revoke.serialNumber) + assert.Equal(t, CRLReasonKeyCompromise.BuildExtension(), revoke.Extensions[0]) + assert.Equal(t, serial, revoke.SerialNumber) assert.Contains(t, testRepository.revoked, *serial) } diff --git a/x509/signing/repository.go b/x509/signing/repository.go index 37cf7f3..c294f97 100644 --- a/x509/signing/repository.go +++ b/x509/signing/repository.go @@ -1,5 +1,7 @@ package signing +import "crypto/x509" + type Repository interface { - StoreCertificate(*CertificateSigned) error + StoreCertificate(certificate *x509.Certificate) error } diff --git a/x509/signing/signer.go b/x509/signing/signer.go index 20f8676..0adfe94 100644 --- a/x509/signing/signer.go +++ b/x509/signing/signer.go @@ -7,36 +7,12 @@ import ( ) type SignerRequest struct { - csr *x509.CertificateRequest - subjectDN pkix.Name - emails []string - dnsNames []string - duration time.Duration - signatureAlgorithm x509.SignatureAlgorithm -} - -func (s *SignerRequest) SignatureAlgorithm() x509.SignatureAlgorithm { - return s.signatureAlgorithm -} - -func (s *SignerRequest) Duration() time.Duration { - return s.duration -} - -func (s *SignerRequest) DnsNames() []string { - return s.dnsNames -} - -func (s *SignerRequest) Emails() []string { - return s.emails -} - -func (s *SignerRequest) Csr() *x509.CertificateRequest { - return s.csr -} - -func (s *SignerRequest) SubjectDN() pkix.Name { - return s.subjectDN + CSR *x509.CertificateRequest + SubjectDN pkix.Name + Emails []string + DnsNames []string + Duration time.Duration + SignatureAlgorithm x509.SignatureAlgorithm } func NewSignerRequest( @@ -47,19 +23,19 @@ func NewSignerRequest( signatureAlgorithm x509.SignatureAlgorithm, ) *SignerRequest { return &SignerRequest{ - csr: csr, - subjectDN: subjectDN, - emails: emails, - dnsNames: dnsNames, - duration: duration, - signatureAlgorithm: signatureAlgorithm, + CSR: csr, + SubjectDN: subjectDN, + Emails: emails, + DnsNames: dnsNames, + Duration: duration, + SignatureAlgorithm: signatureAlgorithm, } } -type SignerResponse interface { - Certificate() *x509.Certificate +type SignerResponse struct { + Certificate *x509.Certificate } type Signer interface { - SignCertificate(*SignerRequest) (SignerResponse, error) + SignCertificate(*SignerRequest) (*SignerResponse, error) } diff --git a/x509/signing/signing.go b/x509/signing/signing.go index 0b08883..3ca385d 100644 --- a/x509/signing/signing.go +++ b/x509/signing/signing.go @@ -73,15 +73,10 @@ func (x *X509Signing) Sign(signingRequest *RequestSignature) (*CertificateSigned return nil, err } - result := NewCertificateSigned(certificateFromSigner) - err = x.repo.StoreCertificate(result) + err = x.repo.StoreCertificate(certificateFromSigner.Certificate) if err != nil { return nil, err } - return result, nil -} - -func NewCertificateSigned(signed SignerResponse) *CertificateSigned { - return &CertificateSigned{certificate: signed.Certificate()} + return &CertificateSigned{certificate: certificateFromSigner.Certificate}, nil } diff --git a/x509/signing/signing_test.go b/x509/signing/signing_test.go index 3a9fee4..394a8d7 100644 --- a/x509/signing/signing_test.go +++ b/x509/signing/signing_test.go @@ -19,9 +19,8 @@ type testRepo struct { certs map[string]x509.Certificate } -func (r *testRepo) StoreCertificate(c *signing.CertificateSigned) error { - cert := c.Certificate() - r.certs[cert.SerialNumber.Text(16)] = *cert +func (r *testRepo) StoreCertificate(certificate *x509.Certificate) error { + r.certs[certificate.SerialNumber.Text(16)] = *certificate return nil } @@ -30,32 +29,24 @@ type testSigner struct { certificate *x509.Certificate } -type testSignerResponse struct { - certificate *x509.Certificate +func newTestSignerResponse(certificate *x509.Certificate) *signing.SignerResponse { + return &signing.SignerResponse{Certificate: certificate} } -func (t testSignerResponse) Certificate() *x509.Certificate { - return t.certificate -} - -func newTestSignerResponse(certificate *x509.Certificate) *testSignerResponse { - return &testSignerResponse{certificate: certificate} -} - -func (s *testSigner) SignCertificate(request *signing.SignerRequest) (signing.SignerResponse, error) { +func (s *testSigner) SignCertificate(request *signing.SignerRequest) (*signing.SignerResponse, error) { startDate := time.Now().Add(-1 * time.Minute) template := &x509.Certificate{ - Subject: request.SubjectDN(), + Subject: request.SubjectDN, SerialNumber: big.NewInt(rand2.Int63()), - EmailAddresses: request.Emails(), + EmailAddresses: request.Emails, NotBefore: startDate, - NotAfter: startDate.Add(request.Duration()), + NotAfter: startDate.Add(request.Duration), KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageDataEncipherment, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageEmailProtection}, - SignatureAlgorithm: request.SignatureAlgorithm(), + SignatureAlgorithm: request.SignatureAlgorithm, } - certBytes, err := x509.CreateCertificate(rand.Reader, template, s.certificate, request.Csr().PublicKey, s.key) + certBytes, err := x509.CreateCertificate(rand.Reader, template, s.certificate, request.CSR.PublicKey, s.key) if err != nil { return nil, err }