Skip to content

Commit

Permalink
Add support for ed25519ph user keys in hashedrekord (#1945)
Browse files Browse the repository at this point in the history
* Update sigstore/sigstore and use LoadOptions

Signed-off-by: Riccardo Schirone <[email protected]>

* Added support for Ed25519ph in HashedRekord entries

- Made X509 Signatures configurable with LoadOptions
- Removed existing check that limited the use of Ed25519 keys in
  HashedRekord entries
- Used Ed25519ph Signer/Verifier for HashedRekord entries

Signed-off-by: Riccardo Schirone <[email protected]>

* Fix entry tests to ensure ed25519 is now accepted

Signed-off-by: Riccardo Schirone <[email protected]>

---------

Signed-off-by: Riccardo Schirone <[email protected]>
  • Loading branch information
ret2libc authored Mar 4, 2024
1 parent 77be26b commit 92742be
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 28 deletions.
12 changes: 9 additions & 3 deletions pkg/pki/x509/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,23 @@ import (
var EmailAddressOID asn1.ObjectIdentifier = []int{1, 2, 840, 113549, 1, 9, 1}

type Signature struct {
signature []byte
signature []byte
verifierLoadOpts []sigsig.LoadOption
}

// NewSignature creates and validates an x509 signature object
func NewSignature(r io.Reader) (*Signature, error) {
return NewSignatureWithOpts(r)
}

func NewSignatureWithOpts(r io.Reader, opts ...sigsig.LoadOption) (*Signature, error) {
b, err := io.ReadAll(r)
if err != nil {
return nil, err
}
return &Signature{
signature: b,
signature: b,
verifierLoadOpts: opts,
}, nil
}

Expand Down Expand Up @@ -84,7 +90,7 @@ func (s Signature) Verify(r io.Reader, k interface{}, opts ...sigsig.VerifyOptio
}
}

verifier, err := sigsig.LoadVerifier(p, crypto.SHA256)
verifier, err := sigsig.LoadVerifierWithOpts(p, s.verifierLoadOpts...)
if err != nil {
return err
}
Expand Down
8 changes: 1 addition & 7 deletions pkg/types/hashedrekord/v0.0.1/entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"bytes"
"context"
"crypto"
"crypto/ed25519"
"crypto/sha256"
"encoding/hex"
"encoding/json"
Expand Down Expand Up @@ -148,7 +147,7 @@ func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) {
return nil, nil, types.ValidationError(errors.New("missing signature"))
}
// Hashed rekord type only works for x509 signature types
sigObj, err := x509.NewSignature(bytes.NewReader(sig.Content))
sigObj, err := x509.NewSignatureWithOpts(bytes.NewReader(sig.Content), options.WithED25519ph())
if err != nil {
return nil, nil, types.ValidationError(err)
}
Expand All @@ -162,11 +161,6 @@ func (v *V001Entry) validate() (pki.Signature, pki.PublicKey, error) {
return nil, nil, types.ValidationError(err)
}

_, isEd25519 := keyObj.CryptoPubKey().(ed25519.PublicKey)
if isEd25519 {
return nil, nil, types.ValidationError(errors.New("ed25519 unsupported for hashedrekord"))
}

data := v.HashedRekordObj.Data
if data == nil {
return nil, nil, types.ValidationError(errors.New("missing data"))
Expand Down
49 changes: 37 additions & 12 deletions pkg/types/hashedrekord/v0.0.1/entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/sigstore/rekor/pkg/types"
"github.com/sigstore/sigstore/pkg/cryptoutils"
"github.com/sigstore/sigstore/pkg/signature"
"github.com/sigstore/sigstore/pkg/signature/options"
"go.uber.org/goleak"
)

Expand Down Expand Up @@ -113,17 +114,17 @@ func TestCrossFieldValidation(t *testing.T) {
Type: "PUBLIC KEY",
})

