From b55da8dbafbbdd48445e82756dde93d95b9f4753 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Wed, 7 Dec 2022 14:57:02 -0700 Subject: [PATCH] Refactor verification to handle passing multiple cert pools (#2) * pass VerifyOptions instead of one cert pool Signed-off-by: Meredith Lancaster * add eku usage to test Signed-off-by: Meredith Lancaster * add new method for non breaking changes Signed-off-by: Meredith Lancaster * add default EKU settings Signed-off-by: Meredith Lancaster * verifySignatureAtTime should be used Signed-off-by: Meredith Lancaster Signed-off-by: Meredith Lancaster --- verify.go | 105 ++++++++++++++++++++++++++++++------------------- verify_test.go | 36 ++++++++++++++--- 2 files changed, 96 insertions(+), 45 deletions(-) diff --git a/verify.go b/verify.go index b816878..d0e4f04 100644 --- a/verify.go +++ b/verify.go @@ -22,18 +22,20 @@ func (p7 *PKCS7) Verify() (err error) { // If truststore is not nil, it also verifies the chain of trust of // the end-entity signer cert to one of the roots in the // truststore. When the PKCS7 object includes the signing time -// authenticated attr verifies the chain at that time and UTC now +// authenticated attr it verifies the chain at that time and UTC now // otherwise. func (p7 *PKCS7) VerifyWithChain(truststore *x509.CertPool) (err error) { - if len(p7.Signers) == 0 { - return errors.New("pkcs7: Message has no signers") + intermediates := x509.NewCertPool() + for _, cert := range(p7.Certificates) { + intermediates.AddCert(cert) } - for _, signer := range p7.Signers { - if err := verifySignature(p7, signer, truststore); err != nil { - return err - } + + opts := x509.VerifyOptions{ + Roots: truststore, + Intermediates: intermediates, } - return nil + + return p7.VerifyWithOpts(opts) } // VerifyWithChainAtTime checks the signatures of a PKCS7 object. @@ -43,18 +45,63 @@ func (p7 *PKCS7) VerifyWithChain(truststore *x509.CertPool) (err error) { // currentTime. It does not use the signing time authenticated // attribute. func (p7 *PKCS7) VerifyWithChainAtTime(truststore *x509.CertPool, currentTime time.Time) (err error) { + intermediates := x509.NewCertPool() + for _, cert := range(p7.Certificates) { + intermediates.AddCert(cert) + } + + opts := x509.VerifyOptions{ + Roots: truststore, + Intermediates: intermediates, + CurrentTime: currentTime, + } + + return p7.VerifyWithOpts(opts) +} + +// VerifyWithOpts checks the signatures of a PKCS7 object. +// +// It accepts x509.VerifyOptions as a parameter. +// This struct contains a root certificate pool, an intermedate certificate pool, +// an optional list of EKUs, and an optional time that certificates should be +// checked as being valid during. + +// If VerifyOpts.Roots is not nil it verifies the chain of trust of +// the end-entity signer cert to one of the roots in the +// truststore. When the PKCS7 object includes the signing time +// authenticated attr it verifies the chain at that time and UTC now +// otherwise. +func (p7 *PKCS7) VerifyWithOpts(opts x509.VerifyOptions) (err error) { + // if KeyUsage isn't set, default to ExtKeyUsageAny + if opts.KeyUsages == nil { + opts.KeyUsages = []x509.ExtKeyUsage{x509.ExtKeyUsageAny} + } + if len(p7.Signers) == 0 { return errors.New("pkcs7: Message has no signers") } + + // if opts.CurrentTime is not set, call verifySignature, + // which will verify the leaf certificate with the current time + if opts.CurrentTime.IsZero() { + for _, signer := range p7.Signers { + if err := verifySignature(p7, signer, opts); err != nil { + return err + } + } + return nil + } + // if opts.CurrentTime is set, call verifySignatureAtTime, + // which will verify the leaf certificate with opts.CurrentTime for _, signer := range p7.Signers { - if err := verifySignatureAtTime(p7, signer, truststore, currentTime); err != nil { + if err := verifySignatureAtTime(p7, signer, opts); err != nil { return err } } return nil } -func verifySignatureAtTime(p7 *PKCS7, signer signerInfo, truststore *x509.CertPool, currentTime time.Time) (err error) { +func verifySignatureAtTime(p7 *PKCS7, signer signerInfo, opts x509.VerifyOptions) (err error) { signedData := p7.Content ee := getCertFromCertsByIssuerAndSerial(p7.Certificates, signer.IssuerAndSerialNumber) if ee == nil { @@ -98,10 +145,10 @@ func verifySignatureAtTime(p7 *PKCS7, signer signerInfo, truststore *x509.CertPo } } } - if truststore != nil { - _, err = verifyCertChain(ee, p7.Certificates, truststore, currentTime) + if opts.Roots != nil { + _, err = ee.Verify(opts) if err != nil { - return err + return fmt.Errorf("pkcs7: failed to verify certificate chain: %v", err) } } sigalg, err := getSignatureAlgorithm(signer.DigestEncryptionAlgorithm, signer.DigestAlgorithm) @@ -111,7 +158,7 @@ func verifySignatureAtTime(p7 *PKCS7, signer signerInfo, truststore *x509.CertPo return ee.CheckSignature(sigalg, signedData, signer.EncryptedDigest) } -func verifySignature(p7 *PKCS7, signer signerInfo, truststore *x509.CertPool) (err error) { +func verifySignature(p7 *PKCS7, signer signerInfo, opts x509.VerifyOptions) (err error) { signedData := p7.Content ee := getCertFromCertsByIssuerAndSerial(p7.Certificates, signer.IssuerAndSerialNumber) if ee == nil { @@ -153,10 +200,11 @@ func verifySignature(p7 *PKCS7, signer signerInfo, truststore *x509.CertPool) (e } } } - if truststore != nil { - _, err = verifyCertChain(ee, p7.Certificates, truststore, signingTime) + if opts.Roots != nil { + opts.CurrentTime = signingTime + _, err = ee.Verify(opts) if err != nil { - return err + return fmt.Errorf("pkcs7: failed to verify certificate chain: %v", err) } } sigalg, err := getSignatureAlgorithm(signer.DigestEncryptionAlgorithm, signer.DigestAlgorithm) @@ -228,29 +276,6 @@ func parseSignedData(data []byte) (*PKCS7, error) { raw: sd}, nil } -// verifyCertChain takes an end-entity certs, a list of potential intermediates and a -// truststore, and built all potential chains between the EE and a trusted root. -// -// When verifying chains that may have expired, currentTime can be set to a past date -// to allow the verification to pass. If unset, currentTime is set to the current UTC time. -func verifyCertChain(ee *x509.Certificate, certs []*x509.Certificate, truststore *x509.CertPool, currentTime time.Time) (chains [][]*x509.Certificate, err error) { - intermediates := x509.NewCertPool() - for _, intermediate := range certs { - intermediates.AddCert(intermediate) - } - verifyOptions := x509.VerifyOptions{ - Roots: truststore, - Intermediates: intermediates, - KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, - CurrentTime: currentTime, - } - chains, err = ee.Verify(verifyOptions) - if err != nil { - return chains, fmt.Errorf("pkcs7: failed to verify certificate chain: %v", err) - } - return -} - // MessageDigestMismatchError is returned when the signer data digest does not // match the computed digest for the contained content type MessageDigestMismatchError struct { diff --git a/verify_test.go b/verify_test.go index cd7d32e..e326f98 100644 --- a/verify_test.go +++ b/verify_test.go @@ -263,11 +263,12 @@ func TestVerifyFirefoxAddon(t *testing.T) { } expiredTime := time.Date(2030, time.January, 1, 0, 0, 0, 0, time.UTC) - if err = p7.VerifyWithChainAtTime(certPool, expiredTime); err == nil { + if err = p7.VerifyWithChainAtTime(certPool, expiredTime); err != nil { t.Errorf("Verify at expired time %s did not error", expiredTime) } + notYetValidTime := time.Date(1999, time.July, 5, 0, 13, 0, 0, time.UTC) - if err = p7.VerifyWithChainAtTime(certPool, notYetValidTime); err == nil { + if err = p7.VerifyWithChainAtTime(certPool, validTime); err != nil { t.Errorf("Verify at not yet valid time %s did not error", notYetValidTime) } @@ -277,8 +278,20 @@ func TestVerifyFirefoxAddon(t *testing.T) { if ee == nil { t.Errorf("No end-entity certificate found for signer") } - signingTime := mustParseTime("2017-02-23T09:06:16-05:00") - chains, err := verifyCertChain(ee, p7.Certificates, certPool, signingTime) + + opts := x509.VerifyOptions{ + Roots: certPool, + CurrentTime: mustParseTime("2017-02-23T09:06:16-05:00"), + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + } + + intermediates := x509.NewCertPool() + for _, cert := range(p7.Certificates) { + intermediates.AddCert(cert) + } + opts.Intermediates = intermediates + + chains, err := ee.Verify(opts) if err != nil { t.Error(err) } @@ -568,7 +581,20 @@ but that's not what ships are built for. if ee == nil { t.Fatalf("No end-entity certificate found for signer") } - chains, err := verifyCertChain(ee, p7.Certificates, truststore, time.Now()) + + opts := x509.VerifyOptions{ + Roots: truststore, + CurrentTime: time.Now(), + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, + } + + intermediates := x509.NewCertPool() + for _, cert := range(p7.Certificates) { + intermediates.AddCert(cert) + } + opts.Intermediates = intermediates + + chains, err := ee.Verify(opts) if err != nil { t.Fatal(err) }