Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ 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
)

Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
84 changes: 84 additions & 0 deletions signature/pkcs7/conformance_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
262 changes: 262 additions & 0 deletions signature/pkcs7/envelope.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
// 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 a PKCS#7/CMS signature envelope for dm-verity
// signing.
package pkcs7

import (
"crypto"
"crypto/rsa"
"crypto/sha256"
"crypto/x509"
"errors"
"fmt"
"io"

"github.com/notaryproject/notation-core-go/signature"
gopkcs7 "go.mozilla.org/pkcs7"
)

Comment thread
dallasd1 marked this conversation as resolved.
// 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"

func init() {
if err := signature.RegisterEnvelopeType(MediaTypeEnvelope, NewEnvelope, ParseEnvelope); err != nil {
panic(err)
}
}
Comment thread
dallasd1 marked this conversation as resolved.

// envelope holds a parsed PKCS#7 signature. raw is canonical; certs and
// sigBytes are caches derived from raw.
type envelope struct {
raw []byte
certs []*x509.Certificate
sigBytes []byte
}

// NewEnvelope creates a new PKCS#7 envelope.
//
// 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{}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jws and cose return &base.Envelope{Envelope: &envelope{}} which has critical validations. returning a bare envelope bypasses all of these checks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This initial PR is scoped to a narrow profile of detached PKCS#7, no signed attributes, rsa-2048, and sha-256, validated end-to-end. The long-term plan would be to broaden this profile after the initial change lands. Broadening the profile would likely mean also coordinating changes to notation/specifications

Given the scope described above, this envelope intentionally does not wrap base.Envelope because it would require SigningTime and SigningScheme to be set. The PR as-is will only require RSA-2048 + SHA-256.

To ensure validation is still present, we use validateSignRequest() and verifySignerOutput(), which does similar validation checks (except for ValidateCodeSigningCertChain(), which can be added if you prefer)

}
Comment on lines +59 to +61
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NewEnvelope() returns a zero-value *envelope, then Sign() mutates e.raw/p7/certs/sigBytes (lines 134–138) so subsequent Verify/Content calls return what was just signed. This conflates construction with signing-result. The JWS/COSE envelopes use base.Envelope precisely to separate these. The PR comment acknowledges skipping base.Envelope "because dm-verity signatures must not have signing-time" — but base.Envelope doesn't force signing-time on you; only the envelope-specific Sign method does. Please reconsider; sharing base.Envelope (with the signing-time bypass) gives consistent state semantics and free Payload() accessors.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sharing from comment above, this initial PR is scoped to a narrow profile of detached PKCS#7 and gated behind NOTATION_EXPERIMENTAL, but if base.Envelope is required at this stage I can draft up that change.


// ParseEnvelope parses PKCS#7 DER bytes into an envelope.
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()}
}

var sigBytes []byte
if len(p7.Signers) > 0 {
sigBytes = p7.Signers[0].EncryptedDigest
}

return &envelope{
raw: envelopeBytes,
certs: p7.Certificates,
sigBytes: sigBytes,
}, nil
Comment thread
dallasd1 marked this conversation as resolved.
}

// 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) {
if err := validateSignRequest(req); err != nil {
return nil, err
}

sig, certs, err := req.Signer.Sign(req.Payload.Content)
if err != nil {
return nil, &signature.InvalidSignatureError{Msg: fmt.Sprintf("signing failed: %v", err)}
}
if err := verifySignerOutput(req.Payload.Content, sig, certs); err != nil {
return nil, err
}
Comment thread
dallasd1 marked this conversation as resolved.

sd, err := gopkcs7.NewSignedData(req.Payload.Content)
if err != nil {
return nil, &signature.InvalidSignatureError{Msg: err.Error()}
}

