LBRYCommunity/lbcd
1
0
Fork 0
Code Issues 17 Pull requests 4 Projects Releases 28 Packages Wiki Activity

Add ParseDERSignature.

Browse source 
This change adds an additional signature parsing function which
performs additional checks to verify the signature is serialized in a
valid DER (and thus, unique) format, instead of allowing the less
strict BER signatures that ParseSignature will happily accept.

Added additional tests and updated test coverage to reflect changes.
This commit is contained in:
Josh Rickmar 2013-10-24 16:13:27 -04:00
parent e748650cc8
commit 98ac46b37d
3 changed files with 125 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
signature.go
View file

@ -11,15 +11,19 @@ import (
"math/big"
)
// Errors returned by canonicalPadding.
var (
errNegativeValue = errors.New("value may be interpreted as negative")
errExcessivelyPaddedValue = errors.New("value is excessively padded")
)
// Signature is a type representing an ecdsa signature.
type Signature struct {
R *big.Int
S *big.Int
}
// ParseSignature parses a signature in DER format for the curve type `curve'
// into a Signature type, perfoming some basic sanity checks.
func ParseSignature(sigStr []byte, curve elliptic.Curve) (*Signature, error) {
func parseSig(sigStr []byte, curve elliptic.Curve, der bool) (*Signature, error) {
// Originally this code used encoding/asn1 in order to parse the
// signature, but a number of problems were found with this approach.
// Despite the fact that signatures are stored as DER, the difference
@ -69,7 +73,16 @@ func ParseSignature(sigStr []byte, curve elliptic.Curve) (*Signature, error) {
}
// Then R itself.
signature.R = new(big.Int).SetBytes(sigStr[index : index+rLen])
rBytes := sigStr[index : index+rLen]
if der {
switch err := canonicalPadding(rBytes); err {
case errNegativeValue:
return nil, errors.New("signature R is negative")
case errExcessivelyPaddedValue:
return nil, errors.New("signature R is excessively padded")
}
}
signature.R = new(big.Int).SetBytes(rBytes)
index += rLen
// 0x02. length already checked in previous if.
if sigStr[index] != 0x02 {
@ -86,7 +99,16 @@ func ParseSignature(sigStr []byte, curve elliptic.Curve) (*Signature, error) {
}
// Then S itself.
signature.S = new(big.Int).SetBytes(sigStr[index : index+sLen])
sBytes := sigStr[index : index+sLen]
if der {
switch err := canonicalPadding(sBytes); err {
case errNegativeValue:
return nil, errors.New("signature S is negative")
case errExcessivelyPaddedValue:
return nil, errors.New("signature S is excessively padded")
}
}
signature.S = new(big.Int).SetBytes(sBytes)
index += sLen
// sanity check length parsing
@ -114,3 +136,32 @@ func ParseSignature(sigStr []byte, curve elliptic.Curve) (*Signature, error) {
return signature, nil
}
// ParseSignature parses a signature in BER format for the curve type `curve'
// into a Signature type, perfoming some basic sanity checks. If parsing
// according to the more strict DER format is needed, use ParseDERSignature.
func ParseSignature(sigStr []byte, curve elliptic.Curve) (*Signature, error) {
return parseSig(sigStr, curve, false)
}
// ParseDERSignature parses a signature in DER format for the curve type
// `curve` into a Signature type. If parsing according to the less strict
// BER format is needed, use ParseSignature.
func ParseDERSignature(sigStr []byte, curve elliptic.Curve) (*Signature, error) {
return parseSig(sigStr, curve, true)
}
// canonicalPadding checks whether a big-endian encoded integer could
// possibly be misinterpreted as a negative number (even though OpenSSL
// treats all numbers as unsigned), or if there is any unnecessary
// leading zero padding.
func canonicalPadding(b []byte) error {
switch {
case b[0]&0x80 == 0x80:
return errNegativeValue
case len(b) > 1 && b[0] == 0x00 && b[1]&0x80 != 0x80:
return errExcessivelyPaddedValue
default:
return nil
}
}

67
signature_test.go
View file

@ -12,6 +12,7 @@ import (
type signatureTest struct {
name string
sig []byte
der bool
isValid bool
}
@ -29,6 +30,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: true,
},
signatureTest{
@ -47,6 +49,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -60,6 +63,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -73,6 +77,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -86,6 +91,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -99,6 +105,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -112,6 +119,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -125,6 +133,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -138,6 +147,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -151,6 +161,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09, 0x01,
},
der: true,
// This test is now passing (used to be failing) because there
// are signatures in the blockchain that have trailing zero
@ -170,6 +181,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -183,6 +195,7 @@ var signatureTests = []signatureTest{
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: false,
isValid: false,
},
signatureTest{
@ -196,6 +209,7 @@ var signatureTests = []signatureTest{
0xFE, 0xBA, 0xAE, 0xDC, 0xE6, 0xAF, 0x48, 0xA0, 0x3B,
0xBF, 0xD2, 0x5E, 0x8C, 0xD0, 0x36, 0x41, 0x41,
},
der: true,
isValid: false,
},
signatureTest{
@ -209,6 +223,7 @@ var signatureTests = []signatureTest{
0xFE, 0xBA, 0xAE, 0xDC, 0xE6, 0xAF, 0x48, 0xA0, 0x3B,
0xBF, 0xD2, 0x5E, 0x8C, 0xD0, 0x36, 0x41, 0x42,
},
der: false,
isValid: false,
},
signatureTest{
@ -219,6 +234,7 @@ var signatureTests = []signatureTest{
0xbb, 0xac, 0x46, 0x22, 0x08, 0x22, 0x21, 0xa8, 0x76,
0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
@ -229,13 +245,45 @@ var signatureTests = []signatureTest{
0x24, 0xc6, 0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd,
0x41, 0x02, 0x00,
},
der: true,
isValid: false,
},
// We don't test for negative numbers here because there isn't a way
// that is the same between openssl and go that will mark a number as
// negative. The Go ASN.1 parser marks numbers as negative when openssl
// does not (it doesn't handle negative numbers that I can tell at all.
// therefore we only check for the coordinates being zero.
signatureTest{
name: "extra R padding.",
sig: []byte{0x30, 0x45, 0x02, 0x21, 0x00, 0x4e, 0x45, 0xe1, 0x69,
0x32, 0xb8, 0xaf, 0x51, 0x49, 0x61, 0xa1, 0xd3, 0xa1,
0xa2, 0x5f, 0xdf, 0x3f, 0x4f, 0x77, 0x32, 0xe9, 0xd6,
0x24, 0xc6, 0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd,
0x41, 0x02, 0x20, 0x18, 0x15, 0x22, 0xec, 0x8e, 0xca,
0x07, 0xde, 0x48, 0x60, 0xa4, 0xac, 0xdd, 0x12, 0x90,
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
signatureTest{
name: "extra S padding.",
sig: []byte{0x30, 0x45, 0x02, 0x20, 0x4e, 0x45, 0xe1, 0x69,
0x32, 0xb8, 0xaf, 0x51, 0x49, 0x61, 0xa1, 0xd3, 0xa1,
0xa2, 0x5f, 0xdf, 0x3f, 0x4f, 0x77, 0x32, 0xe9, 0xd6,
0x24, 0xc6, 0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd,
0x41, 0x02, 0x21, 0x00, 0x18, 0x15, 0x22, 0xec, 0x8e, 0xca,
0x07, 0xde, 0x48, 0x60, 0xa4, 0xac, 0xdd, 0x12, 0x90,
0x9d, 0x83, 0x1c, 0xc5, 0x6c, 0xbb, 0xac, 0x46, 0x22,
0x08, 0x22, 0x21, 0xa8, 0x76, 0x8d, 0x1d, 0x09,
},
der: true,
isValid: false,
},
// Standard checks (in BER format, without checking for 'canonical' DER
// signatures) don't test for negative numbers here because there isn't
// a way that is the same between openssl and go that will mark a number
// as negative. The Go ASN.1 parser marks numbers as negative when
// openssl does not (it doesn't handle negative numbers that I can tell
// at all. When not parsing DER signatures, which is done by by bitcoind
// when accepting transactions into its mempool, we otherwise only check
// for the coordinates being zero.
signatureTest{
name: "X == 0",
sig: []byte{0x30, 0x25, 0x02, 0x01, 0x00, 0x02, 0x20, 0x18,
@ -244,6 +292,7 @@ var signatureTests = []signatureTest{
0x6c, 0xbb, 0xac, 0x46, 0x22, 0x08, 0x22, 0x21, 0xa8,
0x76, 0x8d, 0x1d, 0x09,
},
der: false,
isValid: false,
},
signatureTest{
@ -254,13 +303,19 @@ var signatureTests = []signatureTest{
0x24, 0xc6, 0xc6, 0x15, 0x48, 0xab, 0x5f, 0xb8, 0xcd,
0x41, 0x02, 0x01, 0x00,
},
der: false,
isValid: false,
},
}
func TestSignatures(t *testing.T) {
for _, test := range signatureTests {
_, err := btcec.ParseSignature(test.sig, btcec.S256())
var err error
if test.der {
_, err = btcec.ParseDERSignature(test.sig, btcec.S256())
} else {
_, err = btcec.ParseSignature(test.sig, btcec.S256())
}
if err != nil {
if test.isValid {
t.Errorf("%s signature failed when shouldn't %v",

13
test_coverage.txt
View file

@ -1,23 +1,26 @@
github.com/conformal/btcec/signature.go ParseSignature 100.00% (41/41)
github.com/conformal/btcec/signature.go parseSig 100.00% (51/51)
github.com/conformal/btcec/btcec.go KoblitzCurve.doubleJacobian 100.00% (21/21)
github.com/conformal/btcec/btcec.go KoblitzCurve.ScalarMult 100.00% (9/9)
github.com/conformal/btcec/pubkey.go PublicKey.SerializeHybrid 100.00% (8/8)
github.com/conformal/btcec/btcec.go KoblitzCurve.IsOnCurve 100.00% (7/7)
github.com/conformal/btcec/pubkey.go PublicKey.SerializeCompressed 100.00% (7/7)
github.com/conformal/btcec/btcec.go initS256 100.00% (7/7)
github.com/conformal/btcec/pubkey.go PublicKey.SerializeCompressed 100.00% (7/7)
github.com/conformal/btcec/btcec.go KoblitzCurve.IsOnCurve 100.00% (7/7)
github.com/conformal/btcec/pubkey.go PublicKey.SerializeUncompressed 100.00% (5/5)
github.com/conformal/btcec/btcec.go zForAffine 100.00% (4/4)
github.com/conformal/btcec/signature.go canonicalPadding 100.00% (4/4)
github.com/conformal/btcec/btcec.go KoblitzCurve.QPlus1Div4 100.00% (3/3)
github.com/conformal/btcec/btcec.go KoblitzCurve.Add 100.00% (3/3)
github.com/conformal/btcec/btcec.go S256 100.00% (2/2)
github.com/conformal/btcec/btcec.go initAll 100.00% (1/1)
github.com/conformal/btcec/btcec.go KoblitzCurve.ScalarBaseMult 100.00% (1/1)
github.com/conformal/btcec/pubkey.go isOdd 100.00% (1/1)
github.com/conformal/btcec/btcec.go initAll 100.00% (1/1)
github.com/conformal/btcec/signature.go ParseSignature 100.00% (1/1)
github.com/conformal/btcec/signature.go ParseDERSignature 100.00% (1/1)
github.com/conformal/btcec/btcec.go KoblitzCurve.Params 100.00% (1/1)
github.com/conformal/btcec/pubkey.go ParsePubKey 96.88% (31/32)
github.com/conformal/btcec/btcec.go KoblitzCurve.addJacobian 91.67% (55/60)
github.com/conformal/btcec/btcec.go KoblitzCurve.affineFromJacobian 90.00% (9/10)
github.com/conformal/btcec/btcec.go KoblitzCurve.Double 0.00% (0/2)
github.com/conformal/btcec ------------------------------- 96.00% (216/225)
github.com/conformal/btcec ------------------------------- 96.27% (232/241)