[FAB-17778] Force sanitized signatures be canonically built

This change set forces sanitized signatures to be built via ASN1 marshaling regardless of their S is in the low or high half of the curve order. More details in the JIRA. Change-Id: Ife9c033d72fc0faa4cc808109dff71a7bf444c92 Signed-off-by: yacovm <[email protected]>
hyperledger · Apr 23, 2020 · 5066919 · 5066919
1 parent c66c3c4
commit 5066919
Show file tree

Hide file tree

Showing 6 changed files with 12 additions and 17 deletions.
diff --git a/bccsp/pkcs11/ecdsa.go b/bccsp/pkcs11/ecdsa.go
@@ -20,7 +20,7 @@ func (csp *impl) signECDSA(k ecdsaPrivateKey, digest []byte, opts bccsp.SignerOp
 		return nil, err
 	}
 
-	s, _, err = utils.ToLowS(k.pub.pub, s)
+	s, err = utils.ToLowS(k.pub.pub, s)
 	if err != nil {
 		return nil, err
 	}

diff --git a/bccsp/sw/ecdsa.go b/bccsp/sw/ecdsa.go
@@ -30,7 +30,7 @@ func signECDSA(k *ecdsa.PrivateKey, digest []byte, opts bccsp.SignerOpts) ([]byt
 		return nil, err
 	}
 
-	s, _, err = utils.ToLowS(&k.PublicKey, s)
+	s, err = utils.ToLowS(&k.PublicKey, s)
 	if err != nil {
 		return nil, err
 	}

diff --git a/bccsp/utils/ecdsa.go b/bccsp/utils/ecdsa.go
@@ -72,16 +72,12 @@ func SignatureToLowS(k *ecdsa.PublicKey, signature []byte) ([]byte, error) {
 		return nil, err
 	}
 
-	s, modified, err := ToLowS(k, s)
+	s, err = ToLowS(k, s)
 	if err != nil {
 		return nil, err
 	}
 
-	if modified {
-		return MarshalECDSASignature(r, s)
-	}
-
-	return signature, nil
+	return MarshalECDSASignature(r, s)
 }
 
 // IsLow checks that s is a low-S
@@ -95,19 +91,19 @@ func IsLowS(k *ecdsa.PublicKey, s *big.Int) (bool, error) {
 
 }
 
-func ToLowS(k *ecdsa.PublicKey, s *big.Int) (*big.Int, bool, error) {
+func ToLowS(k *ecdsa.PublicKey, s *big.Int) (*big.Int, error) {
 	lowS, err := IsLowS(k, s)
 	if err != nil {
-		return nil, false, err
+		return nil, err
 	}
 
 	if !lowS {
 		// 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 s, true, nil
+		return s, nil
 	}
 
-	return s, false, nil
+	return s, nil
 }
diff --git a/bccsp/utils/ecdsa_test.go b/bccsp/utils/ecdsa_test.go
@@ -80,9 +80,8 @@ func TestIsLowS(t *testing.T) {
 	lowS, err = IsLowS(&lowLevelKey.PublicKey, s)
 	assert.NoError(t, err)
 	assert.False(t, lowS)
-	s, modified, err := ToLowS(&lowLevelKey.PublicKey, s)
+	s, err = ToLowS(&lowLevelKey.PublicKey, s)
 	assert.NoError(t, err)
-	assert.True(t, modified)
 	lowS, err = IsLowS(&lowLevelKey.PublicKey, s)
 	assert.NoError(t, err)
 	assert.True(t, lowS)

diff --git a/cmd/common/signer/signer.go b/cmd/common/signer/signer.go
@@ -107,7 +107,7 @@ func signECDSA(k *ecdsa.PrivateKey, digest []byte) (signature []byte, err error)
 		return nil, err
 	}
 
-	s, _, err = utils.ToLowS(&k.PublicKey, s)
+	s, err = utils.ToLowS(&k.PublicKey, s)
 	if err != nil {
 		return nil, err
 	}

diff --git a/discovery/test/integration_test.go b/discovery/test/integration_test.go
@@ -33,7 +33,7 @@ import (
 	"github.com/hyperledger/fabric/common/tools/configtxgen/encoder"
 	genesisconfig "github.com/hyperledger/fabric/common/tools/configtxgen/localconfig"
 	"github.com/hyperledger/fabric/common/util"
-	"github.com/hyperledger/fabric/core/cclifecycle"
+	cc "github.com/hyperledger/fabric/core/cclifecycle"
 	lifecyclemocks "github.com/hyperledger/fabric/core/cclifecycle/mocks"
 	"github.com/hyperledger/fabric/core/comm"
 	"github.com/hyperledger/fabric/core/common/ccprovider"
@@ -939,7 +939,7 @@ func signECDSA(k *ecdsa.PrivateKey, digest []byte) (signature []byte, err error)
 		return nil, err
 	}
 
-	s, _, err = bccsp.ToLowS(&k.PublicKey, s)
+	s, err = bccsp.ToLowS(&k.PublicKey, s)
 	if err != nil {
 		return nil, err
 	}