From 15d36d7fb2a04da4bf020485dd34f848d4216719 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Wed, 11 Aug 2021 23:30:24 -0400 Subject: [PATCH 01/10] [network] translate libp2p.PubKey -> flow ECDSA PubKey This will allow us to perform identity checks for e.g. matching protocol-level identities to the proven network-level IDs --- network/p2p/keyTranslator.go | 61 ++++++++++++++++++++++++++++++- network/p2p/keyTranslator_test.go | 27 ++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/network/p2p/keyTranslator.go b/network/p2p/keyTranslator.go index bf4e27dfb3a..2654fc31279 100644 --- a/network/p2p/keyTranslator.go +++ b/network/p2p/keyTranslator.go @@ -11,8 +11,19 @@ import ( "github.com/onflow/flow-go/crypto" fcrypto "github.com/onflow/flow-go/crypto" + + "github.com/btcsuite/btcd/btcec" ) +// This module is meant to help libp2p <-> flow public key conversions +// Flow's Network Public Keys and LibP2P's public keys are a marshalling standard away from each other and inter-convertible. +// Libp2p supports keys as ECDSA public Keys on either the NIST P-256 curve or the "Bitcoin" secp256k1 curve, see https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md#peer-ids +// libp2p represents the P-256 keys as ASN1-DER and the secp256k1 keys as X9.62 encodings in compressed format +// +// While Flow's key types supports both secp256k1 and P-256 keys (under crypto/ecdsa), note that Flow's networking keys are always P-256 keys. +// Flow represents both the P-256 keys and the secp256k1 keys in uncompressed representation, but their byte serializations (Encode) do not include the X9.62 compression bit +// Flow also makes a X9.62 compressed format (with compression bit) accessible (EncodeCompressed) + // assigns a big.Int input to a Go ecdsa private key func setPrKey(c elliptic.Curve, k *big.Int) *goecdsa.PrivateKey { priv := new(goecdsa.PrivateKey) @@ -92,13 +103,61 @@ func PublicKey(fpk fcrypto.PublicKey) (lcrypto.PubKey, error) { } } else if keyType == lcrypto_pb.KeyType_Secp256k1 { bytes = make([]byte, crypto.PubKeyLenECDSASecp256k1+1) // libp2p requires an extra byte - bytes[0] = 4 // magic number in libp2p to refer to an uncompressed key + bytes[0] = 4 // signals uncompressed form as specified in section 4.3.6/7 of ANSI X9.62. copy(bytes[1:], tempBytes) } return um(bytes) } +// This converts some libp2p PubKeys to a flow PublicKey +// - the supported key types are ECDSA P-256 and ECDSA Secp256k1 public keys, +// - libp2p also supports RSA and Ed25519 keys, which Flow doesn't, their conversion will return an error. +func PublicKeyFromNetwork(lpk lcrypto.PubKey) (fcrypto.PublicKey, error) { + + switch ktype := lpk.Type(); ktype { + case lcrypto_pb.KeyType_ECDSA: + pubB, err := lpk.Raw() + if err != nil { + return nil, lcrypto.ErrBadKeyType + } + key, err := x509.ParsePKIXPublicKey(pubB) + if err != nil { + // impossible to decode from ASN1.DER + return nil, lcrypto.ErrBadKeyType + } + cryptoKey, ok := key.(*goecdsa.PublicKey) + if !ok { + // not recognized as crypto.P-256 + return nil, lcrypto.ErrNotECDSAPubKey + } + // ferrying through DecodePublicKey to get the curve checks + pk_uncompressed := elliptic.Marshal(cryptoKey, cryptoKey.X, cryptoKey.Y) + // the first bit is the compression bit of X9.62 + pubKey, err := crypto.DecodePublicKey(crypto.ECDSAP256, pk_uncompressed[1:]) + if err != nil { + return nil, lcrypto.ErrNotECDSAPubKey + } + return pubKey, nil + case lcrypto_pb.KeyType_Secp256k1: + // libp2p uses the compressed representation, flow the uncompressed one + lpk_secp256k1, ok := lpk.(*lcrypto.Secp256k1PublicKey) + if !ok { + return nil, lcrypto.ErrBadKeyType + } + pk_uncompressed := (*btcec.PublicKey)(lpk_secp256k1).SerializeUncompressed() + // the first bit is the compression bit of X9.62 + pk, err := crypto.DecodePublicKey(crypto.ECDSASecp256k1, pk_uncompressed[1:]) + if err != nil { + return nil, lcrypto.ErrNotECDSAPubKey + } + return pk, nil + default: + return nil, lcrypto.ErrBadKeyType + } + +} + // keyType translates Flow signing algorithm constants to the corresponding LibP2P constants func keyType(sa fcrypto.SigningAlgorithm) (lcrypto_pb.KeyType, error) { switch sa { diff --git a/network/p2p/keyTranslator_test.go b/network/p2p/keyTranslator_test.go index 2d9cdf8fc3d..77828516588 100644 --- a/network/p2p/keyTranslator_test.go +++ b/network/p2p/keyTranslator_test.go @@ -110,6 +110,33 @@ func (k *KeyTranslatorTestSuite) TestPublicKeyConversion() { } } +func (k *KeyTranslatorTestSuite) TestPublicKeyRoundTrip() { + sa := []fcrypto.SigningAlgorithm{fcrypto.ECDSAP256, fcrypto.ECDSASecp256k1} + loops := 50 + for _, s := range sa { + for i := 0; i < loops; i++ { + + // generate seed + seed := k.createSeed() + fpk, err := fcrypto.GeneratePrivateKey(s, seed) + require.NoError(k.T(), err) + + // get the Flow public key + fpublic := fpk.PublicKey() + + // convert the Flow public key to a Libp2p public key + lpublic, err := PublicKey(fpublic) + require.NoError(k.T(), err) + + fpublic2, err := PublicKeyFromNetwork(lpublic) + require.NoError(k.T(), err) + require.Equal(k.T(), fpublic, fpublic2) + + } + } + +} + // TestLibP2PIDGenerationIsConsistent tests that a LibP2P peer ID generated using Flow ECDSA key is deterministic func (k *KeyTranslatorTestSuite) TestPeerIDGenerationIsConsistent() { // generate a seed which will be used for both - Flow keys and Libp2p keys From 253ef557f503daa82dcdecfb2cf83f1f6d1550c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Wed, 11 Aug 2021 23:30:27 -0400 Subject: [PATCH 02/10] [crypto] Implement Compressed De/Serialization for supported ECDSA curves --- crypto/ecdsa.go | 73 ++++++++++++++++++++++++++++++++++++++------ crypto/ecdsa_test.go | 60 ++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 10 deletions(-) diff --git a/crypto/ecdsa.go b/crypto/ecdsa.go index fa84b0f7e86..ab3a792c24a 100644 --- a/crypto/ecdsa.go +++ b/crypto/ecdsa.go @@ -8,16 +8,18 @@ package crypto // This implementation does not include any security against side-channel attacks. import ( + "crypto/ecdsa" goecdsa "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "fmt" "math/big" + "github.com/btcsuite/btcd/btcec" "github.com/onflow/flow-go/crypto/hash" ) -// ecdsaAlgo embeds SignAlgo +// EcdsaAlgo embeds SignAlgo type ecdsaAlgo struct { // elliptic curve curve elliptic.Curve @@ -62,7 +64,7 @@ func (sk *PrKeyECDSA) signHash(h hash.Hash) (Signature, error) { // where r and s are padded to the curve order size. func (sk *PrKeyECDSA) Sign(data []byte, alg hash.Hasher) (Signature, error) { // no need to check the hasher output size as all supported hash algos - // have at lease 32 bytes output + // have at least 32 bytes output if alg == nil { return nil, newInvalidInputsError("Sign requires a Hasher") } @@ -200,9 +202,16 @@ func (a *ecdsaAlgo) decodePrivateKey(der []byte) (PrivateKey, error) { return a.rawDecodePrivateKey(der) } +func checkPublicKeyValid(pk *ecdsa.PublicKey) bool { + p := pk.Params().P + + // all the curves supported for now have a cofactor equal to 1, + // so that IsOnCurve guarantees the point is on the right subgroup. + return pk.X.Cmp(p) < 0 && pk.Y.Cmp(p) < 0 && pk.IsOnCurve(pk.X, pk.Y) +} + func (a *ecdsaAlgo) rawDecodePublicKey(der []byte) (PublicKey, error) { - p := (a.curve.Params().P) - plen := bitsToBytes(p.BitLen()) + plen := bitsToBytes(a.curve.Params().P.BitLen()) if len(der) != 2*plen { return nil, newInvalidInputsError("input has incorrect %s key size", a.algo) } @@ -210,17 +219,16 @@ func (a *ecdsaAlgo) rawDecodePublicKey(der []byte) (PublicKey, error) { x.SetBytes(der[:plen]) y.SetBytes(der[plen:]) - // all the curves supported for now have a cofactor equal to 1, - // so that IsOnCurve guarantees the point is on the right subgroup. - if x.Cmp(p) >= 0 || y.Cmp(p) >= 0 || !a.curve.IsOnCurve(&x, &y) { - return nil, newInvalidInputsError("input is not a valid %s key", a.algo) - } - pk := goecdsa.PublicKey{ Curve: a.curve, X: &x, Y: &y, } + + if !checkPublicKeyValid(&pk) { + return nil, newInvalidInputsError("input is not a valid %s key", a.algo) + } + return &PubKeyECDSA{a, &pk}, nil } @@ -316,6 +324,51 @@ func (pk *PubKeyECDSA) Size() int { return 2 * bitsToBytes((pk.goPubKey.Params().P).BitLen()) } +// given a public key (x,y), returns a compressed encoding according to X9.62 section 4.3.6 +// this compressed representation uses an extra bit to disambiguate sign +func (pk *PubKeyECDSA) encodeCompressed() []byte { + if pk.alg.curve == btcec.S256() { + return (*btcec.PublicKey)(pk.goPubKey).SerializeCompressed() + } + return elliptic.MarshalCompressed(pk.goPubKey, pk.goPubKey.X, pk.goPubKey.Y) +} + +// given a compressed public key according to X9.62 section 4.3.6, returns +// this compressed representation uses an extra bit to disambiguate sign +func decodeCompressed(ec elliptic.Curve, pkBytes []byte) (*PubKeyECDSA, error) { + if len(pkBytes) != PubKeyLenECDSAP256/2+1 { + return nil, newInvalidInputsError("input length incompatible with a 128-bits security ECDSA key in compressed form") + } + var keyAlgo *ecdsaAlgo + var goPubKey *ecdsa.PublicKey + + if ec == elliptic.P256() { + keyAlgo = p256Instance + x, y := elliptic.UnmarshalCompressed(keyAlgo.curve, pkBytes) + if x == nil { + return nil, newInvalidInputsError("Key %x can't be interpreted as %v", pkBytes, keyAlgo.algo.String()) + } + goPubKey = new(goecdsa.PublicKey) + goPubKey.Curve = keyAlgo.curve + goPubKey.X = x + goPubKey.Y = y + + } else if ec == btcec.S256() { + keyAlgo = secp256k1Instance + pk, err := btcec.ParsePubKey(pkBytes, btcec.S256()) + if err != nil { + return nil, newInvalidInputsError("Key %x can't be interpreted as %v", pkBytes, keyAlgo.algo.String()) + } + goPubKey = (*ecdsa.PublicKey)(pk) + } else { + return nil, newInvalidInputsError("Key type %v is not convertible to a 128-bits security ECDSA key", keyAlgo.algo.String()) + } + if !checkPublicKeyValid(goPubKey) { + return nil, newInvalidInputsError("input is not a valid %s key", keyAlgo.algo) + } + return &PubKeyECDSA{keyAlgo, goPubKey}, nil +} + // given a public key (x,y), returns a raw uncompressed encoding bytes(x)||bytes(y) // x and y are padded to the field size func (pk *PubKeyECDSA) rawEncode() []byte { diff --git a/crypto/ecdsa_test.go b/crypto/ecdsa_test.go index 7cc142b796f..6b26a26cbfb 100644 --- a/crypto/ecdsa_test.go +++ b/crypto/ecdsa_test.go @@ -1,17 +1,41 @@ package crypto import ( + "math" "testing" "crypto/elliptic" "crypto/rand" "math/big" + "github.com/btcsuite/btcd/btcec" "github.com/onflow/flow-go/crypto/hash" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func createSeed(t *testing.T) []byte { + seedLen := int(math.Max(KeyGenSeedMinLenECDSAP256, KeyGenSeedMinLenECDSASecp256k1)) + seed := make([]byte, seedLen) + n, err := rand.Read(seed) + require.NoError(t, err) + require.Equal(t, n, seedLen) + + return seed +} + +// keyType returns an elliptic.Curve for an fcrypto.SigningAlgorithm if it exists +func ecdsaCurveType(sa SigningAlgorithm) (elliptic.Curve, error) { + switch sa { + case ECDSAP256: + return elliptic.P256(), nil + case ECDSASecp256k1: + return btcec.S256(), nil + default: + return nil, newInvalidInputsError("Input does not correspond to an ECDSA Algorithm") + } +} + // ECDSA tests func TestECDSA(t *testing.T) { ecdsaCurves := []SigningAlgorithm{ @@ -284,3 +308,39 @@ func TestSignatureFormatCheck(t *testing.T) { }) } } + +func TestCompressionRoundTrip(t *testing.T) { + sa := []SigningAlgorithm{ECDSAP256, ECDSASecp256k1} + loops := 50 + for _, s := range sa { + ct, err := ecdsaCurveType(s) + require.NoError(t, err) + + for i := 0; i < loops; i++ { + + // generate seed + seed := createSeed(t) + fpk, err := GeneratePrivateKey(s, seed) + require.NoError(t, err) + + // get the Flow public key + fpublic := fpk.PublicKey() + + // retrieve the ECDSA variant + fpublicEcdsa := fpublic.(*PubKeyECDSA) + + // get the compressed bytes + fpublicBytes := fpublicEcdsa.encodeCompressed() + + // test the length (it's the same value for PubKeyLenECDSASecp256k1) + require.Len(t, fpublicBytes, PubKeyLenECDSAP256/2+1) + + // get the key back + fpublicOut, err := decodeCompressed(ct, fpublicBytes) + + require.NoError(t, err) + require.Equal(t, fpublicEcdsa, fpublicOut) + + } + } +} From 571668fc55c235eaf7fb0aad13add26627d65a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Wed, 11 Aug 2021 23:30:30 -0400 Subject: [PATCH 03/10] [crypto] Add EncodeCompressed, DecodepublicKeyCompressed to the signer API --- crypto/bls.go | 16 +++++++- crypto/ecdsa.go | 85 +++++++++++++++++++-------------------- crypto/ecdsa_test.go | 34 ++-------------- crypto/sign.go | 21 ++++++++-- crypto/sign_test_utils.go | 10 +++++ 5 files changed, 88 insertions(+), 78 deletions(-) diff --git a/crypto/bls.go b/crypto/bls.go index c9e82304faf..53373664b2d 100644 --- a/crypto/bls.go +++ b/crypto/bls.go @@ -211,8 +211,14 @@ func (a *blsBLS12381Algo) decodePrivateKey(privateKeyBytes []byte) (PrivateKey, } // decodePublicKey decodes a slice of bytes into a public key. -// This function includes a membership check in G2 and rejects the infinity point. +// since we use the compressed representation by default, this delegates to decodePublicKeyCompressed func (a *blsBLS12381Algo) decodePublicKey(publicKeyBytes []byte) (PublicKey, error) { + return a.decodePublicKeyCompressed(publicKeyBytes) +} + +// decodePublicKeyCompressed decodes a slice of bytes into a public key. +// This function includes a membership check in G2 and rejects the infinity point. +func (a *blsBLS12381Algo) decodePublicKeyCompressed(publicKeyBytes []byte) (PublicKey, error) { if len(publicKeyBytes) != pubKeyLengthBLSBLS12381 { return nil, newInvalidInputsError( "the input length has to be %d", @@ -339,12 +345,18 @@ func (pk *PubKeyBLSBLS12381) Size() int { // Encode returns a byte encoding of the public key. // The encoding is a compressed encoding of the point // [zcash] https://github.com/zkcrypto/pairing/blob/master/src/bls12_381/README.md#serialization -func (a *PubKeyBLSBLS12381) Encode() []byte { +func (a *PubKeyBLSBLS12381) EncodeCompressed() []byte { dest := make([]byte, pubKeyLengthBLSBLS12381) writePointG2(dest, &a.point) return dest } +// Encode returns a byte encoding of the public key. +// Since we use a compressed encoding by default, this delegates to EncodeCompressed +func (a *PubKeyBLSBLS12381) Encode() []byte { + return a.EncodeCompressed() +} + // Equals checks is two public keys are equal func (pk *PubKeyBLSBLS12381) Equals(other PublicKey) bool { otherBLS, ok := other.(*PubKeyBLSBLS12381) diff --git a/crypto/ecdsa.go b/crypto/ecdsa.go index ab3a792c24a..17baf82135f 100644 --- a/crypto/ecdsa.go +++ b/crypto/ecdsa.go @@ -8,7 +8,6 @@ package crypto // This implementation does not include any security against side-channel attacks. import ( - "crypto/ecdsa" goecdsa "crypto/ecdsa" "crypto/elliptic" "crypto/rand" @@ -19,7 +18,7 @@ import ( "github.com/onflow/flow-go/crypto/hash" ) -// EcdsaAlgo embeds SignAlgo +// ecdsaAlgo embeds SignAlgo type ecdsaAlgo struct { // elliptic curve curve elliptic.Curve @@ -202,7 +201,7 @@ func (a *ecdsaAlgo) decodePrivateKey(der []byte) (PrivateKey, error) { return a.rawDecodePrivateKey(der) } -func checkPublicKeyValid(pk *ecdsa.PublicKey) bool { +func checkPublicKeyValid(pk *goecdsa.PublicKey) bool { p := pk.Params().P // all the curves supported for now have a cofactor equal to 1, @@ -226,7 +225,7 @@ func (a *ecdsaAlgo) rawDecodePublicKey(der []byte) (PublicKey, error) { } if !checkPublicKeyValid(&pk) { - return nil, newInvalidInputsError("input is not a valid %s key", a.algo) + return nil, newInvalidInputsError("input %x is not a valid %s key", der, a.algo) } return &PubKeyECDSA{a, &pk}, nil @@ -236,6 +235,41 @@ func (a *ecdsaAlgo) decodePublicKey(der []byte) (PublicKey, error) { return a.rawDecodePublicKey(der) } +// decodePublicKeyCompressed returns a public key given the bytes of a compressed public key according to X9.62 section 4.3.6. +// this compressed representation uses an extra byte to disambiguate sign +func (a *ecdsaAlgo) decodePublicKeyCompressed(pkBytes []byte) (PublicKey, error) { + expectedLen := bitsToBytes(a.curve.Params().BitSize) + 1 + if len(pkBytes) != expectedLen { + return nil, newInvalidInputsError(fmt.Sprintf("input length incompatible, expected %d, got %d", expectedLen, len(pkBytes))) + } + var goPubKey *goecdsa.PublicKey + + if a.curve == elliptic.P256() { + x, y := elliptic.UnmarshalCompressed(a.curve, pkBytes) + if x == nil { + return nil, newInvalidInputsError("Key %x can't be interpreted as %v", pkBytes, a.algo.String()) + } + goPubKey = new(goecdsa.PublicKey) + goPubKey.Curve = a.curve + goPubKey.X = x + goPubKey.Y = y + + } else if a.curve == btcec.S256() { + pk, err := btcec.ParsePubKey(pkBytes, btcec.S256()) + if err != nil { + return nil, newInvalidInputsError("Key %x can't be interpreted as %v", pkBytes, a.algo.String()) + } + // This assertion never fails + goPubKey = (*goecdsa.PublicKey)(pk) + } else { + return nil, newInvalidInputsError("the input curve is not supported") + } + if !checkPublicKeyValid(goPubKey) { + return nil, newInvalidInputsError("input %x is not a valid %s key", pkBytes, a.algo) + } + return &PubKeyECDSA{a, goPubKey}, nil +} + // PrKeyECDSA is the private key of ECDSA, it implements the generic PrivateKey type PrKeyECDSA struct { // the signature algo @@ -324,51 +358,16 @@ func (pk *PubKeyECDSA) Size() int { return 2 * bitsToBytes((pk.goPubKey.Params().P).BitLen()) } -// given a public key (x,y), returns a compressed encoding according to X9.62 section 4.3.6 -// this compressed representation uses an extra bit to disambiguate sign -func (pk *PubKeyECDSA) encodeCompressed() []byte { +// EncodeCompressed returns a compressed encoding according to X9.62 section 4.3.6. +// This compressed representation uses an extra byte to disambiguate sign. +// The expected input is a public key (x,y). +func (pk *PubKeyECDSA) EncodeCompressed() []byte { if pk.alg.curve == btcec.S256() { return (*btcec.PublicKey)(pk.goPubKey).SerializeCompressed() } return elliptic.MarshalCompressed(pk.goPubKey, pk.goPubKey.X, pk.goPubKey.Y) } -// given a compressed public key according to X9.62 section 4.3.6, returns -// this compressed representation uses an extra bit to disambiguate sign -func decodeCompressed(ec elliptic.Curve, pkBytes []byte) (*PubKeyECDSA, error) { - if len(pkBytes) != PubKeyLenECDSAP256/2+1 { - return nil, newInvalidInputsError("input length incompatible with a 128-bits security ECDSA key in compressed form") - } - var keyAlgo *ecdsaAlgo - var goPubKey *ecdsa.PublicKey - - if ec == elliptic.P256() { - keyAlgo = p256Instance - x, y := elliptic.UnmarshalCompressed(keyAlgo.curve, pkBytes) - if x == nil { - return nil, newInvalidInputsError("Key %x can't be interpreted as %v", pkBytes, keyAlgo.algo.String()) - } - goPubKey = new(goecdsa.PublicKey) - goPubKey.Curve = keyAlgo.curve - goPubKey.X = x - goPubKey.Y = y - - } else if ec == btcec.S256() { - keyAlgo = secp256k1Instance - pk, err := btcec.ParsePubKey(pkBytes, btcec.S256()) - if err != nil { - return nil, newInvalidInputsError("Key %x can't be interpreted as %v", pkBytes, keyAlgo.algo.String()) - } - goPubKey = (*ecdsa.PublicKey)(pk) - } else { - return nil, newInvalidInputsError("Key type %v is not convertible to a 128-bits security ECDSA key", keyAlgo.algo.String()) - } - if !checkPublicKeyValid(goPubKey) { - return nil, newInvalidInputsError("input is not a valid %s key", keyAlgo.algo) - } - return &PubKeyECDSA{keyAlgo, goPubKey}, nil -} - // given a public key (x,y), returns a raw uncompressed encoding bytes(x)||bytes(y) // x and y are padded to the field size func (pk *PubKeyECDSA) rawEncode() []byte { diff --git a/crypto/ecdsa_test.go b/crypto/ecdsa_test.go index 6b26a26cbfb..657ed543895 100644 --- a/crypto/ecdsa_test.go +++ b/crypto/ecdsa_test.go @@ -8,34 +8,11 @@ import ( "crypto/rand" "math/big" - "github.com/btcsuite/btcd/btcec" "github.com/onflow/flow-go/crypto/hash" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func createSeed(t *testing.T) []byte { - seedLen := int(math.Max(KeyGenSeedMinLenECDSAP256, KeyGenSeedMinLenECDSASecp256k1)) - seed := make([]byte, seedLen) - n, err := rand.Read(seed) - require.NoError(t, err) - require.Equal(t, n, seedLen) - - return seed -} - -// keyType returns an elliptic.Curve for an fcrypto.SigningAlgorithm if it exists -func ecdsaCurveType(sa SigningAlgorithm) (elliptic.Curve, error) { - switch sa { - case ECDSAP256: - return elliptic.P256(), nil - case ECDSASecp256k1: - return btcec.S256(), nil - default: - return nil, newInvalidInputsError("Input does not correspond to an ECDSA Algorithm") - } -} - // ECDSA tests func TestECDSA(t *testing.T) { ecdsaCurves := []SigningAlgorithm{ @@ -313,7 +290,7 @@ func TestCompressionRoundTrip(t *testing.T) { sa := []SigningAlgorithm{ECDSAP256, ECDSASecp256k1} loops := 50 for _, s := range sa { - ct, err := ecdsaCurveType(s) + ct, err := ecdsaAlgoFromSA(s) require.NoError(t, err) for i := 0; i < loops; i++ { @@ -326,20 +303,17 @@ func TestCompressionRoundTrip(t *testing.T) { // get the Flow public key fpublic := fpk.PublicKey() - // retrieve the ECDSA variant - fpublicEcdsa := fpublic.(*PubKeyECDSA) - // get the compressed bytes - fpublicBytes := fpublicEcdsa.encodeCompressed() + fpublicBytes := fpublic.EncodeCompressed() // test the length (it's the same value for PubKeyLenECDSASecp256k1) require.Len(t, fpublicBytes, PubKeyLenECDSAP256/2+1) // get the key back - fpublicOut, err := decodeCompressed(ct, fpublicBytes) + fpublicOut, err := ct.decodePublicKeyCompressed(fpublicBytes) require.NoError(t, err) - require.Equal(t, fpublicEcdsa, fpublicOut) + require.Equal(t, fpublic, fpublicOut) } } diff --git a/crypto/sign.go b/crypto/sign.go index 40cd58a0d35..3585004cd27 100644 --- a/crypto/sign.go +++ b/crypto/sign.go @@ -16,12 +16,14 @@ import ( // Signer interface type signer interface { - // generatePrKey generates a private key + // generatePrivateKey generates a private key generatePrivateKey([]byte) (PrivateKey, error) - // decodePrKey loads a private key from a byte array + // decodePrivateKey loads a private key from a byte array decodePrivateKey([]byte) (PrivateKey, error) - // decodePubKey loads a public key from a byte array + // decodePublicKey loads a public key from a byte array decodePublicKey([]byte) (PublicKey, error) + // decodePublicKeyCompressed loads a public key from a byte array representing a point in compressed form + decodePublicKeyCompressed([]byte) (PublicKey, error) } // newNonRelicSigner returns a signer that does not depend on Relic library. @@ -107,6 +109,15 @@ func DecodePublicKey(algo SigningAlgorithm, data []byte) (PublicKey, error) { return signer.decodePublicKey(data) } +// DecodePublicKeyCompressed decodes an array of bytes given in a compressed representation into a public key of the given algorithm +func DecodePublicKeyCompressed(algo SigningAlgorithm, data []byte) (PublicKey, error) { + signer, err := newSigner(algo) + if err != nil { + return nil, newInvalidInputsError("decode public key failed: %s", err) + } + return signer.decodePublicKeyCompressed(data) +} + // Signature type tools // Bytes returns a byte array of the signature data @@ -153,6 +164,10 @@ type PublicKey interface { Verify(Signature, []byte, hash.Hasher) (bool, error) // Encode returns a bytes representation of the public key. Encode() []byte + // Encode returns a compressed byte representation of the public key. + // The compressed serialization concept is generic to elliptic curves, + // but we refer to individual curve parameters for details of the compressed format + EncodeCompressed() []byte // Equals returns true if the given PublicKeys are equal. Keys are considered unequal if their algorithms are // unequal or if their encoded representations are unequal. If the encoding of either key fails, they are considered // unequal as well. diff --git a/crypto/sign_test_utils.go b/crypto/sign_test_utils.go index 3f4d02769fd..47e06d1b84e 100644 --- a/crypto/sign_test_utils.go +++ b/crypto/sign_test_utils.go @@ -134,6 +134,16 @@ func testEncodeDecode(t *testing.T, salg SigningAlgorithm) { assert.Equal(t, pkBytes, pkCheckBytes, "keys should be equal") distinctPkBytes := distinctSk.PublicKey().Encode() assert.NotEqual(t, pkBytes, distinctPkBytes, "keys should be different") + + // same for the compressed encoding + pkComprBytes := pk.EncodeCompressed() + pkComprCheck, err := DecodePublicKeyCompressed(salg, pkComprBytes) + require.Nil(t, err, "the key decoding has failed") + assert.True(t, pk.Equals(pkComprCheck), "key equality check failed") + pkCheckComprBytes := pkComprCheck.EncodeCompressed() + assert.Equal(t, pkComprBytes, pkCheckComprBytes, "keys should be equal") + distinctPkComprBytes := distinctSk.PublicKey().EncodeCompressed() + assert.NotEqual(t, pkComprBytes, distinctPkComprBytes, "keys should be different") } // test invalid private keys (equal to the curve group order) From be0f6514d7d9172d8f1ae9d5c97bda950af483e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Wed, 11 Aug 2021 23:30:33 -0400 Subject: [PATCH 04/10] [network] Simplify libp2p -> flow after adding DecodepublicKeyCompressed --- network/p2p/keyTranslator.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/network/p2p/keyTranslator.go b/network/p2p/keyTranslator.go index 2654fc31279..78efbac1cc9 100644 --- a/network/p2p/keyTranslator.go +++ b/network/p2p/keyTranslator.go @@ -11,8 +11,6 @@ import ( "github.com/onflow/flow-go/crypto" fcrypto "github.com/onflow/flow-go/crypto" - - "github.com/btcsuite/btcd/btcec" ) // This module is meant to help libp2p <-> flow public key conversions @@ -145,9 +143,11 @@ func PublicKeyFromNetwork(lpk lcrypto.PubKey) (fcrypto.PublicKey, error) { if !ok { return nil, lcrypto.ErrBadKeyType } - pk_uncompressed := (*btcec.PublicKey)(lpk_secp256k1).SerializeUncompressed() - // the first bit is the compression bit of X9.62 - pk, err := crypto.DecodePublicKey(crypto.ECDSASecp256k1, pk_uncompressed[1:]) + secpBytes, err := lpk_secp256k1.Raw() + if err != nil { // this never errors + return nil, lcrypto.ErrBadKeyType + } + pk, err := crypto.DecodePublicKeyCompressed(crypto.ECDSASecp256k1, secpBytes) if err != nil { return nil, lcrypto.ErrNotECDSAPubKey } From a6ebfd93963b915510210c95ff7b02de23d91fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Wed, 11 Aug 2021 23:30:36 -0400 Subject: [PATCH 05/10] [network] re-activate message signing & verification ensure signerIDs are populated in pubSub tests --- network/p2p/libp2pNode.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/network/p2p/libp2pNode.go b/network/p2p/libp2pNode.go index 1df2536a3d6..fe6adc5c7e8 100644 --- a/network/p2p/libp2pNode.go +++ b/network/p2p/libp2pNode.go @@ -655,9 +655,9 @@ func DefaultPubsubOptions(maxPubSubMsgSize int) []PubsubOption { } return []PubsubOption{ // skip message signing - pubSubOptionFunc(pubsub.WithMessageSigning(false)), + pubSubOptionFunc(pubsub.WithMessageSigning(true)), // skip message signature - pubSubOptionFunc(pubsub.WithStrictSignatureVerification(false)), + pubSubOptionFunc(pubsub.WithStrictSignatureVerification(true)), // set max message size limit for 1-k PubSub messaging pubSubOptionFunc(pubsub.WithMaxMessageSize(maxPubSubMsgSize)), // no discovery From 141211d4d081f312963c8a130abf56f625ce701d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Thu, 12 Aug 2021 16:24:48 -0400 Subject: [PATCH 06/10] [network] Rename functions in keyTranslator for clarity --- network/p2p/keyTranslator.go | 6 +++--- network/p2p/keyTranslator_test.go | 10 +++++----- network/p2p/libp2pNode.go | 2 +- network/p2p/libp2pUtils.go | 2 +- network/p2p/libp2pUtils_test.go | 2 +- utils/grpc/grpc.go | 4 ++-- utils/grpc/grpc_test.go | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/network/p2p/keyTranslator.go b/network/p2p/keyTranslator.go index 78efbac1cc9..f6b106dcf48 100644 --- a/network/p2p/keyTranslator.go +++ b/network/p2p/keyTranslator.go @@ -44,7 +44,7 @@ func setPubKey(c elliptic.Curve, x *big.Int, y *big.Int) *goecdsa.PublicKey { // These utility functions convert a Flow crypto key to a LibP2P key (Flow --> LibP2P) // PrivKey converts a Flow private key to a LibP2P Private key -func PrivKey(fpk fcrypto.PrivateKey) (lcrypto.PrivKey, error) { +func LibP2PPrivKeyFromFlow(fpk fcrypto.PrivateKey) (lcrypto.PrivKey, error) { // get the signature algorithm keyType, err := keyType(fpk.Algorithm()) if err != nil { @@ -75,7 +75,7 @@ func PrivKey(fpk fcrypto.PrivateKey) (lcrypto.PrivKey, error) { } // PublicKey converts a Flow public key to a LibP2P public key -func PublicKey(fpk fcrypto.PublicKey) (lcrypto.PubKey, error) { +func LibP2PPublicKeyFromFlow(fpk fcrypto.PublicKey) (lcrypto.PubKey, error) { keyType, err := keyType(fpk.Algorithm()) if err != nil { return nil, err @@ -111,7 +111,7 @@ func PublicKey(fpk fcrypto.PublicKey) (lcrypto.PubKey, error) { // This converts some libp2p PubKeys to a flow PublicKey // - the supported key types are ECDSA P-256 and ECDSA Secp256k1 public keys, // - libp2p also supports RSA and Ed25519 keys, which Flow doesn't, their conversion will return an error. -func PublicKeyFromNetwork(lpk lcrypto.PubKey) (fcrypto.PublicKey, error) { +func FlowPublicKeyFromLibP2P(lpk lcrypto.PubKey) (fcrypto.PublicKey, error) { switch ktype := lpk.Type(); ktype { case lcrypto_pb.KeyType_ECDSA: diff --git a/network/p2p/keyTranslator_test.go b/network/p2p/keyTranslator_test.go index 77828516588..abd087d02dd 100644 --- a/network/p2p/keyTranslator_test.go +++ b/network/p2p/keyTranslator_test.go @@ -45,7 +45,7 @@ func (k *KeyTranslatorTestSuite) TestPrivateKeyConversion() { require.NoError(k.T(), err) // convert it to a LibP2P private key - lpk, err := PrivKey(fpk) + lpk, err := LibP2PPrivKeyFromFlow(fpk) require.NoError(k.T(), err) // get the raw bytes of both the keys @@ -91,7 +91,7 @@ func (k *KeyTranslatorTestSuite) TestPublicKeyConversion() { fpublic := fpk.PublicKey() // convert the Flow public key to a Libp2p public key - lpublic, err := PublicKey(fpublic) + lpublic, err := LibP2PPublicKeyFromFlow(fpublic) require.NoError(k.T(), err) // compare raw bytes of the public keys @@ -125,10 +125,10 @@ func (k *KeyTranslatorTestSuite) TestPublicKeyRoundTrip() { fpublic := fpk.PublicKey() // convert the Flow public key to a Libp2p public key - lpublic, err := PublicKey(fpublic) + lpublic, err := LibP2PPublicKeyFromFlow(fpublic) require.NoError(k.T(), err) - fpublic2, err := PublicKeyFromNetwork(lpublic) + fpublic2, err := FlowPublicKeyFromLibP2P(lpublic) require.NoError(k.T(), err) require.Equal(k.T(), fpublic, fpublic2) @@ -150,7 +150,7 @@ func (k *KeyTranslatorTestSuite) TestPeerIDGenerationIsConsistent() { fpublic := fpk.PublicKey() // convert it to the Libp2p Public key - lconverted, err := PublicKey(fpublic) + lconverted, err := LibP2PPublicKeyFromFlow(fpublic) require.NoError(k.T(), err) // check that the LibP2P Id generation is deterministic diff --git a/network/p2p/libp2pNode.go b/network/p2p/libp2pNode.go index fe6adc5c7e8..164c3fd18e8 100644 --- a/network/p2p/libp2pNode.go +++ b/network/p2p/libp2pNode.go @@ -598,7 +598,7 @@ func DefaultLibP2PHost(ctx context.Context, address string, key fcrypto.PrivateK // DefaultLibP2POptions creates and returns the standard LibP2P host options that are used for the Flow Libp2p network func DefaultLibP2POptions(address string, key fcrypto.PrivateKey) ([]config.Option, error) { - libp2pKey, err := PrivKey(key) + libp2pKey, err := LibP2PPrivKeyFromFlow(key) if err != nil { return nil, fmt.Errorf("could not generate libp2p key: %w", err) } diff --git a/network/p2p/libp2pUtils.go b/network/p2p/libp2pUtils.go index 7f758df79f3..ad43adbc877 100644 --- a/network/p2p/libp2pUtils.go +++ b/network/p2p/libp2pUtils.go @@ -106,7 +106,7 @@ func networkingInfo(identity flow.Identity) (string, string, crypto.PubKey, erro } // convert the Flow key to a LibP2P key - lkey, err := PublicKey(identity.NetworkPubKey) + lkey, err := LibP2PPublicKeyFromFlow(identity.NetworkPubKey) if err != nil { return "", "", nil, fmt.Errorf("could not convert flow key to libp2p key: %w", err) } diff --git a/network/p2p/libp2pUtils_test.go b/network/p2p/libp2pUtils_test.go index 4452f91de76..0660976329f 100644 --- a/network/p2p/libp2pUtils_test.go +++ b/network/p2p/libp2pUtils_test.go @@ -53,7 +53,7 @@ func idsAndPeerInfos(t *testing.T) (flow.IdentityList, []peer.AddrInfo) { ids[i] = id // create a libp2p PeerAddressInfo - libp2pKey, err := PublicKey(id.NetworkPubKey) + libp2pKey, err := LibP2PPublicKeyFromFlow(id.NetworkPubKey) assert.NoError(t, err) peerID, err := peer.IDFromPublicKey(libp2pKey) assert.NoError(t, err) diff --git a/utils/grpc/grpc.go b/utils/grpc/grpc.go index ceb8921289f..2fe22c59bde 100644 --- a/utils/grpc/grpc.go +++ b/utils/grpc/grpc.go @@ -24,7 +24,7 @@ const DefaultMaxMsgSize = 1024 * 1024 * 16 func X509Certificate(privKey crypto.PrivateKey) (*tls.Certificate, error) { // convert the Flow crypto private key to a Libp2p private crypto key - libP2PKey, err := p2p.PrivKey(privKey) + libP2PKey, err := p2p.LibP2PPrivKeyFromFlow(privKey) if err != nil { return nil, fmt.Errorf("could not convert Flow key to libp2p key: %w", err) } @@ -102,7 +102,7 @@ func DefaultClientTLSConfig(publicKey crypto.PublicKey) (*tls.Config, error) { func verifyPeerCertificateFunc(expectedPublicKey crypto.PublicKey) (func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error, error) { // convert the Flow.crypto key to LibP2P key for easy comparision using LibP2P TLS utils - expectedLibP2PKey, err := p2p.PublicKey(expectedPublicKey) + expectedLibP2PKey, err := p2p.LibP2PPublicKeyFromFlow(expectedPublicKey) if err != nil { return nil, fmt.Errorf("failed to generate a libp2p key from a Flow key: %w", err) } diff --git a/utils/grpc/grpc_test.go b/utils/grpc/grpc_test.go index 8f90564c8c3..3bb89d744a8 100644 --- a/utils/grpc/grpc_test.go +++ b/utils/grpc/grpc_test.go @@ -36,7 +36,7 @@ func TestCertificateGeneration(t *testing.T) { require.NoError(t, err) // convert the test key to a libp2p key for easy comparision - libp2pKey, err := p2p.PrivKey(key) + libp2pKey, err := p2p.LibP2PPrivKeyFromFlow(key) expectedKey := libp2pKey.GetPublic() require.NoError(t, err) From e4c0bc86a9895747857a2d580a4290e4dfba5b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Mon, 16 Aug 2021 14:52:24 -0400 Subject: [PATCH 07/10] [network] Lint UnsubscribeTest --- network/test/middleware_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/network/test/middleware_test.go b/network/test/middleware_test.go index 5584f2d0ee5..62715056af9 100644 --- a/network/test/middleware_test.go +++ b/network/test/middleware_test.go @@ -400,34 +400,30 @@ func (m *MiddlewareTestSuite) TestUnsubscribe() { } } - origin := 0 - target := m.size - 1 - - originID := m.ids[origin].NodeID message1 := createMessage(firstNode, lastNode, "hello1") - m.ov[target].On("Receive", originID, mockery.Anything).Return(nil).Once() + m.ov[last].On("Receive", firstNode, mockery.Anything).Return(nil).Once() // first test that when both nodes are subscribed to the channel, the target node receives the message - err := m.mws[origin].Publish(message1, testChannel) + err := m.mws[first].Publish(message1, testChannel) assert.NoError(m.T(), err) assert.Eventually(m.T(), func() bool { - return m.ov[target].AssertCalled(m.T(), "Receive", originID, mockery.Anything) + return m.ov[last].AssertCalled(m.T(), "Receive", firstNode, mockery.Anything) }, 2*time.Second, time.Millisecond) // now unsubscribe the target node from the channel - err = m.mws[target].Unsubscribe(testChannel) + err = m.mws[last].Unsubscribe(testChannel) assert.NoError(m.T(), err) // create and send a new message on the channel from the origin node message2 := createMessage(firstNode, lastNode, "hello2") - err = m.mws[origin].Publish(message2, testChannel) + err = m.mws[first].Publish(message2, testChannel) assert.NoError(m.T(), err) // assert that the new message is not received by the target node assert.Never(m.T(), func() bool { - return !m.ov[target].AssertNumberOfCalls(m.T(), "Receive", 1) + return !m.ov[last].AssertNumberOfCalls(m.T(), "Receive", 1) }, 2*time.Second, time.Millisecond) } From 47a0f2a63875479b25a771fb0236becc41b644d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 17 Aug 2021 11:40:29 -0400 Subject: [PATCH 08/10] [crypto] Add a test proving elliptic.UnmarshalCompressed does not support secp256k1 --- crypto/ecdsa_test.go | 47 ++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/crypto/ecdsa_test.go b/crypto/ecdsa_test.go index 657ed543895..2fa3d55b493 100644 --- a/crypto/ecdsa_test.go +++ b/crypto/ecdsa_test.go @@ -1,13 +1,14 @@ package crypto import ( - "math" + "encoding/hex" "testing" "crypto/elliptic" "crypto/rand" "math/big" + "github.com/btcsuite/btcd/btcec" "github.com/onflow/flow-go/crypto/hash" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -286,35 +287,35 @@ func TestSignatureFormatCheck(t *testing.T) { } } -func TestCompressionRoundTrip(t *testing.T) { - sa := []SigningAlgorithm{ECDSAP256, ECDSASecp256k1} - loops := 50 - for _, s := range sa { - ct, err := ecdsaAlgoFromSA(s) - require.NoError(t, err) +func TestEllipticUnmarshalSecp256k1(t *testing.T) { - for i := 0; i < loops; i++ { + testVectors := []string{ + "028b10bf56476bf7da39a3286e29df389177a2fa0fca2d73348ff78887515d8da1", // IsOnCurve for elliptic returns false + "03d39427f07f680d202fe8504306eb29041aceaf4b628c2c69b0ec248155443166", // negative, IsOnCurve for elliptic returns false + "0267d1942a6cbe4daec242ea7e01c6cdb82dadb6e7077092deb55c845bf851433e", // arith of sqrt in elliptic doesn't match secp256k1 + "0345d45eda6d087918b041453a96303b78c478dce89a4ae9b3c933a018888c5e06", // negative, arith of sqrt in elliptic doesn't match secp256k1 + } - // generate seed - seed := createSeed(t) - fpk, err := GeneratePrivateKey(s, seed) - require.NoError(t, err) + s := ECDSASecp256k1 - // get the Flow public key - fpublic := fpk.PublicKey() + for _, testVector := range testVectors { - // get the compressed bytes - fpublicBytes := fpublic.EncodeCompressed() + // get the compressed bytes + publicBytes, err := hex.DecodeString(testVector) + require.NoError(t, err) - // test the length (it's the same value for PubKeyLenECDSASecp256k1) - require.Len(t, fpublicBytes, PubKeyLenECDSAP256/2+1) + // decompress, check that those are perfectly valid Secp256k1 public keys + retrieved, err := DecodePublicKeyCompressed(s, publicBytes) + require.NoError(t, err) - // get the key back - fpublicOut, err := ct.decodePublicKeyCompressed(fpublicBytes) + // check the compression is canonical by re-compressing to the same bytes + require.Equal(t, retrieved.EncodeCompressed(), publicBytes) - require.NoError(t, err) - require.Equal(t, fpublic, fpublicOut) + // check that elliptic fails at decompressing them + x, y := elliptic.UnmarshalCompressed(btcec.S256(), publicBytes) + require.Nil(t, x) + require.Nil(t, y) - } } + } From b1ee91ad0f6ec5ae698a904c913086653e1716d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 17 Aug 2021 12:43:26 -0400 Subject: [PATCH 09/10] [crypto] Invert (encode|decode)PublicKey delegation for BLS - check for parametrization of compression --- crypto/bls.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/crypto/bls.go b/crypto/bls.go index 53373664b2d..db0976a5d2d 100644 --- a/crypto/bls.go +++ b/crypto/bls.go @@ -211,14 +211,8 @@ func (a *blsBLS12381Algo) decodePrivateKey(privateKeyBytes []byte) (PrivateKey, } // decodePublicKey decodes a slice of bytes into a public key. -// since we use the compressed representation by default, this delegates to decodePublicKeyCompressed -func (a *blsBLS12381Algo) decodePublicKey(publicKeyBytes []byte) (PublicKey, error) { - return a.decodePublicKeyCompressed(publicKeyBytes) -} - -// decodePublicKeyCompressed decodes a slice of bytes into a public key. // This function includes a membership check in G2 and rejects the infinity point. -func (a *blsBLS12381Algo) decodePublicKeyCompressed(publicKeyBytes []byte) (PublicKey, error) { +func (a *blsBLS12381Algo) decodePublicKey(publicKeyBytes []byte) (PublicKey, error) { if len(publicKeyBytes) != pubKeyLengthBLSBLS12381 { return nil, newInvalidInputsError( "the input length has to be %d", @@ -238,6 +232,15 @@ func (a *blsBLS12381Algo) decodePublicKeyCompressed(publicKeyBytes []byte) (Publ return &pk, nil } +// decodePublicKeyCompressed decodes a slice of bytes into a public key. +// since we use the compressed representation by default, this checks the default and delegates to decodePublicKeyCompressed +func (a *blsBLS12381Algo) decodePublicKeyCompressed(publicKeyBytes []byte) (PublicKey, error) { + if serializationG2 != compressed { + panic("library is not configured to use compressed public key serialization") + } + return a.decodePublicKey(publicKeyBytes) +} + // PrKeyBLSBLS12381 is the private key of BLS using BLS12_381, it implements PrivateKey type PrKeyBLSBLS12381 struct { // public key @@ -346,15 +349,18 @@ func (pk *PubKeyBLSBLS12381) Size() int { // The encoding is a compressed encoding of the point // [zcash] https://github.com/zkcrypto/pairing/blob/master/src/bls12_381/README.md#serialization func (a *PubKeyBLSBLS12381) EncodeCompressed() []byte { - dest := make([]byte, pubKeyLengthBLSBLS12381) - writePointG2(dest, &a.point) - return dest + if serializationG2 != compressed { + panic("library is not configured to use compressed public key serialization") + } + return a.Encode() } // Encode returns a byte encoding of the public key. // Since we use a compressed encoding by default, this delegates to EncodeCompressed func (a *PubKeyBLSBLS12381) Encode() []byte { - return a.EncodeCompressed() + dest := make([]byte, pubKeyLengthBLSBLS12381) + writePointG2(dest, &a.point) + return dest } // Equals checks is two public keys are equal From d1e58f6c66a58bbf0cffdc75af2ceae17f6d834e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Garillot?= Date: Tue, 17 Aug 2021 12:58:01 -0400 Subject: [PATCH 10/10] [crypto] partial revert of checkPublicKeyValid as existing DecodeCompressed implementations already check for validity --- crypto/ecdsa.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/crypto/ecdsa.go b/crypto/ecdsa.go index 17baf82135f..70d4a60fd44 100644 --- a/crypto/ecdsa.go +++ b/crypto/ecdsa.go @@ -201,16 +201,9 @@ func (a *ecdsaAlgo) decodePrivateKey(der []byte) (PrivateKey, error) { return a.rawDecodePrivateKey(der) } -func checkPublicKeyValid(pk *goecdsa.PublicKey) bool { - p := pk.Params().P - - // all the curves supported for now have a cofactor equal to 1, - // so that IsOnCurve guarantees the point is on the right subgroup. - return pk.X.Cmp(p) < 0 && pk.Y.Cmp(p) < 0 && pk.IsOnCurve(pk.X, pk.Y) -} - func (a *ecdsaAlgo) rawDecodePublicKey(der []byte) (PublicKey, error) { - plen := bitsToBytes(a.curve.Params().P.BitLen()) + p := (a.curve.Params().P) + plen := bitsToBytes(p.BitLen()) if len(der) != 2*plen { return nil, newInvalidInputsError("input has incorrect %s key size", a.algo) } @@ -218,16 +211,18 @@ func (a *ecdsaAlgo) rawDecodePublicKey(der []byte) (PublicKey, error) { x.SetBytes(der[:plen]) y.SetBytes(der[plen:]) + // all the curves supported for now have a cofactor equal to 1, + // so that IsOnCurve guarantees the point is on the right subgroup. + if x.Cmp(p) >= 0 || y.Cmp(p) >= 0 || !a.curve.IsOnCurve(&x, &y) { + return nil, newInvalidInputsError("input %x is not a valid %s key", der, a.algo) + } + pk := goecdsa.PublicKey{ Curve: a.curve, X: &x, Y: &y, } - if !checkPublicKeyValid(&pk) { - return nil, newInvalidInputsError("input %x is not a valid %s key", der, a.algo) - } - return &PubKeyECDSA{a, &pk}, nil } @@ -264,9 +259,6 @@ func (a *ecdsaAlgo) decodePublicKeyCompressed(pkBytes []byte) (PublicKey, error) } else { return nil, newInvalidInputsError("the input curve is not supported") } - if !checkPublicKeyValid(goPubKey) { - return nil, newInvalidInputsError("input %x is not a valid %s key", pkBytes, a.algo) - } return &PubKeyECDSA{a, goPubKey}, nil }