Merge "Maleability resistance for PKCS11 BCCSP"

bccsp/pkcs11/ecdsa.go

@@ -15,9 +15,103 @@ limitations under the License.
 */
 package pkcs11
 
-import "math/big"
+import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"encoding/asn1"
+	"errors"
+	"fmt"
+	"math/big"
+
+	"github.com/hyperledger/fabric/bccsp"
+)
 
-// ECDSASignature represents an ECDSA signature
 type ecdsaSignature struct {
 	R, S *big.Int
 }
+
+var (
+	// curveHalfOrders contains the precomputed curve group orders halved.
+	// It is used to ensure that signature' S value is lower or equal to the
+	// curve group order halved. We accept only low-S signatures.
+	// They are precomputed for efficiency reasons.
+	curveHalfOrders map[elliptic.Curve]*big.Int = map[elliptic.Curve]*big.Int{
+		elliptic.P224(): new(big.Int).Rsh(elliptic.P224().Params().N, 1),
+		elliptic.P256(): new(big.Int).Rsh(elliptic.P256().Params().N, 1),
+		elliptic.P384(): new(big.Int).Rsh(elliptic.P384().Params().N, 1),
+		elliptic.P521(): new(big.Int).Rsh(elliptic.P521().Params().N, 1),
+	}
+)
+
+func marshalECDSASignature(r, s *big.Int) ([]byte, error) {
+	return asn1.Marshal(ecdsaSignature{r, s})
+}
+
+func unmarshalECDSASignature(raw []byte) (*big.Int, *big.Int, error) {
+	// Unmarshal
+	sig := new(ecdsaSignature)
+	_, err := asn1.Unmarshal(raw, sig)
+	if err != nil {
+		return nil, nil, fmt.Errorf("Failed unmashalling signature [%s]", err)
+	}
+
+	// Validate sig
+	if sig.R == nil {
+		return nil, nil, errors.New("Invalid signature. R must be different from nil.")
+	}
+	if sig.S == nil {
+		return nil, nil, errors.New("Invalid signature. S must be different from nil.")
+	}
+
+	if sig.R.Sign() != 1 {
+		return nil, nil, errors.New("Invalid signature. R must be larger than zero")
+	}
+	if sig.S.Sign() != 1 {
+		return nil, nil, errors.New("Invalid signature. S must be larger than zero")
+	}
+
+	return sig.R, sig.S, nil
+}
+
+func (csp *impl) signECDSA(k *ecdsa.PrivateKey, digest []byte, opts bccsp.SignerOpts) (signature []byte, err error) {
+	r, s, err := ecdsa.Sign(rand.Reader, k, digest)
+	if err != nil {
+		return nil, err
+	}
+
+	// check for low-S
+	halfOrder, ok := curveHalfOrders[k.Curve]
+	if !ok {
+		return nil, fmt.Errorf("Curve not recognized [%s]", k.Curve)
+	}
+
+	// is s > halfOrder Then
+	if s.Cmp(halfOrder) == 1 {
+		// Set s to N - s that will be then in the lower part of signature space
+		// less or equal to half order
+		s.Sub(k.Params().N, s)
+	}
+
+	return marshalECDSASignature(r, s)
+}
+
+func (csp *impl) verifyECDSA(k *ecdsa.PublicKey, signature, digest []byte, opts bccsp.SignerOpts) (valid bool, err error) {
+	r, s, err := unmarshalECDSASignature(signature)
+	if err != nil {
+		return false, fmt.Errorf("Failed unmashalling signature [%s]", err)
+	}
+
+	// check for low-S
+	halfOrder, ok := curveHalfOrders[k.Curve]
+	if !ok {
+		return false, fmt.Errorf("Curve not recognized [%s]", k.Curve)
+	}
+
+	// If s > halfOrder Then
+	if s.Cmp(halfOrder) == 1 {
+		return false, fmt.Errorf("Invalid S. Must be smaller than half the order [%s][%s].", s, halfOrder)
+	}
+
+	return ecdsa.Verify(k, digest, r, s), nil
+}

bccsp/pkcs11/impl.go