// dm-verity profile: SHA-256 digest, RSASSA-PKCS#1 v1.5 signature,
// no signed attributes, detached content.
sd.SetDigestAlgorithm(gopkcs7.OIDDigestAlgorithmSHA256)
Comment thread
dallasd1 marked this conversation as resolved.
Comment thread
dallasd1 marked this conversation as resolved.
sd.SetEncryptionAlgorithm(gopkcs7.OIDEncryptionAlgorithmRSA)
adapter := &signerAdapter{sig: sig, certs: certs}
if err := sd.SignWithoutAttr(certs[0], adapter, gopkcs7.SignerInfoConfig{}); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per the upstream comment: "This function is needed to sign old Android APKs, something you probably shouldn't do unless you're maintaining backward compatibility for old applications." Please add a comment explaining why we use it here (kernel dm-verity requires no SignedAttributes) so future readers don't think we hit this by accident.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add comment in a new commit bundled with the pending tspclient change

return nil, &signature.InvalidSignatureError{Msg: err.Error()}
}
for i := 1; i < len(certs); i++ {
sd.AddCertificate(certs[i])
}
sd.Detach()

encoded, err := sd.Finish()
if err != nil {
return nil, &signature.InvalidSignatureError{Msg: err.Error()}
}

// 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.certs = certs
if len(p7.Signers) > 0 {
e.sigBytes = p7.Signers[0].EncryptedDigest
}
return encoded, nil
}

// 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"}
}
Comment thread
dallasd1 marked this conversation as resolved.
Outdated
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),
}
}
Comment thread
dallasd1 marked this conversation as resolved.
return nil
}

// 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 leaf certificate's public key.
func (a *signerAdapter) Public() crypto.PublicKey {
if len(a.certs) == 0 {
panic("pkcs7: signerAdapter constructed with empty cert chain")
}
return a.certs[0].PublicKey
}

// 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
}
Comment thread
dallasd1 marked this conversation as resolved.

// Verify always returns ErrDetachedNotVerifiable for a populated envelope.
func (e *envelope) Verify() (*signature.EnvelopeContent, error) {
if e.raw == nil {
return nil, &signature.SignatureEnvelopeNotFoundError{}
}
Comment thread
dallasd1 marked this conversation as resolved.
return nil, ErrDetachedNotVerifiable
}
Comment on lines +247 to +252
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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()
}
// Verify implements signature.Envelope interface by performing detached-PKCS#7
// signature verification: it recomputes SHA-256(content) and verifies the
// signature against the leaf certificate's public key.
//
// NOTE: dm-verity verification on the kernel side is independent and out of
// scope. This method establishes the cryptographic validity of the signature
// for any in-process consumer (tests, pre-submission checks, audit tooling).
func (e *envelope) Verify() (*signature.EnvelopeContent, error) {
if e.raw == nil {
return nil, &signature.SignatureEnvelopeNotFoundError{}
}
if e.p7 == nil {
return nil, &signature.InvalidSignatureError{Msg: "envelope not parsed"}
}
if err := e.p7.Verify(); err != nil {
return nil, &signature.SignatureIntegrityError{Err: err}
}
return e.Content()
}

A signature.Envelope.Verify() that doesn't verify is a contract violation that downstream code (including notation-go's verifier dispatch) cannot detect. The Mozilla pkcs7 library exposes (*PKCS7).Verify() — please use it. If the envelope is detached, the parser already needs the content to verify; we should require callers to provide it via something equivalent to the existing signature.SignRequest/Envelope.Content() flow. If we genuinely cannot do this verification (because dm-verity is out-of-process), then PKCS#7 should not implement signature.Envelope at all; it should be a separate, smaller 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{}
}
return &signature.EnvelopeContent{
SignerInfo: signature.SignerInfo{
CertificateChain: e.certs,
Signature: e.sigBytes,
},
Payload: signature.Payload{
ContentType: MediaTypeEnvelope,
},
Comment thread
dallasd1 marked this conversation as resolved.
Outdated
}, nil
}

var _ signature.Envelope = (*envelope)(nil)
Loading
Loading