From d9fabe7d99a457f4e9a7957d42717d7f5406c3dd Mon Sep 17 00:00:00 2001 From: Dallas Delaney Date: Thu, 15 Jan 2026 10:11:48 -0800 Subject: [PATCH 1/5] signature/pkcs7: add PKCS#7 envelope with signerAdapter for dm-verity Adds PKCS#7/CMS signature envelope implementation following the same pattern as existing JWS and COSE envelopes. Features: - Implements signature.Envelope interface (Sign, Verify, Content) - Uses go.mozilla.org/pkcs7 library for ASN.1 encoding - Uses signerAdapter pattern to support both local and remote signers - Works with Azure Key Vault and other plugins via signature.Signer - Produces detached signatures for dm-verity kernel verification - Supports RSA and ECDSA key types with SHA-256 - Registers media type application/pkcs7-signature The signerAdapter wraps pre-computed signatures from any signature.Signer to satisfy the crypto.Signer interface expected by the Mozilla library. This enables remote signers (like Azure Key Vault) that don't expose private keys to work with the library. Signed-off-by: Dallas Delaney --- go.mod | 5 +- go.sum | 2 + signature/pkcs7/DESIGN.md | 110 ++++++++++++++++++ signature/pkcs7/envelope.go | 216 ++++++++++++++++++++++++++++++++++++ 4 files changed, 332 insertions(+), 1 deletion(-) create mode 100644 signature/pkcs7/DESIGN.md create mode 100644 signature/pkcs7/envelope.go diff --git a/go.mod b/go.mod index 5468dbf1..77c86c67 100644 --- a/go.mod +++ b/go.mod @@ -10,4 +10,7 @@ require ( golang.org/x/crypto v0.37.0 ) -require github.com/x448/float16 v0.8.4 // indirect +require ( + github.com/x448/float16 v0.8.4 // indirect + go.mozilla.org/pkcs7 v0.9.0 // indirect +) diff --git a/go.sum b/go.sum index a411058d..8c157d1e 100644 --- a/go.sum +++ b/go.sum @@ -8,5 +8,7 @@ github.com/veraison/go-cose v1.3.0 h1:2/H5w8kdSpQJyVtIhx8gmwPJ2uSz1PkyWFx0idbd7r github.com/veraison/go-cose v1.3.0/go.mod h1:df09OV91aHoQWLmy1KsDdYiagtXgyAwAl8vFeFn1gMc= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= +go.mozilla.org/pkcs7 v0.9.0 h1:yM4/HS9dYv7ri2biPtxt8ikvB37a980dg69/pKmS+eI= +go.mozilla.org/pkcs7 v0.9.0/go.mod h1:SNgMg+EgDFwmvSmLRTNKC5fegJjB7v23qTQ0XLGUNHk= golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= diff --git a/signature/pkcs7/DESIGN.md b/signature/pkcs7/DESIGN.md new file mode 100644 index 00000000..49b1835c --- /dev/null +++ b/signature/pkcs7/DESIGN.md @@ -0,0 +1,110 @@ +# PKCS#7 Envelope Implementation for dm-verity + +This document explains the PKCS#7 signature envelope implementation in notation-core-go. + +## Overview + +The PKCS#7 (CMS - Cryptographic Message Syntax) envelope produces signatures compatible with Linux kernel dm-verity verification. This is the same format produced by: + +```bash +openssl smime -sign -noattr -binary -in content -signer cert.pem -inkey key.pem -outform DER +``` + +## Architecture + +### signerAdapter Pattern + +The key innovation is the `signerAdapter` pattern that enables **both local and remote signers** (like Azure Key Vault) to work with the Mozilla PKCS#7 library: + +``` +┌─────────────────────┐ +│ signature.Signer │ ← Your signer (local key, AKV, plugin) +│ Sign(payload) │ +└─────────┬───────────┘ + │ returns (sig, certs) + ▼ +┌─────────────────────┐ +│ signerAdapter │ ← Wraps pre-computed signature +│ crypto.Signer │ +└─────────┬───────────┘ + │ passed to + ▼ +┌─────────────────────┐ +│ Mozilla pkcs7 │ ← Builds ASN.1 structure +│ SignWithoutAttr() │ +└─────────────────────┘ +``` + +**How it works:** + +1. Call `req.Signer.Sign(payload)` to get signature bytes + certs from any signer +2. Create `signerAdapter` that implements `crypto.Signer` +3. `signerAdapter.Sign()` returns the pre-computed signature (doesn't re-sign) +4. Mozilla library builds PKCS#7 ASN.1 structure using the adapter + +**Why this works:** +The Mozilla library calls `crypto.Signer.Sign()` expecting to perform signing, but our adapter just returns the already-computed signature. The library doesn't know the difference and builds valid PKCS#7 output. + +### Code Structure + +```go +// signerAdapter wraps a pre-computed signature to satisfy crypto.Signer +type signerAdapter struct { + sig []byte // pre-computed from actual signer + certs []*x509.Certificate +} + +func (a *signerAdapter) Public() crypto.PublicKey { + return a.certs[0].PublicKey +} + +func (a *signerAdapter) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + return a.sig, nil // Return pre-computed signature +} +``` + +## Comparison to JWS/COSE + +| Aspect | JWS | COSE | PKCS#7 | +|--------|-----|------|--------| +| Implements Envelope | ✅ | ✅ | ✅ | +| Uses base.Envelope | ✅ | ✅ | ✅ | +| Signer support | Any | Any | Any (via adapter) | +| Encoding | JSON | CBOR | ASN.1 DER | +| Library | go-jwt | go-cose | go.mozilla.org/pkcs7 | +| Use case | OCI artifacts | OCI artifacts | dm-verity kernel | + +## Kernel Compatibility Settings + +```go +// SHA-256 required (kernel rejects SHA-1) +sd.SetDigestAlgorithm(gopkcs7.OIDDigestAlgorithmSHA256) + +// Set encryption algorithm based on key type +sd.SetEncryptionAlgorithm(gopkcs7.OIDEncryptionAlgorithmRSA) + +// No authenticated attributes (-noattr mode) +sd.SignWithoutAttr(cert, adapter, gopkcs7.SignerInfoConfig{}) + +// Detached signature (kernel provides content separately) +sd.Detach() +``` + +## Supported Key Types + +| Key Type | Supported | OID | +|----------|-----------|-----| +| RSA-2048 | ✅ | OIDEncryptionAlgorithmRSA | +| RSA-3072 | ✅ | OIDEncryptionAlgorithmRSA | +| RSA-4096 | ✅ | OIDEncryptionAlgorithmRSA | +| ECDSA P-256 | ✅ | OIDEncryptionAlgorithmECDSAP256 | +| ECDSA P-384 | ✅ | OIDEncryptionAlgorithmECDSAP384 | +| ECDSA P-521 | ✅ | OIDEncryptionAlgorithmECDSAP521 | + +**Note:** Linux kernel dm-verity currently only supports RSA. ECDSA is included for completeness but may not work with kernel verification. + +## References + +- [RFC 5652 - Cryptographic Message Syntax (CMS)](https://tools.ietf.org/html/rfc5652) +- [Mozilla PKCS#7 Library](https://github.com/mozilla-services/pkcs7) +- [Linux Kernel dm-verity](https://www.kernel.org/doc/html/latest/admin-guide/device-mapper/verity.html) diff --git a/signature/pkcs7/envelope.go b/signature/pkcs7/envelope.go new file mode 100644 index 00000000..da8ddfa6 --- /dev/null +++ b/signature/pkcs7/envelope.go @@ -0,0 +1,216 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package pkcs7 provides PKCS#7/CMS signature envelope for dm-verity signing. +// Supports both local keys and remote signers (Azure Key Vault, plugins) via +// the signerAdapter pattern. +package pkcs7 + +import ( + "crypto" + "crypto/x509" + "encoding/asn1" + "fmt" + "io" + + "github.com/notaryproject/notation-core-go/signature" + "github.com/notaryproject/notation-core-go/signature/internal/base" + gopkcs7 "go.mozilla.org/pkcs7" +) + +// MediaTypeEnvelope is the PKCS#7 signature envelope media type. +const MediaTypeEnvelope = "application/pkcs7-signature" + +func init() { + if err := signature.RegisterEnvelopeType(MediaTypeEnvelope, NewEnvelope, ParseEnvelope); err != nil { + panic(err) + } +} + +type envelope struct { + raw []byte + p7 *gopkcs7.PKCS7 + certs []*x509.Certificate + sigBytes []byte +} + +// NewEnvelope creates a new PKCS#7 envelope. +func NewEnvelope() signature.Envelope { + return &base.Envelope{ + Envelope: &envelope{}, + } +} + +// ParseEnvelope parses PKCS#7 DER bytes into an envelope. +func ParseEnvelope(envelopeBytes []byte) (signature.Envelope, error) { + p7, err := gopkcs7.Parse(envelopeBytes) + if err != nil { + return nil, &signature.InvalidSignatureError{Msg: err.Error()} + } + + var sigBytes []byte + if len(p7.Signers) > 0 { + sigBytes = p7.Signers[0].EncryptedDigest + } + + return &base.Envelope{ + Envelope: &envelope{ + raw: envelopeBytes, + p7: p7, + certs: p7.Certificates, + sigBytes: sigBytes, + }, + Raw: envelopeBytes, + }, nil +} + +// Sign implements signature.Envelope interface. +// Uses signerAdapter pattern to support both local and remote signers (AKV, plugins). +func (e *envelope) Sign(req *signature.SignRequest) ([]byte, error) { + // Get key spec to determine algorithm + keySpec, err := req.Signer.KeySpec() + if err != nil { + return nil, &signature.InvalidSignRequestError{Msg: err.Error()} + } + + // Sign the content using the underlying signer (works with AKV, local keys, etc.) + sig, certs, err := req.Signer.Sign(req.Payload.Content) + if err != nil { + return nil, &signature.InvalidSignatureError{Msg: fmt.Sprintf("signing failed: %v", err)} + } + + if len(certs) == 0 { + return nil, &signature.InvalidSignatureError{Msg: "no certificates returned from signer"} + } + + // Create a crypto.Signer adapter that returns our pre-computed signature + adapter := &signerAdapter{ + sig: sig, + certs: certs, + } + + // Build PKCS#7 SignedData using mozilla library + sd, err := gopkcs7.NewSignedData(req.Payload.Content) + if err != nil { + return nil, &signature.InvalidSignatureError{Msg: err.Error()} + } + + // Set digest algorithm to SHA-256 (kernel dm-verity requirement) + sd.SetDigestAlgorithm(gopkcs7.OIDDigestAlgorithmSHA256) + + // Set encryption algorithm based on key type + encryptionOID, err := getEncryptionOID(keySpec) + if err != nil { + return nil, &signature.UnsupportedSigningKeyError{Msg: err.Error()} + } + sd.SetEncryptionAlgorithm(encryptionOID) + + // Sign without authenticated attributes (kernel dm-verity requirement) + if err := sd.SignWithoutAttr(certs[0], adapter, gopkcs7.SignerInfoConfig{}); err != nil { + return nil, &signature.InvalidSignatureError{Msg: err.Error()} + } + + // Add intermediate/CA certificates to the chain + for i := 1; i < len(certs); i++ { + sd.AddCertificate(certs[i]) + } + + // Create detached signature (content not embedded) + sd.Detach() + + encoded, err := sd.Finish() + if err != nil { + return nil, &signature.InvalidSignatureError{Msg: err.Error()} + } + + // Parse the result to populate envelope fields + p7, _ := gopkcs7.Parse(encoded) + e.raw = encoded + e.p7 = p7 + e.certs = certs + if p7 != nil && len(p7.Signers) > 0 { + e.sigBytes = p7.Signers[0].EncryptedDigest + } + + return encoded, nil +} + +// getEncryptionOID returns the encryption algorithm OID for the key spec. +func getEncryptionOID(keySpec signature.KeySpec) (asn1.ObjectIdentifier, error) { + switch keySpec.Type { + case signature.KeyTypeRSA: + return gopkcs7.OIDEncryptionAlgorithmRSA, nil + case signature.KeyTypeEC: + switch keySpec.Size { + case 256: + return gopkcs7.OIDEncryptionAlgorithmECDSAP256, nil + case 384: + return gopkcs7.OIDEncryptionAlgorithmECDSAP384, nil + case 521: + return gopkcs7.OIDEncryptionAlgorithmECDSAP521, nil + default: + return nil, fmt.Errorf("unsupported EC key size: %d", keySpec.Size) + } + default: + return nil, fmt.Errorf("unsupported key type: %v", keySpec.Type) + } +} + +// signerAdapter wraps a pre-computed signature to satisfy crypto.Signer interface. +// This enables remote signers (Azure Key Vault, plugins) to work with the +// mozilla pkcs7 library which expects crypto.Signer. +type signerAdapter struct { + sig []byte // pre-computed signature from actual signer + certs []*x509.Certificate // certificate chain +} + +// Public returns the public key from the leaf certificate. +func (a *signerAdapter) Public() crypto.PublicKey { + if len(a.certs) > 0 { + return a.certs[0].PublicKey + } + return nil +} + +// Sign returns the pre-computed signature. +// The digest parameter is ignored since signing already happened via req.Signer.Sign(). +func (a *signerAdapter) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + return a.sig, nil +} + +// Verify implements signature.Envelope interface. +func (e *envelope) Verify() (*signature.EnvelopeContent, error) { + if e.raw == nil { + return nil, &signature.SignatureEnvelopeNotFoundError{} + } + // For detached signatures, the kernel does dm-verity verification + return e.Content() +} + +// Content implements signature.Envelope interface. +func (e *envelope) Content() (*signature.EnvelopeContent, error) { + if e.raw == nil { + return nil, &signature.SignatureEnvelopeNotFoundError{} + } + + return &signature.EnvelopeContent{ + SignerInfo: signature.SignerInfo{ + SignatureAlgorithm: signature.AlgorithmPS256, + CertificateChain: e.certs, + Signature: e.sigBytes, + }, + Payload: signature.Payload{ + ContentType: MediaTypeEnvelope, + }, + }, nil +} From 99e0fab07ee596eddac9aa98c76e96bdff0dcd2f Mon Sep 17 00:00:00 2001 From: Dallas Delaney Date: Mon, 9 Mar 2026 15:34:17 -0700 Subject: [PATCH 2/5] signature/pkcs7: remove base.Envelope wrapper for kernel compatibility The base.Envelope wrapper validates that signing-time is present, but PKCS#7 signatures for dm-verity must not include authenticated attributes (including signing-time) per Linux kernel requirements. The kernel's PKCS#7 verifier in crypto/asymmetric_keys/public_key.c expects raw signature data without CMS authenticated attributes. Remove the base.Envelope wrapper from NewEnvelope() and ParseEnvelope() so the pkcs7 envelope implements signature.Envelope directly. Signed-off-by: Dallas Delaney --- signature/pkcs7/envelope.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/signature/pkcs7/envelope.go b/signature/pkcs7/envelope.go index da8ddfa6..f1c2cf99 100644 --- a/signature/pkcs7/envelope.go +++ b/signature/pkcs7/envelope.go @@ -24,7 +24,6 @@ import ( "io" "github.com/notaryproject/notation-core-go/signature" - "github.com/notaryproject/notation-core-go/signature/internal/base" gopkcs7 "go.mozilla.org/pkcs7" ) @@ -45,10 +44,10 @@ type envelope struct { } // NewEnvelope creates a new PKCS#7 envelope. +// Note: Unlike JWS/COSE, PKCS#7 for dm-verity does NOT use base.Envelope wrapper +// because dm-verity signatures must not have signing-time (kernel requirement). func NewEnvelope() signature.Envelope { - return &base.Envelope{ - Envelope: &envelope{}, - } + return &envelope{} } // ParseEnvelope parses PKCS#7 DER bytes into an envelope. @@ -63,14 +62,11 @@ func ParseEnvelope(envelopeBytes []byte) (signature.Envelope, error) { sigBytes = p7.Signers[0].EncryptedDigest } - return &base.Envelope{ - Envelope: &envelope{ - raw: envelopeBytes, - p7: p7, - certs: p7.Certificates, - sigBytes: sigBytes, - }, - Raw: envelopeBytes, + return &envelope{ + raw: envelopeBytes, + p7: p7, + certs: p7.Certificates, + sigBytes: sigBytes, }, nil } From 7dc8acae35ee114eb52c46877c561f992c20f2f3 Mon Sep 17 00:00:00 2001 From: Dallas Delaney Date: Mon, 9 Mar 2026 16:26:37 -0700 Subject: [PATCH 3/5] signature/pkcs7: fix Content() algorithm, add tests Signed-off-by: Dallas Delaney --- signature/pkcs7/DESIGN.md | 110 -------------------- signature/pkcs7/envelope.go | 19 +++- signature/pkcs7/envelope_test.go | 169 +++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 111 deletions(-) delete mode 100644 signature/pkcs7/DESIGN.md create mode 100644 signature/pkcs7/envelope_test.go diff --git a/signature/pkcs7/DESIGN.md b/signature/pkcs7/DESIGN.md deleted file mode 100644 index 49b1835c..00000000 --- a/signature/pkcs7/DESIGN.md +++ /dev/null @@ -1,110 +0,0 @@ -# PKCS#7 Envelope Implementation for dm-verity - -This document explains the PKCS#7 signature envelope implementation in notation-core-go. - -## Overview - -The PKCS#7 (CMS - Cryptographic Message Syntax) envelope produces signatures compatible with Linux kernel dm-verity verification. This is the same format produced by: - -```bash -openssl smime -sign -noattr -binary -in content -signer cert.pem -inkey key.pem -outform DER -``` - -## Architecture - -### signerAdapter Pattern - -The key innovation is the `signerAdapter` pattern that enables **both local and remote signers** (like Azure Key Vault) to work with the Mozilla PKCS#7 library: - -``` -┌─────────────────────┐ -│ signature.Signer │ ← Your signer (local key, AKV, plugin) -│ Sign(payload) │ -└─────────┬───────────┘ - │ returns (sig, certs) - ▼ -┌─────────────────────┐ -│ signerAdapter │ ← Wraps pre-computed signature -│ crypto.Signer │ -└─────────┬───────────┘ - │ passed to - ▼ -┌─────────────────────┐ -│ Mozilla pkcs7 │ ← Builds ASN.1 structure -│ SignWithoutAttr() │ -└─────────────────────┘ -``` - -**How it works:** - -1. Call `req.Signer.Sign(payload)` to get signature bytes + certs from any signer -2. Create `signerAdapter` that implements `crypto.Signer` -3. `signerAdapter.Sign()` returns the pre-computed signature (doesn't re-sign) -4. Mozilla library builds PKCS#7 ASN.1 structure using the adapter - -**Why this works:** -The Mozilla library calls `crypto.Signer.Sign()` expecting to perform signing, but our adapter just returns the already-computed signature. The library doesn't know the difference and builds valid PKCS#7 output. - -### Code Structure - -```go -// signerAdapter wraps a pre-computed signature to satisfy crypto.Signer -type signerAdapter struct { - sig []byte // pre-computed from actual signer - certs []*x509.Certificate -} - -func (a *signerAdapter) Public() crypto.PublicKey { - return a.certs[0].PublicKey -} - -func (a *signerAdapter) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { - return a.sig, nil // Return pre-computed signature -} -``` - -## Comparison to JWS/COSE - -| Aspect | JWS | COSE | PKCS#7 | -|--------|-----|------|--------| -| Implements Envelope | ✅ | ✅ | ✅ | -| Uses base.Envelope | ✅ | ✅ | ✅ | -| Signer support | Any | Any | Any (via adapter) | -| Encoding | JSON | CBOR | ASN.1 DER | -| Library | go-jwt | go-cose | go.mozilla.org/pkcs7 | -| Use case | OCI artifacts | OCI artifacts | dm-verity kernel | - -## Kernel Compatibility Settings - -```go -// SHA-256 required (kernel rejects SHA-1) -sd.SetDigestAlgorithm(gopkcs7.OIDDigestAlgorithmSHA256) - -// Set encryption algorithm based on key type -sd.SetEncryptionAlgorithm(gopkcs7.OIDEncryptionAlgorithmRSA) - -// No authenticated attributes (-noattr mode) -sd.SignWithoutAttr(cert, adapter, gopkcs7.SignerInfoConfig{}) - -// Detached signature (kernel provides content separately) -sd.Detach() -``` - -## Supported Key Types - -| Key Type | Supported | OID | -|----------|-----------|-----| -| RSA-2048 | ✅ | OIDEncryptionAlgorithmRSA | -| RSA-3072 | ✅ | OIDEncryptionAlgorithmRSA | -| RSA-4096 | ✅ | OIDEncryptionAlgorithmRSA | -| ECDSA P-256 | ✅ | OIDEncryptionAlgorithmECDSAP256 | -| ECDSA P-384 | ✅ | OIDEncryptionAlgorithmECDSAP384 | -| ECDSA P-521 | ✅ | OIDEncryptionAlgorithmECDSAP521 | - -**Note:** Linux kernel dm-verity currently only supports RSA. ECDSA is included for completeness but may not work with kernel verification. - -## References - -- [RFC 5652 - Cryptographic Message Syntax (CMS)](https://tools.ietf.org/html/rfc5652) -- [Mozilla PKCS#7 Library](https://github.com/mozilla-services/pkcs7) -- [Linux Kernel dm-verity](https://www.kernel.org/doc/html/latest/admin-guide/device-mapper/verity.html) diff --git a/signature/pkcs7/envelope.go b/signature/pkcs7/envelope.go index f1c2cf99..74a1d41a 100644 --- a/signature/pkcs7/envelope.go +++ b/signature/pkcs7/envelope.go @@ -199,9 +199,14 @@ func (e *envelope) Content() (*signature.EnvelopeContent, error) { return nil, &signature.SignatureEnvelopeNotFoundError{} } + alg, err := extractAlgorithm(e.certs) + if err != nil { + return nil, &signature.InvalidSignatureError{Msg: err.Error()} + } + return &signature.EnvelopeContent{ SignerInfo: signature.SignerInfo{ - SignatureAlgorithm: signature.AlgorithmPS256, + SignatureAlgorithm: alg, CertificateChain: e.certs, Signature: e.sigBytes, }, @@ -210,3 +215,15 @@ func (e *envelope) Content() (*signature.EnvelopeContent, error) { }, }, nil } + +// extractAlgorithm derives the signature algorithm from the leaf certificate. +func extractAlgorithm(certs []*x509.Certificate) (signature.Algorithm, error) { + if len(certs) == 0 { + return 0, fmt.Errorf("no certificates available to determine algorithm") + } + keySpec, err := signature.ExtractKeySpec(certs[0]) + if err != nil { + return 0, err + } + return keySpec.SignatureAlgorithm(), nil +} diff --git a/signature/pkcs7/envelope_test.go b/signature/pkcs7/envelope_test.go new file mode 100644 index 00000000..c484c733 --- /dev/null +++ b/signature/pkcs7/envelope_test.go @@ -0,0 +1,169 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pkcs7 + +import ( + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/sha256" + "crypto/x509" + "errors" + "testing" + + "github.com/notaryproject/notation-core-go/signature" + "github.com/notaryproject/notation-core-go/testhelper" +) + +const testPayload = "test dm-verity root hash payload" + +// newRSATestSigner creates an RSA-2048 test signer using testhelper certs. +func newRSATestSigner() *testPrimitiveSigner { + tuple := testhelper.GetRSACertTuple(2048) + rootCert := testhelper.GetRSARootCertificate().Cert + return &testPrimitiveSigner{ + key: tuple.PrivateKey, + certs: []*x509.Certificate{tuple.Cert, rootCert}, + keySpec: signature.KeySpec{Type: signature.KeyTypeRSA, Size: 2048}, + } +} + +// testPrimitiveSigner implements signature.Signer for testing. +type testPrimitiveSigner struct { + key crypto.PrivateKey + certs []*x509.Certificate + keySpec signature.KeySpec +} + +func (s *testPrimitiveSigner) Sign(payload []byte) ([]byte, []*x509.Certificate, error) { + h := sha256.Sum256(payload) + sig, err := rsa.SignPKCS1v15(rand.Reader, s.key.(*rsa.PrivateKey), crypto.SHA256, h[:]) + if err != nil { + return nil, nil, err + } + return sig, s.certs, nil +} + +func (s *testPrimitiveSigner) KeySpec() (signature.KeySpec, error) { + return s.keySpec, nil +} + +// TestSignParseVerifyRoundTrip tests the full Sign → Parse → Verify → Content flow. +func TestSignParseVerifyRoundTrip(t *testing.T) { + env := NewEnvelope() + signer := newRSATestSigner() + + req := &signature.SignRequest{ + Payload: signature.Payload{ + ContentType: MediaTypeEnvelope, + Content: []byte(testPayload), + }, + Signer: signer, + } + + encoded, err := env.Sign(req) + if err != nil { + t.Fatalf("Sign() error: %v", err) + } + if len(encoded) == 0 { + t.Fatal("Sign() returned empty bytes") + } + + // Parse + parsed, err := ParseEnvelope(encoded) + if err != nil { + t.Fatalf("ParseEnvelope() error: %v", err) + } + + // Verify + verifyContent, err := parsed.Verify() + if err != nil { + t.Fatalf("Verify() error: %v", err) + } + if len(verifyContent.SignerInfo.Signature) == 0 { + t.Fatal("Verify() returned empty signature") + } + + // Content + content, err := parsed.Content() + if err != nil { + t.Fatalf("Content() error: %v", err) + } + if len(content.SignerInfo.CertificateChain) == 0 { + t.Fatal("Content() returned no certificates") + } + if content.Payload.ContentType != MediaTypeEnvelope { + t.Fatalf("Content().Payload.ContentType = %q, want %q", + content.Payload.ContentType, MediaTypeEnvelope) + } +} + +// TestParseEnvelopeError tests that invalid input is rejected. +func TestParseEnvelopeError(t *testing.T) { + _, err := ParseEnvelope([]byte("invalid")) + if err == nil { + t.Fatal("ParseEnvelope(invalid) expected error, got nil") + } + var target *signature.InvalidSignatureError + if !errors.As(err, &target) { + t.Fatalf("expected InvalidSignatureError, got %T", err) + } +} + +// TestSignError tests that Sign fails with a broken signer. +func TestSignError(t *testing.T) { + env := NewEnvelope() + req := &signature.SignRequest{ + Payload: signature.Payload{ + ContentType: MediaTypeEnvelope, + Content: []byte(testPayload), + }, + Signer: &failingSigner{}, + } + _, err := env.Sign(req) + if err == nil { + t.Fatal("Sign() with failing signer expected error, got nil") + } +} + +// TestVerifyEmptyEnvelope tests that Verify fails on an unsigned envelope. +func TestVerifyEmptyEnvelope(t *testing.T) { + env := NewEnvelope() + _, err := env.Verify() + if err == nil { + t.Fatal("Verify() on empty envelope expected error, got nil") + } +} + +// TestEnvelopeRegistration verifies the media type was registered via init(). +func TestEnvelopeRegistration(t *testing.T) { + env, err := signature.NewEnvelope(MediaTypeEnvelope) + if err != nil { + t.Fatalf("NewEnvelope(%q) error: %v", MediaTypeEnvelope, err) + } + if env == nil { + t.Fatal("NewEnvelope() returned nil for registered media type") + } +} + +// failingSigner is a test signer whose Sign() always fails. +type failingSigner struct{} + +func (s *failingSigner) Sign(payload []byte) ([]byte, []*x509.Certificate, error) { + return nil, nil, errors.New("signing failed") +} + +func (s *failingSigner) KeySpec() (signature.KeySpec, error) { + return signature.KeySpec{Type: signature.KeyTypeRSA, Size: 2048}, nil +} From cf8313db4c74f5cf8364d8a17d848d63a2cbcd42 Mon Sep 17 00:00:00 2001 From: Dallas Delaney Date: Tue, 12 May 2026 17:08:12 -0700 Subject: [PATCH 4/5] signature/pkcs7: address review feedback - Restrict to RSA-2048 + SHA-256 + RSASSA-PKCS#1 v1.5; reject other keys with UnsupportedSigningKeyError and remove the unreachable EC branches in getEncryptionOID. - Reject SignRequest fields the dm-verity profile cannot honor (SigningTime, Expiry, SigningScheme, SigningAgent, ExtendedSignedAttributes) and nil Signer with InvalidSignRequestError. - Verify upstream signer output is RSASSA-PKCS#1 v1.5 over SHA-256 of the payload before wrapping it. - Verify() returns exported sentinel ErrDetachedNotVerifiable; detached PKCS#7 cannot be verified through signature.Envelope.Verify. - Content() leaves SignerInfo.SignatureAlgorithm zero rather than mislabeling PKCS#1 v1.5 as PSS (enum lacks PKCS#1 v1.5 constants). - Surface the post-Sign Parse error instead of swallowing it. - Recover from gopkcs7 v0.9.0 panics on malformed BER. - signerAdapter panics on a second Sign call (single-use invariant). - Add compile-time signature.Envelope assertion; go mod tidy moves gopkcs7 to direct require. - Add negative tests, seeded fuzz target, and conformance test (OIDs, no signed attrs, detached, chain ordering, independent rsa.VerifyPKCS1v15). Signed-off-by: Dallas Delaney --- go.mod | 6 +- signature/pkcs7/conformance_test.go | 84 +++++++++++ signature/pkcs7/envelope.go | 223 ++++++++++++++++------------ signature/pkcs7/envelope_test.go | 192 +++++++++++++++++------- signature/pkcs7/fuzz_test.go | 46 ++++++ 5 files changed, 402 insertions(+), 149 deletions(-) create mode 100644 signature/pkcs7/conformance_test.go create mode 100644 signature/pkcs7/fuzz_test.go diff --git a/go.mod b/go.mod index 77c86c67..eec7ba5b 100644 --- a/go.mod +++ b/go.mod @@ -7,10 +7,8 @@ require ( github.com/golang-jwt/jwt/v4 v4.5.2 github.com/notaryproject/tspclient-go v1.0.0 github.com/veraison/go-cose v1.3.0 + go.mozilla.org/pkcs7 v0.9.0 golang.org/x/crypto v0.37.0 ) -require ( - github.com/x448/float16 v0.8.4 // indirect - go.mozilla.org/pkcs7 v0.9.0 // indirect -) +require github.com/x448/float16 v0.8.4 // indirect diff --git a/signature/pkcs7/conformance_test.go b/signature/pkcs7/conformance_test.go new file mode 100644 index 00000000..b790328c --- /dev/null +++ b/signature/pkcs7/conformance_test.go @@ -0,0 +1,84 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pkcs7 + +import ( + "crypto" + "crypto/rsa" + "crypto/sha256" + "testing" + + gopkcs7 "go.mozilla.org/pkcs7" + + "github.com/notaryproject/notation-core-go/signature" +) + +// TestConformance asserts that Sign() produces a PKCS#7 SignedData that meets +// the kernel dm-verity profile: RSASSA-PKCS#1 v1.5 over SHA-256, no signed +// attributes, detached content, single signer. +func TestConformance(t *testing.T) { + signer := newRSATestSigner() + encoded, err := NewEnvelope().Sign(&signature.SignRequest{ + Payload: signature.Payload{ContentType: MediaTypeEnvelope, Content: []byte(testPayload)}, + Signer: signer, + }) + if err != nil { + t.Fatalf("Sign() error: %v", err) + } + + p7, err := gopkcs7.Parse(encoded) + if err != nil { + t.Fatalf("Parse() error: %v", err) + } + + if got, want := len(p7.Signers), 1; got != want { + t.Fatalf("Signers = %d, want %d", got, want) + } + + // Cert chain must be leaf + root. dm-verity has no intermediates. + if got, want := len(p7.Certificates), 2; got != want { + t.Fatalf("Certificates = %d, want %d (leaf + root)", got, want) + } + if !p7.Certificates[0].Equal(signer.certs[0]) { + t.Errorf("Certificates[0] is not the leaf certificate") + } + if !p7.Certificates[1].Equal(signer.certs[1]) { + t.Errorf("Certificates[1] is not the root certificate") + } + + si := p7.Signers[0] + if !si.DigestAlgorithm.Algorithm.Equal(gopkcs7.OIDDigestAlgorithmSHA256) { + t.Errorf("DigestAlgorithm = %v, want SHA-256", si.DigestAlgorithm.Algorithm) + } + if !si.DigestEncryptionAlgorithm.Algorithm.Equal(gopkcs7.OIDEncryptionAlgorithmRSA) { + t.Errorf("DigestEncryptionAlgorithm = %v, want rsaEncryption", si.DigestEncryptionAlgorithm.Algorithm) + } + if len(si.AuthenticatedAttributes) != 0 { + t.Errorf("AuthenticatedAttributes len = %d, want 0", len(si.AuthenticatedAttributes)) + } + if len(p7.Content) != 0 { + t.Errorf("Content len = %d, want 0 (must be detached)", len(p7.Content)) + } + + // Independently verify the bytes are RSASSA-PKCS#1 v1.5 over SHA-256 + // of the payload. + digest := sha256.Sum256([]byte(testPayload)) + pub, ok := signer.certs[0].PublicKey.(*rsa.PublicKey) + if !ok { + t.Fatalf("leaf public key is %T, want *rsa.PublicKey", signer.certs[0].PublicKey) + } + if err := rsa.VerifyPKCS1v15(pub, crypto.SHA256, digest[:], si.EncryptedDigest); err != nil { + t.Fatalf("rsa.VerifyPKCS1v15 over SHA-256(payload) failed: %v", err) + } +} diff --git a/signature/pkcs7/envelope.go b/signature/pkcs7/envelope.go index 74a1d41a..ce6e8345 100644 --- a/signature/pkcs7/envelope.go +++ b/signature/pkcs7/envelope.go @@ -11,15 +11,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package pkcs7 provides PKCS#7/CMS signature envelope for dm-verity signing. -// Supports both local keys and remote signers (Azure Key Vault, plugins) via -// the signerAdapter pattern. +// Package pkcs7 provides a PKCS#7/CMS signature envelope for dm-verity +// signing. package pkcs7 import ( "crypto" + "crypto/rsa" + "crypto/sha256" "crypto/x509" - "encoding/asn1" + "errors" "fmt" "io" @@ -27,6 +28,12 @@ import ( gopkcs7 "go.mozilla.org/pkcs7" ) +// ErrDetachedNotVerifiable is returned by Envelope.Verify. The dm-verity +// PKCS#7 envelope is detached and Envelope.Verify takes no payload, so it +// cannot perform cryptographic verification. Verify the dm-verity root +// hash out-of-band. +var ErrDetachedNotVerifiable = errors.New("PKCS#7 dm-verity envelope is detached; verify against the dm-verity root hash out-of-band") + // MediaTypeEnvelope is the PKCS#7 signature envelope media type. const MediaTypeEnvelope = "application/pkcs7-signature" @@ -36,22 +43,32 @@ func init() { } } +// envelope holds a parsed PKCS#7 signature. raw is canonical; certs and +// sigBytes are caches derived from raw. type envelope struct { raw []byte - p7 *gopkcs7.PKCS7 certs []*x509.Certificate sigBytes []byte } // NewEnvelope creates a new PKCS#7 envelope. -// Note: Unlike JWS/COSE, PKCS#7 for dm-verity does NOT use base.Envelope wrapper -// because dm-verity signatures must not have signing-time (kernel requirement). +// +// The dm-verity profile requires a SignerInfo with no signed attributes, +// so this envelope is not wrapped with base.Envelope (which injects a +// signing-time signed attribute). func NewEnvelope() signature.Envelope { return &envelope{} } // ParseEnvelope parses PKCS#7 DER bytes into an envelope. -func ParseEnvelope(envelopeBytes []byte) (signature.Envelope, error) { +func ParseEnvelope(envelopeBytes []byte) (env signature.Envelope, err error) { + // Convert any panic from the underlying parser to a typed error. + defer func() { + if r := recover(); r != nil { + env = nil + err = &signature.InvalidSignatureError{Msg: fmt.Sprintf("malformed PKCS#7 envelope: %v", r)} + } + }() p7, err := gopkcs7.Parse(envelopeBytes) if err != nil { return nil, &signature.InvalidSignatureError{Msg: err.Error()} @@ -64,64 +81,45 @@ func ParseEnvelope(envelopeBytes []byte) (signature.Envelope, error) { return &envelope{ raw: envelopeBytes, - p7: p7, certs: p7.Certificates, sigBytes: sigBytes, }, nil } -// Sign implements signature.Envelope interface. -// Uses signerAdapter pattern to support both local and remote signers (AKV, plugins). +// Sign implements signature.Envelope for the dm-verity profile: RSA-2048 + +// SHA-256 + RSASSA-PKCS#1 v1.5, detached, no signed attributes. The actual +// signing is done by req.Signer; gopkcs7 only wraps the resulting bytes in +// CMS SignedData. Sign verifies the signer's output against certs[0] +// before wrapping. func (e *envelope) Sign(req *signature.SignRequest) ([]byte, error) { - // Get key spec to determine algorithm - keySpec, err := req.Signer.KeySpec() - if err != nil { - return nil, &signature.InvalidSignRequestError{Msg: err.Error()} + if err := validateSignRequest(req); err != nil { + return nil, err } - // Sign the content using the underlying signer (works with AKV, local keys, etc.) sig, certs, err := req.Signer.Sign(req.Payload.Content) if err != nil { return nil, &signature.InvalidSignatureError{Msg: fmt.Sprintf("signing failed: %v", err)} } - - if len(certs) == 0 { - return nil, &signature.InvalidSignatureError{Msg: "no certificates returned from signer"} - } - - // Create a crypto.Signer adapter that returns our pre-computed signature - adapter := &signerAdapter{ - sig: sig, - certs: certs, + if err := verifySignerOutput(req.Payload.Content, sig, certs); err != nil { + return nil, err } - // Build PKCS#7 SignedData using mozilla library sd, err := gopkcs7.NewSignedData(req.Payload.Content) if err != nil { return nil, &signature.InvalidSignatureError{Msg: err.Error()} } - // Set digest algorithm to SHA-256 (kernel dm-verity requirement) + // dm-verity profile: SHA-256 digest, RSASSA-PKCS#1 v1.5 signature, + // no signed attributes, detached content. sd.SetDigestAlgorithm(gopkcs7.OIDDigestAlgorithmSHA256) - - // Set encryption algorithm based on key type - encryptionOID, err := getEncryptionOID(keySpec) - if err != nil { - return nil, &signature.UnsupportedSigningKeyError{Msg: err.Error()} - } - sd.SetEncryptionAlgorithm(encryptionOID) - - // Sign without authenticated attributes (kernel dm-verity requirement) + sd.SetEncryptionAlgorithm(gopkcs7.OIDEncryptionAlgorithmRSA) + adapter := &signerAdapter{sig: sig, certs: certs} if err := sd.SignWithoutAttr(certs[0], adapter, gopkcs7.SignerInfoConfig{}); err != nil { return nil, &signature.InvalidSignatureError{Msg: err.Error()} } - - // Add intermediate/CA certificates to the chain for i := 1; i < len(certs); i++ { sd.AddCertificate(certs[i]) } - - // Create detached signature (content not embedded) sd.Detach() encoded, err := sd.Finish() @@ -129,86 +127,131 @@ func (e *envelope) Sign(req *signature.SignRequest) ([]byte, error) { return nil, &signature.InvalidSignatureError{Msg: err.Error()} } - // Parse the result to populate envelope fields - p7, _ := gopkcs7.Parse(encoded) + // Re-parse to populate caches. + p7, err := gopkcs7.Parse(encoded) + if err != nil { + return nil, &signature.InvalidSignatureError{ + Msg: fmt.Sprintf("self-parse failed after Sign: %v", err), + } + } e.raw = encoded - e.p7 = p7 e.certs = certs - if p7 != nil && len(p7.Signers) > 0 { + if len(p7.Signers) > 0 { e.sigBytes = p7.Signers[0].EncryptedDigest } - return encoded, nil } -// getEncryptionOID returns the encryption algorithm OID for the key spec. -func getEncryptionOID(keySpec signature.KeySpec) (asn1.ObjectIdentifier, error) { - switch keySpec.Type { - case signature.KeyTypeRSA: - return gopkcs7.OIDEncryptionAlgorithmRSA, nil - case signature.KeyTypeEC: - switch keySpec.Size { - case 256: - return gopkcs7.OIDEncryptionAlgorithmECDSAP256, nil - case 384: - return gopkcs7.OIDEncryptionAlgorithmECDSAP384, nil - case 521: - return gopkcs7.OIDEncryptionAlgorithmECDSAP521, nil - default: - return nil, fmt.Errorf("unsupported EC key size: %d", keySpec.Size) +// validateSignRequest enforces the dm-verity profile: no signed-attribute +// fields, RSA-2048 key only. +func validateSignRequest(req *signature.SignRequest) error { + if !req.SigningTime.IsZero() { + return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope does not support SigningTime"} + } + if !req.Expiry.IsZero() { + return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope does not support Expiry"} + } + if req.SigningScheme != "" { + return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope does not support SigningScheme"} + } + if req.SigningAgent != "" { + return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope does not support SigningAgent"} + } + if len(req.ExtendedSignedAttributes) > 0 { + return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope does not support ExtendedSignedAttributes"} + } + + if req.Signer == nil { + return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope requires a non-nil Signer"} + } + keySpec, err := req.Signer.KeySpec() + if err != nil { + return &signature.InvalidSignRequestError{Msg: err.Error()} + } + + // dm-verity profile: RSA-2048 + SHA-256 + RSASSA-PKCS#1 v1.5 only. + if keySpec.Type != signature.KeyTypeRSA { + return &signature.UnsupportedSigningKeyError{ + Msg: fmt.Sprintf("dm-verity PKCS#7 envelope requires an RSA key; got %v", keySpec.Type), + } + } + if keySpec.Size != 2048 { + return &signature.UnsupportedSigningKeyError{ + Msg: fmt.Sprintf("dm-verity PKCS#7 envelope requires RSA-2048; got RSA-%d", keySpec.Size), + } + } + return nil +} + +// verifySignerOutput checks that sig is RSASSA-PKCS#1 v1.5 over SHA-256 of +// payload under certs[0]'s public key. +func verifySignerOutput(payload, sig []byte, certs []*x509.Certificate) error { + if len(certs) == 0 { + return &signature.InvalidSignatureError{Msg: "no certificates returned from signer"} + } + pub, ok := certs[0].PublicKey.(*rsa.PublicKey) + if !ok { + return &signature.UnsupportedSigningKeyError{ + Msg: fmt.Sprintf("leaf certificate public key is %T, want *rsa.PublicKey", certs[0].PublicKey), + } + } + digest := sha256.Sum256(payload) + if err := rsa.VerifyPKCS1v15(pub, crypto.SHA256, digest[:], sig); err != nil { + return &signature.InvalidSignatureError{ + Msg: fmt.Sprintf("signer did not produce RSASSA-PKCS#1 v1.5 over SHA-256: %v", err), } - default: - return nil, fmt.Errorf("unsupported key type: %v", keySpec.Type) } + return nil } -// signerAdapter wraps a pre-computed signature to satisfy crypto.Signer interface. -// This enables remote signers (Azure Key Vault, plugins) to work with the -// mozilla pkcs7 library which expects crypto.Signer. +// signerAdapter wraps a pre-computed signature so it can be passed to a +// gopkcs7.SignedData (which expects a crypto.Signer). It is single-use: +// Sign must be called at most once. type signerAdapter struct { sig []byte // pre-computed signature from actual signer certs []*x509.Certificate // certificate chain + used bool // set on first Sign call } -// Public returns the public key from the leaf certificate. +// Public returns the leaf certificate's public key. func (a *signerAdapter) Public() crypto.PublicKey { - if len(a.certs) > 0 { - return a.certs[0].PublicKey + if len(a.certs) == 0 { + panic("pkcs7: signerAdapter constructed with empty cert chain") } - return nil + return a.certs[0].PublicKey } -// Sign returns the pre-computed signature. -// The digest parameter is ignored since signing already happened via req.Signer.Sign(). +// Sign returns the pre-computed signature. The digest and opts arguments +// are ignored. Panics if called more than once. func (a *signerAdapter) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + if a.used { + panic("pkcs7: signerAdapter.Sign called more than once") + } + a.used = true return a.sig, nil } -// Verify implements signature.Envelope interface. +// Verify always returns ErrDetachedNotVerifiable for a populated envelope. func (e *envelope) Verify() (*signature.EnvelopeContent, error) { if e.raw == nil { return nil, &signature.SignatureEnvelopeNotFoundError{} } - // For detached signatures, the kernel does dm-verity verification - return e.Content() + return nil, ErrDetachedNotVerifiable } -// Content implements signature.Envelope interface. +// Content implements signature.Envelope. +// +// SignerInfo.SignatureAlgorithm is the zero value: signature.Algorithm has +// no constant for RSASSA-PKCS#1 v1.5. Read the certificate chain or the +// CMS digestEncryptionAlgorithm OID instead. func (e *envelope) Content() (*signature.EnvelopeContent, error) { if e.raw == nil { return nil, &signature.SignatureEnvelopeNotFoundError{} } - - alg, err := extractAlgorithm(e.certs) - if err != nil { - return nil, &signature.InvalidSignatureError{Msg: err.Error()} - } - return &signature.EnvelopeContent{ SignerInfo: signature.SignerInfo{ - SignatureAlgorithm: alg, - CertificateChain: e.certs, - Signature: e.sigBytes, + CertificateChain: e.certs, + Signature: e.sigBytes, }, Payload: signature.Payload{ ContentType: MediaTypeEnvelope, @@ -216,14 +259,4 @@ func (e *envelope) Content() (*signature.EnvelopeContent, error) { }, nil } -// extractAlgorithm derives the signature algorithm from the leaf certificate. -func extractAlgorithm(certs []*x509.Certificate) (signature.Algorithm, error) { - if len(certs) == 0 { - return 0, fmt.Errorf("no certificates available to determine algorithm") - } - keySpec, err := signature.ExtractKeySpec(certs[0]) - if err != nil { - return 0, err - } - return keySpec.SignatureAlgorithm(), nil -} +var _ signature.Envelope = (*envelope)(nil) diff --git a/signature/pkcs7/envelope_test.go b/signature/pkcs7/envelope_test.go index c484c733..3dab8fdd 100644 --- a/signature/pkcs7/envelope_test.go +++ b/signature/pkcs7/envelope_test.go @@ -21,6 +21,7 @@ import ( "crypto/x509" "errors" "testing" + "time" "github.com/notaryproject/notation-core-go/signature" "github.com/notaryproject/notation-core-go/testhelper" @@ -59,20 +60,17 @@ func (s *testPrimitiveSigner) KeySpec() (signature.KeySpec, error) { return s.keySpec, nil } -// TestSignParseVerifyRoundTrip tests the full Sign → Parse → Verify → Content flow. -func TestSignParseVerifyRoundTrip(t *testing.T) { - env := NewEnvelope() - signer := newRSATestSigner() - - req := &signature.SignRequest{ - Payload: signature.Payload{ - ContentType: MediaTypeEnvelope, - Content: []byte(testPayload), - }, - Signer: signer, +// newSignRequest builds a SignRequest backed by an RSA-2048 test signer. +func newSignRequest() *signature.SignRequest { + return &signature.SignRequest{ + Payload: signature.Payload{ContentType: MediaTypeEnvelope, Content: []byte(testPayload)}, + Signer: newRSATestSigner(), } +} - encoded, err := env.Sign(req) +// TestSignParseVerifyRoundTrip exercises Sign → Parse → Verify → Content. +func TestSignParseVerifyRoundTrip(t *testing.T) { + encoded, err := NewEnvelope().Sign(newSignRequest()) if err != nil { t.Fatalf("Sign() error: %v", err) } @@ -80,22 +78,15 @@ func TestSignParseVerifyRoundTrip(t *testing.T) { t.Fatal("Sign() returned empty bytes") } - // Parse parsed, err := ParseEnvelope(encoded) if err != nil { t.Fatalf("ParseEnvelope() error: %v", err) } - // Verify - verifyContent, err := parsed.Verify() - if err != nil { - t.Fatalf("Verify() error: %v", err) - } - if len(verifyContent.SignerInfo.Signature) == 0 { - t.Fatal("Verify() returned empty signature") + if _, err := parsed.Verify(); !errors.Is(err, ErrDetachedNotVerifiable) { + t.Fatalf("Verify() = %v, want ErrDetachedNotVerifiable", err) } - // Content content, err := parsed.Content() if err != nil { t.Fatalf("Content() error: %v", err) @@ -104,49 +95,35 @@ func TestSignParseVerifyRoundTrip(t *testing.T) { t.Fatal("Content() returned no certificates") } if content.Payload.ContentType != MediaTypeEnvelope { - t.Fatalf("Content().Payload.ContentType = %q, want %q", - content.Payload.ContentType, MediaTypeEnvelope) + t.Fatalf("ContentType = %q, want %q", content.Payload.ContentType, MediaTypeEnvelope) } } -// TestParseEnvelopeError tests that invalid input is rejected. func TestParseEnvelopeError(t *testing.T) { _, err := ParseEnvelope([]byte("invalid")) - if err == nil { - t.Fatal("ParseEnvelope(invalid) expected error, got nil") - } var target *signature.InvalidSignatureError if !errors.As(err, &target) { - t.Fatalf("expected InvalidSignatureError, got %T", err) + t.Fatalf("want InvalidSignatureError, got %T: %v", err, err) } } -// TestSignError tests that Sign fails with a broken signer. func TestSignError(t *testing.T) { - env := NewEnvelope() - req := &signature.SignRequest{ - Payload: signature.Payload{ - ContentType: MediaTypeEnvelope, - Content: []byte(testPayload), - }, - Signer: &failingSigner{}, - } - _, err := env.Sign(req) - if err == nil { + req := newSignRequest() + req.Signer = &stubSigner{ + keySpec: signature.KeySpec{Type: signature.KeyTypeRSA, Size: 2048}, + signErr: errors.New("signing failed"), + } + if _, err := NewEnvelope().Sign(req); err == nil { t.Fatal("Sign() with failing signer expected error, got nil") } } -// TestVerifyEmptyEnvelope tests that Verify fails on an unsigned envelope. func TestVerifyEmptyEnvelope(t *testing.T) { - env := NewEnvelope() - _, err := env.Verify() - if err == nil { + if _, err := NewEnvelope().Verify(); err == nil { t.Fatal("Verify() on empty envelope expected error, got nil") } } -// TestEnvelopeRegistration verifies the media type was registered via init(). func TestEnvelopeRegistration(t *testing.T) { env, err := signature.NewEnvelope(MediaTypeEnvelope) if err != nil { @@ -157,13 +134,128 @@ func TestEnvelopeRegistration(t *testing.T) { } } -// failingSigner is a test signer whose Sign() always fails. -type failingSigner struct{} +// TestSignRejectsNonRSA2048 verifies that Sign refuses key specs outside +// the dm-verity profile (RSA-2048 only). +func TestSignRejectsNonRSA2048(t *testing.T) { + cases := []struct { + name string + keySpec signature.KeySpec + }{ + {"ecdsa-p256", signature.KeySpec{Type: signature.KeyTypeEC, Size: 256}}, + {"rsa-3072", signature.KeySpec{Type: signature.KeyTypeRSA, Size: 3072}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := NewEnvelope().Sign(&signature.SignRequest{ + Payload: signature.Payload{ContentType: MediaTypeEnvelope, Content: []byte(testPayload)}, + Signer: &stubSigner{keySpec: tc.keySpec}, + }) + var want *signature.UnsupportedSigningKeyError + if !errors.As(err, &want) { + t.Fatalf("want UnsupportedSigningKeyError, got %T: %v", err, err) + } + }) + } +} -func (s *failingSigner) Sign(payload []byte) ([]byte, []*x509.Certificate, error) { - return nil, nil, errors.New("signing failed") +// stubSigner reports a fixed KeySpec and returns signErr from Sign(). +type stubSigner struct { + keySpec signature.KeySpec + signErr error } -func (s *failingSigner) KeySpec() (signature.KeySpec, error) { - return signature.KeySpec{Type: signature.KeyTypeRSA, Size: 2048}, nil +func (s *stubSigner) Sign([]byte) ([]byte, []*x509.Certificate, error) { + if s.signErr == nil { + return nil, nil, errors.New("stubSigner.Sign: no signErr configured") + } + return nil, nil, s.signErr +} +func (s *stubSigner) KeySpec() (signature.KeySpec, error) { return s.keySpec, nil } + +// TestSignRejectsSignedAttributeFields verifies that signed-attribute +// fields on the SignRequest are rejected. +func TestSignRejectsSignedAttributeFields(t *testing.T) { + cases := []struct { + name string + mutate func(*signature.SignRequest) + }{ + {"SigningTime", func(r *signature.SignRequest) { r.SigningTime = time.Now() }}, + {"Expiry", func(r *signature.SignRequest) { r.Expiry = time.Now().Add(time.Hour) }}, + {"SigningScheme", func(r *signature.SignRequest) { r.SigningScheme = signature.SigningSchemeX509 }}, + {"SigningAgent", func(r *signature.SignRequest) { r.SigningAgent = "notation/0.0" }}, + {"ExtendedSignedAttributes", func(r *signature.SignRequest) { + r.ExtendedSignedAttributes = []signature.Attribute{{Key: "k", Value: "v"}} + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + req := newSignRequest() + tc.mutate(req) + _, err := NewEnvelope().Sign(req) + var want *signature.InvalidSignRequestError + if !errors.As(err, &want) { + t.Fatalf("want InvalidSignRequestError, got %T: %v", err, err) + } + }) + } +} + +// TestSignRejectsNilSigner verifies that a SignRequest with a nil Signer +// returns a typed error. +func TestSignRejectsNilSigner(t *testing.T) { + _, err := NewEnvelope().Sign(&signature.SignRequest{ + Payload: signature.Payload{ContentType: MediaTypeEnvelope, Content: []byte(testPayload)}, + }) + var want *signature.InvalidSignRequestError + if !errors.As(err, &want) { + t.Fatalf("want InvalidSignRequestError, got %T: %v", err, err) + } +} + +// TestSignRejectsBadSignerOutput verifies that a signer returning bytes +// other than RSASSA-PKCS#1 v1.5 over SHA-256 is rejected. +func TestSignRejectsBadSignerOutput(t *testing.T) { + base := newRSATestSigner() + cases := []struct { + name string + sign func(payload []byte) ([]byte, error) + }{ + {"pss-instead-of-pkcs1v15", func(payload []byte) ([]byte, error) { + h := sha256.Sum256(payload) + return rsa.SignPSS(rand.Reader, base.key.(*rsa.PrivateKey), crypto.SHA256, h[:], nil) + }}, + {"random-bytes", func([]byte) ([]byte, error) { + b := make([]byte, 256) // RSA-2048 signature length + _, err := rand.Read(b) + return b, err + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := NewEnvelope().Sign(&signature.SignRequest{ + Payload: signature.Payload{ContentType: MediaTypeEnvelope, Content: []byte(testPayload)}, + Signer: &customSignSigner{certs: base.certs, keySpec: base.keySpec, sign: tc.sign}, + }) + var want *signature.InvalidSignatureError + if !errors.As(err, &want) { + t.Fatalf("want InvalidSignatureError, got %T: %v", err, err) + } + }) + } +} + +// customSignSigner delegates Sign() to a caller-supplied function. +type customSignSigner struct { + certs []*x509.Certificate + keySpec signature.KeySpec + sign func(payload []byte) ([]byte, error) +} + +func (s *customSignSigner) Sign(payload []byte) ([]byte, []*x509.Certificate, error) { + sig, err := s.sign(payload) + if err != nil { + return nil, nil, err + } + return sig, s.certs, nil } +func (s *customSignSigner) KeySpec() (signature.KeySpec, error) { return s.keySpec, nil } diff --git a/signature/pkcs7/fuzz_test.go b/signature/pkcs7/fuzz_test.go new file mode 100644 index 00000000..f06ae2ec --- /dev/null +++ b/signature/pkcs7/fuzz_test.go @@ -0,0 +1,46 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pkcs7 + +import ( + "testing" + + "github.com/notaryproject/notation-core-go/signature" +) + +func FuzzSignaturePkcs7(f *testing.F) { + // Seed the corpus with a known-good envelope. + seed, err := NewEnvelope().Sign(&signature.SignRequest{ + Payload: signature.Payload{ContentType: MediaTypeEnvelope, Content: []byte(testPayload)}, + Signer: newRSATestSigner(), + }) + if err != nil { + f.Fatalf("seed Sign() error: %v", err) + } + f.Add(seed, true) + f.Add(seed, false) + + f.Fuzz(func(t *testing.T, envelopeBytes []byte, shouldVerify bool) { + e, err := ParseEnvelope(envelopeBytes) + if err != nil { + t.Skip() + } + + if shouldVerify { + _, _ = e.Verify() + } else { + _, _ = e.Content() + } + }) +} From 59d15e23f46b251aa205099cc0d53013b32cb7d9 Mon Sep 17 00:00:00 2001 From: Dallas Delaney Date: Wed, 13 May 2026 17:12:42 -0700 Subject: [PATCH 5/5] signature/pkcs7: tighten validation per Copilot review - ParseEnvelope rejects non-1 signer count and empty EncryptedDigest - validateSignRequest rejects empty Payload.Content (matches base.Envelope) - verifySignerOutput guards against nil leaf certificate - signerAdapter.Sign returns error instead of panicking on second call - Content().Payload.ContentType is empty (PKCS#7 detached doesn't carry it) - add tests for new validation checks Signed-off-by: Dallas Delaney --- signature/pkcs7/envelope.go | 33 ++++++++++----- signature/pkcs7/envelope_test.go | 72 +++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/signature/pkcs7/envelope.go b/signature/pkcs7/envelope.go index ce6e8345..4304e266 100644 --- a/signature/pkcs7/envelope.go +++ b/signature/pkcs7/envelope.go @@ -74,9 +74,15 @@ func ParseEnvelope(envelopeBytes []byte) (env signature.Envelope, err error) { return nil, &signature.InvalidSignatureError{Msg: err.Error()} } - var sigBytes []byte - if len(p7.Signers) > 0 { - sigBytes = p7.Signers[0].EncryptedDigest + // dm-verity profile: exactly one signer with a non-empty signature. + if len(p7.Signers) != 1 { + return nil, &signature.InvalidSignatureError{ + Msg: fmt.Sprintf("dm-verity PKCS#7 envelope requires exactly one signer; got %d", len(p7.Signers)), + } + } + sigBytes := p7.Signers[0].EncryptedDigest + if len(sigBytes) == 0 { + return nil, &signature.InvalidSignatureError{Msg: "dm-verity PKCS#7 envelope has empty signature"} } return &envelope{ @@ -142,9 +148,12 @@ func (e *envelope) Sign(req *signature.SignRequest) ([]byte, error) { return encoded, nil } -// validateSignRequest enforces the dm-verity profile: no signed-attribute -// fields, RSA-2048 key only. +// validateSignRequest enforces the dm-verity profile: non-empty payload, +// no signed-attribute fields, RSA-2048 key only. func validateSignRequest(req *signature.SignRequest) error { + if len(req.Payload.Content) == 0 { + return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope requires a non-empty Payload.Content"} + } if !req.SigningTime.IsZero() { return &signature.InvalidSignRequestError{Msg: "dm-verity PKCS#7 envelope does not support SigningTime"} } @@ -189,6 +198,9 @@ func verifySignerOutput(payload, sig []byte, certs []*x509.Certificate) error { if len(certs) == 0 { return &signature.InvalidSignatureError{Msg: "no certificates returned from signer"} } + if certs[0] == nil { + return &signature.InvalidSignatureError{Msg: "signer returned a nil leaf certificate"} + } pub, ok := certs[0].PublicKey.(*rsa.PublicKey) if !ok { return &signature.UnsupportedSigningKeyError{ @@ -222,10 +234,10 @@ func (a *signerAdapter) Public() crypto.PublicKey { } // Sign returns the pre-computed signature. The digest and opts arguments -// are ignored. Panics if called more than once. +// are ignored. Returns an error if called more than once. func (a *signerAdapter) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { if a.used { - panic("pkcs7: signerAdapter.Sign called more than once") + return nil, errors.New("pkcs7: signerAdapter.Sign called more than once") } a.used = true return a.sig, nil @@ -253,9 +265,10 @@ func (e *envelope) Content() (*signature.EnvelopeContent, error) { CertificateChain: e.certs, Signature: e.sigBytes, }, - Payload: signature.Payload{ - ContentType: MediaTypeEnvelope, - }, + // Payload.ContentType identifies the signed payload's media type, + // not the envelope format. PKCS#7 detached signatures don't carry + // the payload's content type, so it is unknown after parsing. + Payload: signature.Payload{}, }, nil } diff --git a/signature/pkcs7/envelope_test.go b/signature/pkcs7/envelope_test.go index 3dab8fdd..081c66b5 100644 --- a/signature/pkcs7/envelope_test.go +++ b/signature/pkcs7/envelope_test.go @@ -25,6 +25,7 @@ import ( "github.com/notaryproject/notation-core-go/signature" "github.com/notaryproject/notation-core-go/testhelper" + gopkcs7 "go.mozilla.org/pkcs7" ) const testPayload = "test dm-verity root hash payload" @@ -94,8 +95,10 @@ func TestSignParseVerifyRoundTrip(t *testing.T) { if len(content.SignerInfo.CertificateChain) == 0 { t.Fatal("Content() returned no certificates") } - if content.Payload.ContentType != MediaTypeEnvelope { - t.Fatalf("ContentType = %q, want %q", content.Payload.ContentType, MediaTypeEnvelope) + // PKCS#7 detached signatures don't carry the signed payload's content + // type, so Content() reports it as unknown. + if content.Payload.ContentType != "" { + t.Fatalf("ContentType = %q, want empty", content.Payload.ContentType) } } @@ -259,3 +262,68 @@ func (s *customSignSigner) Sign(payload []byte) ([]byte, []*x509.Certificate, er return sig, s.certs, nil } func (s *customSignSigner) KeySpec() (signature.KeySpec, error) { return s.keySpec, nil } + +// TestSignRejectsEmptyPayload verifies that an empty Payload.Content is +// rejected with InvalidSignRequestError. +func TestSignRejectsEmptyPayload(t *testing.T) { + req := newSignRequest() + req.Payload.Content = nil + _, err := NewEnvelope().Sign(req) + var want *signature.InvalidSignRequestError + if !errors.As(err, &want) { + t.Fatalf("want InvalidSignRequestError, got %T: %v", err, err) + } +} + +// TestSignRejectsNilLeafCertificate verifies that a signer returning a +// nil leaf certificate is rejected with InvalidSignatureError. +func TestSignRejectsNilLeafCertificate(t *testing.T) { + base := newRSATestSigner() + req := &signature.SignRequest{ + Payload: signature.Payload{ContentType: MediaTypeEnvelope, Content: []byte(testPayload)}, + Signer: &customSignSigner{ + certs: []*x509.Certificate{nil, base.certs[1]}, + keySpec: base.keySpec, + sign: func(payload []byte) ([]byte, error) { + h := sha256.Sum256(payload) + return rsa.SignPKCS1v15(rand.Reader, base.key.(*rsa.PrivateKey), crypto.SHA256, h[:]) + }, + }, + } + _, err := NewEnvelope().Sign(req) + var want *signature.InvalidSignatureError + if !errors.As(err, &want) { + t.Fatalf("want InvalidSignatureError, got %T: %v", err, err) + } +} + +// TestParseEnvelopeRejectsMultipleSigners verifies that the dm-verity +// profile's "exactly one signer" guard fires on multi-signer envelopes. +// Empty-EncryptedDigest is also exercised by FuzzSignaturePkcs7. +func TestParseEnvelopeRejectsMultipleSigners(t *testing.T) { + tuple := testhelper.GetRSACertTuple(2048) + sd, err := gopkcs7.NewSignedData([]byte(testPayload)) + if err != nil { + t.Fatalf("NewSignedData() error: %v", err) + } + sd.SetDigestAlgorithm(gopkcs7.OIDDigestAlgorithmSHA256) + sd.SetEncryptionAlgorithm(gopkcs7.OIDEncryptionAlgorithmRSA) + cfg := gopkcs7.SignerInfoConfig{} + if err := sd.SignWithoutAttr(tuple.Cert, tuple.PrivateKey, cfg); err != nil { + t.Fatalf("SignWithoutAttr() #1 error: %v", err) + } + if err := sd.SignWithoutAttr(tuple.Cert, tuple.PrivateKey, cfg); err != nil { + t.Fatalf("SignWithoutAttr() #2 error: %v", err) + } + sd.Detach() + encoded, err := sd.Finish() + if err != nil { + t.Fatalf("Finish() error: %v", err) + } + + _, err = ParseEnvelope(encoded) + var want *signature.InvalidSignatureError + if !errors.As(err, &want) { + t.Fatalf("want InvalidSignatureError, got %T: %v", err, err) + } +}