// testing lack of support for ed25519
invalidEdPubKey, _, err := ed25519.GenerateKey(rand.Reader)
// testing support ed25519
edPubKey, edPrivKey, err := ed25519.GenerateKey(rand.Reader)
if err != nil {
t.Fatal(err)
}
invalidDer, err := x509.MarshalPKIXPublicKey(invalidEdPubKey)
edDer, err := x509.MarshalPKIXPublicKey(edPubKey)
if err != nil {
t.Fatal(err)
}
invalidKeyBytes := pem.EncodeToMemory(&pem.Block{
Bytes: invalidDer,
edPubKeyBytes := pem.EncodeToMemory(&pem.Block{
Bytes: edDer,
Type: "PUBLIC KEY",
})

Expand All @@ -142,6 +143,9 @@ func TestCrossFieldValidation(t *testing.T) {
sha512Signer, _ := signature.LoadSigner(key, crypto.SHA512)
sha512SigBytes, _ := sha512Signer.SignMessage(bytes.NewReader(dataBytes))

edsha512Signer, _ := signature.LoadSignerWithOpts(edPrivKey, options.WithHash(crypto.SHA512), options.WithED25519ph())
edsha512SigBytes, _ := edsha512Signer.SignMessage(bytes.NewReader(dataBytes))

incorrectLengthHash := sha256.Sum224(dataBytes)
incorrectLengthSHA := hex.EncodeToString(incorrectLengthHash[:])

Expand Down Expand Up @@ -197,16 +201,15 @@ func TestCrossFieldValidation(t *testing.T) {
entry: V001Entry{
HashedRekordObj: models.HashedrekordV001Schema{
Signature: &models.HashedrekordV001SchemaSignature{
Content: sha256SigBytes,
Content: edsha512SigBytes,
PublicKey: &models.HashedrekordV001SchemaSignaturePublicKey{
Content: invalidKeyBytes,
Content: edPubKeyBytes,
},
},
},
},
expectedHashValue: "sha256:" + dataSHA256,
expectUnmarshalSuccess: false,
// successful even if unmarshalling fails, because the ed25519 key is valid
expectedHashValue: "sha512:" + dataSHA512,
expectUnmarshalSuccess: false,
expectedVerifierSuccess: true,
},
{
Expand Down Expand Up @@ -242,6 +245,29 @@ func TestCrossFieldValidation(t *testing.T) {
expectUnmarshalSuccess: false,
expectedVerifierSuccess: true,
},
{
caseDesc: "signature with ed25519 public key (with data)",
entry: V001Entry{
HashedRekordObj: models.HashedrekordV001Schema{
Signature: &models.HashedrekordV001SchemaSignature{
Content: edsha512SigBytes,
PublicKey: &models.HashedrekordV001SchemaSignaturePublicKey{
Content: edPubKeyBytes,
},
},
Data: &models.HashedrekordV001SchemaData{
Hash: &models.HashedrekordV001SchemaDataHash{
Algorithm: swag.String(models.HashedrekordV001SchemaDataHashAlgorithmSha512),
Value: swag.String(dataSHA512),
},
},
},
},
expectedHashValue: "sha512:" + dataSHA512,
expectUnmarshalSuccess: true,
expectCanonicalizeSuccess: true,
expectedVerifierSuccess: true,
},
{
caseDesc: "signature with sha256 hash",
entry: V001Entry{
Expand Down Expand Up @@ -457,8 +483,7 @@ func TestCrossFieldValidation(t *testing.T) {
t.Errorf("%v: unexpected error, got %v", tc.caseDesc, err)
} else {
pub, _ := verifiers[0].CanonicalValue()
// invalidKeyBytes is a valid ed25519 key
if !reflect.DeepEqual(pub, keyBytes) && !reflect.DeepEqual(pub, invalidKeyBytes) {
if !reflect.DeepEqual(pub, keyBytes) && !reflect.DeepEqual(pub, edPubKeyBytes) {
t.Errorf("verifier and public keys do not match: %v, %v", string(pub), string(keyBytes))
}
}
Expand Down
14 changes: 8 additions & 6 deletions pkg/util/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,20 +310,22 @@ func TestSigningRoundtripCheckpoint(t *testing.T) {
if err != nil {
t.Fatalf("error creating signed checkpoint")
}
signer, _ := signature.LoadSigner(test.signer, crypto.SHA256)
if _, ok := test.signer.(*rsa.PrivateKey); ok {
signer, _ = signature.LoadRSAPSSSigner(test.signer.(*rsa.PrivateKey), crypto.SHA256, test.opts.(*rsa.PSSOptions))
signerOpts := []signature.LoadOption{options.WithHash(crypto.SHA256)}
if rsaTestOpts, ok := test.opts.(*rsa.PSSOptions); ok && rsaTestOpts != nil {
signerOpts = append(signerOpts, options.WithRSAPSS(rsaTestOpts))
}
signer, _ := signature.LoadSignerWithOpts(test.signer, signerOpts...)

_, err = sth.Sign(test.identity, signer, options.WithCryptoSignerOpts(test.opts))
if (err != nil) != test.wantSignErr {
t.Fatalf("signing test failed: wantSignErr %v, err %v", test.wantSignErr, err)
}
if !test.wantSignErr {
verifier, _ := signature.LoadVerifier(test.pubKey, crypto.SHA256)
if _, ok := test.pubKey.(*rsa.PublicKey); ok {
verifier, _ = signature.LoadRSAPSSVerifier(test.pubKey.(*rsa.PublicKey), crypto.SHA256, test.opts.(*rsa.PSSOptions))
verifierOpts := []signature.LoadOption{options.WithHash(crypto.SHA256)}
if rsaTestOpts, ok := test.opts.(*rsa.PSSOptions); ok && rsaTestOpts != nil {
verifierOpts = append(verifierOpts, options.WithRSAPSS(rsaTestOpts))
}
verifier, _ := signature.LoadVerifierWithOpts(test.pubKey, verifierOpts...)

if !sth.Verify(verifier) != test.wantVerifyErr {
t.Fatalf("verification test failed %v", sth.Verify(verifier))
Expand Down

0 comments on commit 92742be

Please sign in to comment.