Skip to content

Commit

Permalink
fix: use Signer hash algorithm when possible
Browse files Browse the repository at this point in the history
  • Loading branch information
@tri-adam
tri-adam committed Feb 6, 2023
1 parent f61233a commit b191ade
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 63 deletions.
55 changes: 36 additions & 19 deletions pkg/integrity/dsse.go
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2022, Sylabs Inc. All rights reserved.
// Copyright (c) 2022-2023, Sylabs Inc. All rights reserved.
// This software is licensed under a 3-clause BSD license. Please consult the LICENSE.md file
// distributed with the sources of this project regarding your rights to use or distribute this
// software.
Expand Down Expand Up @@ -26,27 +26,26 @@ type dsseEncoder struct {
payloadType string
}

// newDSSEEncoder returns an encoder that signs messages in DSSE format according to opts, with key
// material from ss. SHA256 is used as the hash algorithm, unless overridden by opts.
func newDSSEEncoder(ss []signature.Signer, opts ...signature.SignOption) (*dsseEncoder, error) {
var so crypto.SignerOpts
for _, opt := range opts {
opt.ApplyCryptoSignerOpts(&so)
}
var errMultipleHashes = errors.New("multiple hash algorithms specified")

// If SignerOpts not explicitly supplied, set default hash algorithm.
if so == nil {
so = crypto.SHA256
opts = append(opts, options.WithCryptoSignerOpts(so))
}
// newDSSEEncoder returns an encoder that signs messages in DSSE format, with key material from ss.
func newDSSEEncoder(ss ...signature.Signer) (*dsseEncoder, error) {
var h crypto.Hash

dss := make([]dsse.SignVerifier, 0, len(ss))
for _, s := range ss {
ds, err := newDSSESigner(s, opts...)
for i, s := range ss {
ds, err := newDSSESigner(s)
if err != nil {
return nil, err
}

// All signers must use the same hash, since the descriptor can only express one value.
if i == 0 {
h = ds.HashFunc()
} else if h != ds.HashFunc() {
return nil, errMultipleHashes
}

dss = append(dss, ds)
}

Expand All @@ -57,7 +56,7 @@ func newDSSEEncoder(ss []signature.Signer, opts ...signature.SignOption) (*dsseE

return &dsseEncoder{
es: es,
h: so.HashFunc(),
h: h,
payloadType: metadataMediaType,
}, nil
}
Expand Down Expand Up @@ -137,12 +136,23 @@ func (de *dsseDecoder) verifyMessage(r io.Reader, h crypto.Hash, vr *VerifyResul
type dsseSigner struct {
s signature.Signer
opts []signature.SignOption
h crypto.Hash
pub crypto.PublicKey
}

// newDSSESigner returns a dsse.SignVerifier that uses s to sign according to opts. Note that the
// returned value is suitable only for signing, and not verification.
func newDSSESigner(s signature.Signer, opts ...signature.SignOption) (*dsseSigner, error) {
// newDSSESigner returns a dsse.SignVerifier that uses s to sign. The SHA-256 hash algorithm is
// used unless s implements the crypto.SignerOpts interface and specifies an alternative algorithm.
// Note that the returned value is suitable only for signing, and not verification.
func newDSSESigner(s signature.Signer) (*dsseSigner, error) {
var opts []signature.SignOption

so, ok := s.(crypto.SignerOpts)
if !ok {
// Unable to determine hash algorithm used by signer, so override with SHA256.
so = crypto.SHA256
opts = append(opts, options.WithCryptoSignerOpts(so))
}

pub, err := s.PublicKey()
if err != nil {
return nil, err
Expand All @@ -151,6 +161,7 @@ func newDSSESigner(s signature.Signer, opts ...signature.SignOption) (*dsseSigne
return &dsseSigner{
s: s,
opts: opts,
h: so.HashFunc(),
pub: pub,
}, nil
}
Expand All @@ -160,6 +171,12 @@ func (s *dsseSigner) Sign(data []byte) ([]byte, error) {
return s.s.SignMessage(bytes.NewReader(data), s.opts...)
}

// HashFunc returns an identifier for the hash function used to produce the message passed to
// Signer.Sign, or else zero to indicate no hashing.
func (s *dsseSigner) HashFunc() crypto.Hash {
return s.h
}

var errSignNotImplemented = errors.New("sign not implemented")

// Verify is not implemented, but required for the dsse.SignVerifier interface.
Expand Down
74 changes: 32 additions & 42 deletions pkg/integrity/dsse_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,23 @@ import (
"github.com/sebdah/goldie/v2"
"github.com/secure-systems-lab/go-securesystemslib/dsse"
"github.com/sigstore/sigstore/pkg/signature"
"github.com/sigstore/sigstore/pkg/signature/options"
)

func Test_dsseEncoder_signMessage(t *testing.T) {
tests := []struct {
name string
signers []signature.Signer
signOpts []signature.SignOption
wantErr bool
wantErr error
wantHash crypto.Hash
}{
{
name: "MultipleHashAlgorithms",
signers: []signature.Signer{
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA384),
},
wantErr: errMultipleHashes,
},
{
name: "Multi",
signers: []signature.Signer{
Expand All @@ -40,37 +46,28 @@ func Test_dsseEncoder_signMessage(t *testing.T) {
{
name: "ED25519",
signers: []signature.Signer{
getTestSigner(t, "ed25519-private.pem", crypto.Hash(0)),
},
signOpts: []signature.SignOption{
options.WithCryptoSignerOpts(crypto.Hash(0)),
getTestSignerWithOpts(t, "ed25519-private.pem", crypto.Hash(0)),
},
wantHash: crypto.Hash(0),
},
{
name: "RSA_SHA256",
signers: []signature.Signer{
getTestSigner(t, "rsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA256),
},
wantHash: crypto.SHA256,
},
{
name: "RSA_SHA384",
signers: []signature.Signer{
getTestSigner(t, "rsa-private.pem", crypto.SHA384),
},
signOpts: []signature.SignOption{
options.WithCryptoSignerOpts(crypto.SHA384),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA384),
},
wantHash: crypto.SHA384,
},
{
name: "RSA_SHA512",
signers: []signature.Signer{
getTestSigner(t, "rsa-private.pem", crypto.SHA512),
},
signOpts: []signature.SignOption{
options.WithCryptoSignerOpts(crypto.SHA512),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA512),
},
wantHash: crypto.SHA512,
},
Expand All @@ -81,17 +78,17 @@ func Test_dsseEncoder_signMessage(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
b := bytes.Buffer{}

en, err := newDSSEEncoder(tt.signers, tt.signOpts...)
if err != nil {
t.Fatal(err)
}

ht, err := en.signMessage(&b, strings.NewReader(testMessage))
if got, want := err, tt.wantErr; (got != nil) != want {
en, err := newDSSEEncoder(tt.signers...)
if got, want := err, tt.wantErr; !errors.Is(got, want) {
t.Fatalf("got error %v, wantErr %v", got, want)
}

if err == nil {
ht, err := en.signMessage(&b, strings.NewReader(testMessage))
if err != nil {
t.Fatal(err)
}

if got, want := ht, tt.wantHash; got != want {
t.Errorf("got hash %v, want %v", got, want)
}
Expand Down Expand Up @@ -149,7 +146,6 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
tests := []struct {
name string
signers []signature.Signer
signOpts []signature.SignOption
corrupter func(*testing.T, *dsseEncoder, *dsse.Envelope)
de *dsseDecoder
wantErr error
Expand Down Expand Up @@ -197,8 +193,8 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "Multi_SHA256",
signers: []signature.Signer{
getTestSigner(t, "ecdsa-private.pem", crypto.SHA256),
getTestSigner(t, "rsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "ecdsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA256),
},
de: newDSSEDecoder(
getTestVerifier(t, "ecdsa-public.pem", crypto.SHA256),
Expand All @@ -213,8 +209,8 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "Multi_SHA256_ECDSA",
signers: []signature.Signer{
getTestSigner(t, "ecdsa-private.pem", crypto.SHA256),
getTestSigner(t, "rsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "ecdsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA256),
},
de: newDSSEDecoder(
getTestVerifier(t, "ecdsa-public.pem", crypto.SHA256),
Expand All @@ -227,8 +223,8 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "Multi_SHA256_RSA",
signers: []signature.Signer{
getTestSigner(t, "ecdsa-private.pem", crypto.SHA256),
getTestSigner(t, "rsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "ecdsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA256),
},
de: newDSSEDecoder(
getTestVerifier(t, "rsa-public.pem", crypto.SHA256),
Expand All @@ -241,7 +237,7 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "ECDSA_SHA256",
signers: []signature.Signer{
getTestSigner(t, "ecdsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "ecdsa-private.pem", crypto.SHA256),
},
de: newDSSEDecoder(
getTestVerifier(t, "ecdsa-public.pem", crypto.SHA256),
Expand All @@ -254,7 +250,7 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "ED25519",
signers: []signature.Signer{
getTestSigner(t, "ed25519-private.pem", crypto.Hash(0)),
getTestSignerWithOpts(t, "ed25519-private.pem", crypto.Hash(0)),
},
de: newDSSEDecoder(
getTestVerifier(t, "ed25519-public.pem", crypto.Hash(0)),
Expand All @@ -267,7 +263,7 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "RSA_SHA256",
signers: []signature.Signer{
getTestSigner(t, "rsa-private.pem", crypto.SHA256),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA256),
},
de: newDSSEDecoder(
getTestVerifier(t, "rsa-public.pem", crypto.SHA256),
Expand All @@ -280,10 +276,7 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "RSA_SHA384",
signers: []signature.Signer{
getTestSigner(t, "rsa-private.pem", crypto.SHA384),
},
signOpts: []signature.SignOption{
options.WithCryptoSignerOpts(crypto.SHA384),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA384),
},
de: newDSSEDecoder(
getTestVerifier(t, "rsa-public.pem", crypto.SHA384),
Expand All @@ -296,10 +289,7 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
{
name: "RSA_SHA512",
signers: []signature.Signer{
getTestSigner(t, "rsa-private.pem", crypto.SHA512),
},
signOpts: []signature.SignOption{
options.WithCryptoSignerOpts(crypto.SHA512),
getTestSignerWithOpts(t, "rsa-private.pem", crypto.SHA512),
},
de: newDSSEDecoder(
getTestVerifier(t, "rsa-public.pem", crypto.SHA512),
Expand All @@ -316,7 +306,7 @@ func Test_dsseDecoder_verifyMessage(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
b := bytes.Buffer{}

en, err := newDSSEEncoder(tt.signers, tt.signOpts...)
en, err := newDSSEEncoder(tt.signers...)
if err != nil {
t.Fatal(err)
}
Expand Down
18 changes: 18 additions & 0 deletions pkg/integrity/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,24 @@ func getTestSigner(t *testing.T, name string, h crypto.Hash) signature.Signer {
return sv
}

type wrapSigner struct {
signature.Signer
h crypto.Hash
}

func (s *wrapSigner) HashFunc() crypto.Hash { return s.h }

// getTestSignerWithOpts returns a Signer read from the PEM file at path, wrapped to implement the
// crypto.SignerOpts interface.
func getTestSignerWithOpts(t *testing.T, name string, h crypto.Hash) *wrapSigner {
t.Helper()

return &wrapSigner{
Signer: getTestSigner(t, name, h),
h: h,
}
}

// getTestVerifier returns a Verifier read from the PEM file at path.
func getTestVerifier(t *testing.T, name string, h crypto.Hash) signature.Verifier { //nolint:ireturn
t.Helper()
Expand Down
4 changes: 2 additions & 2 deletions pkg/integrity/sign.go
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020-2022, Sylabs Inc. All rights reserved.
// Copyright (c) 2020-2023, Sylabs Inc. All rights reserved.
// This software is licensed under a 3-clause BSD license. Please consult the LICENSE.md file
// distributed with the sources of this project regarding your rights to use or distribute this
// software.
Expand Down Expand Up @@ -330,7 +330,7 @@ func NewSigner(f *sif.FileImage, opts ...SignerOpt) (*Signer, error) {
switch {
case so.ss != nil:
var err error
en, err = newDSSEEncoder(so.ss)
en, err = newDSSEEncoder(so.ss...)
if err != nil {
return nil, fmt.Errorf("integrity: %w", err)
}
Expand Down

0 comments on commit b191ade

Please sign in to comment.