From b383cc527924308d55b143aa2eb9a15b76925495 Mon Sep 17 00:00:00 2001 From: Thibault Normand Date: Fri, 22 Apr 2022 17:30:12 +0200 Subject: [PATCH] feat(key): add crypto related controls. --- data/types.go | 8 +++++++- go.mod | 1 + go.sum | 2 ++ pkg/keys/ecdsa.go | 16 +++++++++++++++- pkg/keys/ecdsa_test.go | 41 ++++++++++++++++++++++++++++++++++++++-- pkg/keys/ed25519.go | 38 +++++++++++++++++++++++++++++++++++-- pkg/keys/ed25519_test.go | 35 +++++++++++++++++++++++++++++++++- 7 files changed, 134 insertions(+), 7 deletions(-) diff --git a/data/types.go b/data/types.go index 6ec2fc340..be1f0745b 100644 --- a/data/types.go +++ b/data/types.go @@ -1,6 +1,7 @@ package data import ( + "bytes" "crypto/sha256" "encoding/hex" "encoding/json" @@ -284,7 +285,12 @@ func (d *DelegatedRole) MarshalJSON() ([]byte, error) { func (d *DelegatedRole) UnmarshalJSON(b []byte) error { type delegatedRoleAlias DelegatedRole - if err := json.Unmarshal(b, (*delegatedRoleAlias)(d)); err != nil { + // Prepare decoder + dec := json.NewDecoder(bytes.NewReader(b)) + dec.DisallowUnknownFields() + + // Unmarshal delegated role + if err := dec.Decode((*delegatedRoleAlias)(d)); err != nil { return err } diff --git a/go.mod b/go.mod index 80d640b3c..e6067edc5 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.16 require ( github.com/dustin/go-humanize v1.0.0 github.com/flynn/go-docopt v0.0.0-20140912013429-f6dd2ebbb31e + github.com/google/gofuzz v1.2.0 github.com/onsi/gomega v1.18.1 // indirect github.com/secure-systems-lab/go-securesystemslib v0.3.1 github.com/stretchr/testify v1.7.1 diff --git a/go.sum b/go.sum index 0dbe845af..772abe2d3 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,8 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0= +github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= diff --git a/pkg/keys/ecdsa.go b/pkg/keys/ecdsa.go index 2443036ba..e638985c0 100644 --- a/pkg/keys/ecdsa.go +++ b/pkg/keys/ecdsa.go @@ -8,6 +8,7 @@ import ( "encoding/asn1" "encoding/json" "errors" + "fmt" "io" "math/big" @@ -67,14 +68,27 @@ func (p *p256Verifier) UnmarshalPublicKey(key *data.PublicKey) error { // Unmarshal key value if err := dec.Decode(p); err != nil { + switch { + case errors.Is(err, io.EOF), errors.Is(err, io.ErrUnexpectedEOF): + return fmt.Errorf("tuf: the public key is truncated or too large: %w", err) + default: + } return err } + curve := elliptic.P256() + // Parse as uncompressed marshalled point. - x, _ := elliptic.Unmarshal(elliptic.P256(), p.PublicKey) + x, y := elliptic.Unmarshal(curve, p.PublicKey) if x == nil { return errors.New("tuf: invalid ecdsa public key point") } + + // Check point + if !curve.IsOnCurve(x, y) { + return errors.New("tuf: ecdsa public key point is not on the associated curve") + } + p.key = key return nil } diff --git a/pkg/keys/ecdsa_test.go b/pkg/keys/ecdsa_test.go index b31f274d4..7bede7c75 100644 --- a/pkg/keys/ecdsa_test.go +++ b/pkg/keys/ecdsa_test.go @@ -1,10 +1,15 @@ package keys import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" + "encoding/hex" "encoding/json" "io" + "strings" + fuzz "github.com/google/gofuzz" "github.com/theupdateframework/go-tuf/data" . "gopkg.in/check.v1" ) @@ -13,7 +18,26 @@ type ECDSASuite struct{} var _ = Suite(&ECDSASuite{}) -func (ECDSASuite) TestUnmarshalECDSA(c *C) { +func (Ed25519Suite) TestUnmarshalECDSA(c *C) { + priv, _ := ecdsa.GenerateKey(elliptic.P256(), strings.NewReader("00001-deterministic-buffer-for-key-generation")) + + // Marshall as non compressed point + pub := elliptic.Marshal(elliptic.P256(), priv.X, priv.Y) + + publicKey, _ := json.Marshal(map[string]string{ + "public": hex.EncodeToString(pub), + }) + badKey := &data.PublicKey{ + Type: data.KeyTypeECDSA_SHA2_P256, + Scheme: data.KeySchemeECDSA_SHA2_P256, + Algorithms: data.HashAlgorithms, + Value: publicKey, + } + verifier := NewEcdsaVerifier() + c.Assert(verifier.UnmarshalPublicKey(badKey), IsNil) +} + +func (ECDSASuite) TestUnmarshalECDSA_Invalid(c *C) { badKeyValue, _ := json.Marshal(true) badKey := &data.PublicKey{ Type: data.KeyTypeECDSA_SHA2_P256, @@ -25,6 +49,19 @@ func (ECDSASuite) TestUnmarshalECDSA(c *C) { c.Assert(verifier.UnmarshalPublicKey(badKey), ErrorMatches, "json: cannot unmarshal.*") } +func (ECDSASuite) TestUnmarshalECDSA_FastFuzz(c *C) { + verifier := NewEcdsaVerifier() + for i := 0; i < 50; i++ { + // Ensure no basic panic + + f := fuzz.New() + var publicData data.PublicKey + f.Fuzz(&publicData) + + verifier.UnmarshalPublicKey(&publicData) + } +} + func (ECDSASuite) TestUnmarshalECDSA_TooLongContent(c *C) { randomSeed := make(json.RawMessage, 1024*1024) io.ReadFull(rand.Reader, randomSeed) @@ -42,5 +79,5 @@ func (ECDSASuite) TestUnmarshalECDSA_TooLongContent(c *C) { Value: tooLongPayload, } verifier := NewEcdsaVerifier() - c.Assert(verifier.UnmarshalPublicKey(badKey), ErrorMatches, "unexpected EOF") + c.Assert(verifier.UnmarshalPublicKey(badKey), ErrorMatches, ".*: unexpected EOF") } diff --git a/pkg/keys/ed25519.go b/pkg/keys/ed25519.go index bca9dc52d..2d92974c8 100644 --- a/pkg/keys/ed25519.go +++ b/pkg/keys/ed25519.go @@ -5,8 +5,10 @@ import ( "crypto" "crypto/ed25519" "crypto/rand" + "crypto/subtle" "encoding/json" "errors" + "fmt" "io" "github.com/theupdateframework/go-tuf/data" @@ -54,6 +56,11 @@ func (e *ed25519Verifier) UnmarshalPublicKey(key *data.PublicKey) error { // Unmarshal key value if err := dec.Decode(e); err != nil { + switch { + case errors.Is(err, io.EOF), errors.Is(err, io.ErrUnexpectedEOF): + return fmt.Errorf("tuf: the public key is truncated or too large: %w", err) + default: + } return err } if len(e.PublicKey) != ed25519.PublicKeySize { @@ -122,9 +129,36 @@ func (e *ed25519Signer) MarshalPrivateKey() (*data.PrivateKey, error) { func (e *ed25519Signer) UnmarshalPrivateKey(key *data.PrivateKey) error { keyValue := &Ed25519PrivateKeyValue{} - if err := json.Unmarshal(key.Value, keyValue); err != nil { - return err + + // Prepare decoder limited to 512Kb + dec := json.NewDecoder(io.LimitReader(bytes.NewReader(key.Value), 512*1024)) + dec.DisallowUnknownFields() + + if err := dec.Decode(keyValue); err != nil { + switch { + case errors.Is(err, io.EOF), errors.Is(err, io.ErrUnexpectedEOF): + return fmt.Errorf("tuf: the private key is truncated or too large: %w", err) + default: + } + } + + // Check private key length + if len(keyValue.Private) != ed25519.PrivateKeySize { + return errors.New("tuf: invalid ed25519 private key length") + } + + // Generate public key from private key + pub, _, err := ed25519.GenerateKey(bytes.NewReader(keyValue.Private)) + if err != nil { + return fmt.Errorf("tuf: unable to derive public key from private key: %w", err) } + + // Compare keys + if subtle.ConstantTimeCompare(keyValue.Public, pub) != 1 { + return errors.New("tuf: public and private keys doesn't match") + } + + // Prepare signer *e = ed25519Signer{ PrivateKey: ed25519.PrivateKey(data.HexBytes(keyValue.Private)), keyType: key.Type, diff --git a/pkg/keys/ed25519_test.go b/pkg/keys/ed25519_test.go index 639ffc329..2884ce4be 100644 --- a/pkg/keys/ed25519_test.go +++ b/pkg/keys/ed25519_test.go @@ -1,10 +1,14 @@ package keys import ( + "crypto/ed25519" "crypto/rand" + "encoding/hex" "encoding/json" "io" + "strings" + fuzz "github.com/google/gofuzz" "github.com/theupdateframework/go-tuf/data" . "gopkg.in/check.v1" ) @@ -14,6 +18,22 @@ type Ed25519Suite struct{} var _ = Suite(&Ed25519Suite{}) func (Ed25519Suite) TestUnmarshalEd25519(c *C) { + pub, _, _ := ed25519.GenerateKey(strings.NewReader("00001-deterministic-buffer-for-key-generation")) + + publicKey, _ := json.Marshal(map[string]string{ + "public": hex.EncodeToString(pub), + }) + badKey := &data.PublicKey{ + Type: data.KeyTypeEd25519, + Scheme: data.KeySchemeEd25519, + Algorithms: data.HashAlgorithms, + Value: publicKey, + } + verifier := NewEd25519Verifier() + c.Assert(verifier.UnmarshalPublicKey(badKey), IsNil) +} + +func (Ed25519Suite) TestUnmarshalEd25519_Invalid(c *C) { badKeyValue, _ := json.Marshal(true) badKey := &data.PublicKey{ Type: data.KeyTypeEd25519, @@ -25,6 +45,19 @@ func (Ed25519Suite) TestUnmarshalEd25519(c *C) { c.Assert(verifier.UnmarshalPublicKey(badKey), ErrorMatches, "json: cannot unmarshal.*") } +func (Ed25519Suite) TestUnmarshalEd25519_FastFuzz(c *C) { + verifier := NewEd25519Verifier() + for i := 0; i < 50; i++ { + // Ensure no basic panic + + f := fuzz.New() + var publicData data.PublicKey + f.Fuzz(&publicData) + + verifier.UnmarshalPublicKey(&publicData) + } +} + func (Ed25519Suite) TestUnmarshalEd25519_TooLongContent(c *C) { randomSeed := make(json.RawMessage, 1024*1024) io.ReadFull(rand.Reader, randomSeed) @@ -42,7 +75,7 @@ func (Ed25519Suite) TestUnmarshalEd25519_TooLongContent(c *C) { Value: tooLongPayload, } verifier := NewEd25519Verifier() - c.Assert(verifier.UnmarshalPublicKey(badKey), ErrorMatches, "unexpected EOF") + c.Assert(verifier.UnmarshalPublicKey(badKey), ErrorMatches, ".*: unexpected EOF") } func (Ed25519Suite) TestSignVerify(c *C) {