@@ -18,7 +18,6 @@ package pkcs11
 import (
 	"crypto/ecdsa"
 	"crypto/rand"
-	"encoding/asn1"
 	"errors"
 	"fmt"
 	"math/big"
@@ -603,7 +602,7 @@ func (csp *impl) Sign(k bccsp.Key, digest []byte, opts bccsp.SignerOpts) (signat
 	// Check key type
 	switch k.(type) {
 	case *ecdsaPrivateKey:
-		return k.(*ecdsaPrivateKey).privKey.Sign(rand.Reader, digest, nil)
+		return csp.signECDSA(k.(*ecdsaPrivateKey).privKey, digest, opts)
 	case *rsaPrivateKey:
 		if opts == nil {
 			return nil, errors.New("Invalid options. Nil.")
@@ -631,21 +630,9 @@ func (csp *impl) Verify(k bccsp.Key, signature, digest []byte, opts bccsp.Signer
 	// Check key type
 	switch k.(type) {
 	case *ecdsaPrivateKey:
-		ecdsaSignature := new(ecdsaSignature)
-		_, err := asn1.Unmarshal(signature, ecdsaSignature)
-		if err != nil {
-			return false, fmt.Errorf("Failed unmashalling signature [%s]", err)
-		}
-
-		return ecdsa.Verify(&(k.(*ecdsaPrivateKey).privKey.PublicKey), digest, ecdsaSignature.R, ecdsaSignature.S), nil
+		return csp.verifyECDSA(&(k.(*ecdsaPrivateKey).privKey.PublicKey), signature, digest, opts)
 	case *ecdsaPublicKey:
-		ecdsaSignature := new(ecdsaSignature)
-		_, err := asn1.Unmarshal(signature, ecdsaSignature)
-		if err != nil {
-			return false, fmt.Errorf("Failed unmashalling signature [%s]", err)
-		}
-
-		return ecdsa.Verify(k.(*ecdsaPublicKey).pubKey, digest, ecdsaSignature.R, ecdsaSignature.S), nil
+		return csp.verifyECDSA(k.(*ecdsaPublicKey).pubKey, signature, digest, opts)
 	case *rsaPrivateKey:
 		if opts == nil {
 			return false, errors.New("Invalid options. It must not be nil.")

bccsp/pkcs11/impl_test.go

@@ -1014,6 +1014,106 @@ func TestKeyImportFromX509ECDSAPublicKey(t *testing.T) {
 	}
 }
 
+func TestECDSASignatureEncoding(t *testing.T) {
+	v := []byte{0x30, 0x07, 0x02, 0x01, 0x8F, 0x02, 0x02, 0xff, 0xf1}
+	_, err := asn1.Unmarshal(v, &ecdsaSignature{})
+	if err == nil {
+		t.Fatalf("Unmarshalling should fail for [% x]", v)
+	}
+	t.Logf("Unmarshalling correctly failed for [% x] [%s]", v, err)
+
+	v = []byte{0x30, 0x07, 0x02, 0x01, 0x8F, 0x02, 0x02, 0x00, 0x01}
+	_, err = asn1.Unmarshal(v, &ecdsaSignature{})
+	if err == nil {
+		t.Fatalf("Unmarshalling should fail for [% x]", v)
+	}
+	t.Logf("Unmarshalling correctly failed for [% x] [%s]", v, err)
+
+	v = []byte{0x30, 0x07, 0x02, 0x01, 0x8F, 0x02, 0x81, 0x01, 0x01}
+	_, err = asn1.Unmarshal(v, &ecdsaSignature{})
+	if err == nil {
+		t.Fatalf("Unmarshalling should fail for [% x]", v)
+	}
+	t.Logf("Unmarshalling correctly failed for [% x] [%s]", v, err)
+
+	v = []byte{0x30, 0x07, 0x02, 0x01, 0x8F, 0x02, 0x81, 0x01, 0x8F}
+	_, err = asn1.Unmarshal(v, &ecdsaSignature{})
+	if err == nil {
+		t.Fatalf("Unmarshalling should fail for [% x]", v)
+	}
+	t.Logf("Unmarshalling correctly failed for [% x] [%s]", v, err)
+
+	v = []byte{0x30, 0x0A, 0x02, 0x01, 0x8F, 0x02, 0x05, 0x00, 0x00, 0x00, 0x00, 0x8F}
+	_, err = asn1.Unmarshal(v, &ecdsaSignature{})
+	if err == nil {
+		t.Fatalf("Unmarshalling should fail for [% x]", v)
+	}
+	t.Logf("Unmarshalling correctly failed for [% x] [%s]", v, err)
+
+}
+
+func TestECDSALowS(t *testing.T) {
+	// Ensure that signature with low-S are generated
+	k, err := currentBCCSP.KeyGen(&bccsp.ECDSAKeyGenOpts{Temporary: false})
+	if err != nil {
+		t.Fatalf("Failed generating ECDSA key [%s]", err)
+	}
+
+	msg := []byte("Hello World")
+
+	digest, err := currentBCCSP.Hash(msg, &bccsp.SHAOpts{})
+	if err != nil {
+		t.Fatalf("Failed computing HASH [%s]", err)
+	}
+
+	signature, err := currentBCCSP.Sign(k, digest, nil)
+	if err != nil {
+		t.Fatalf("Failed generating ECDSA signature [%s]", err)
+	}
+
+	R, S, err := unmarshalECDSASignature(signature)
+	if err != nil {
+		t.Fatalf("Failed unmarshalling signature [%s]", err)
+	}
+
+	if S.Cmp(curveHalfOrders[k.(*ecdsaPrivateKey).privKey.Curve]) >= 0 {
+		t.Fatal("Invalid signature. It must have low-S")
+	}
+
+	valid, err := currentBCCSP.Verify(k, signature, digest, nil)
+	if err != nil {
+		t.Fatalf("Failed verifying ECDSA signature [%s]", err)
+	}
+	if !valid {
+		t.Fatal("Failed verifying ECDSA signature. Signature not valid.")
+	}
+
+	// Ensure that signature with high-S are rejected.
+	for {
+		R, S, err = ecdsa.Sign(rand.Reader, k.(*ecdsaPrivateKey).privKey, digest)
+		if err != nil {
+			t.Fatalf("Failed generating signature [%s]", err)
+		}
+
+		if S.Cmp(curveHalfOrders[k.(*ecdsaPrivateKey).privKey.Curve]) > 0 {
+			break
+		}
+	}
+
+	sig, err := marshalECDSASignature(R, S)
+	if err != nil {
+		t.Fatalf("Failing unmarshalling signature [%s]", err)
+	}
+
+	valid, err = currentBCCSP.Verify(k, sig, digest, nil)
+	if err == nil {
+		t.Fatal("Failed verifying ECDSA signature. It must fail for a signature with high-S")
+	}
+	if valid {
+		t.Fatal("Failed verifying ECDSA signature. It must fail for a signature with high-S")
+	}
+}
+
 func TestAESKeyGen(t *testing.T) {
 
 	k, err := currentBCCSP.KeyGen(&bccsp.AESKeyGenOpts{Temporary: false})