schnorr: Rework signature parsing.

This reworks the Schnorr signature parsing code to be more consistent
with the ecdsa parsing code and pave the way for eventually being able
to remove the reliance on big ints.

It also adds a full set of signature parsing tests that include 100%
coverage of all branches in the parsing code and test for the explicit
reason for the failure to ensure that each test is actually testing the
intended condition.

In particular, the parsing code has been changed as follows:

- Reject signatures that have the r and/or s components out of
  the allowed ranges for their respective type
- Make use of the specialized mod N scalar and field val types
- Introduce new error codes for signature errors and return them

dcrec/secp256k1/schnorr/error.go

@@ -61,6 +61,22 @@ const (
 	// threshold signature failed to have a matching R value.
 	ErrNonmatchingR
 
+	// ErrSigTooShort is returned when a signature that should be a Schnorr
+	// signature is too short.
+	ErrSigTooShort
+
+	// ErrSigTooLong is returned when a signature that should be a Schnorr
+	// signature is too long.
+	ErrSigTooLong
+
+	// ErrSigRTooBig is returned when a signature has r with a value that is
+	// greater than or equal to the prime of the field underlying the group.
+	ErrSigRTooBig
+
+	// ErrSigSTooBig is returned when a signature has s with a value that is
+	// greater than or equal to the group order.
+	ErrSigSTooBig
+
 	// numErrorCodes is the maximum error code number used in tests.  This entry
 	// MUST be the last entry in the enum.
 	numErrorCodes
@@ -80,6 +96,10 @@ var errorCodeStrings = map[ErrorCode]string{
 	ErrBadNonce:          "ErrBadNonce",
 	ErrZeroSigS:          "ErrZeroSigS",
 	ErrNonmatchingR:      "ErrNonmatchingR",
+	ErrSigTooShort:       "ErrSigTooShort",
+	ErrSigTooLong:        "ErrSigTooLong",
+	ErrSigRTooBig:        "ErrSigRTooBig",
+	ErrSigSTooBig:        "ErrSigSTooBig",
 }
 
 // String returns the ErrorCode as a human-readable name.

dcrec/secp256k1/schnorr/error_test.go

@@ -27,6 +27,10 @@ func TestErrorCodeStringer(t *testing.T) {
 		{ErrBadNonce, "ErrBadNonce"},
 		{ErrZeroSigS, "ErrZeroSigS"},
 		{ErrNonmatchingR, "ErrNonmatchingR"},
+		{ErrSigTooShort, "ErrSigTooShort"},
+		{ErrSigTooLong, "ErrSigTooLong"},
+		{ErrSigRTooBig, "ErrSigRTooBig"},
+		{ErrSigSTooBig, "ErrSigSTooBig"},
 		{0xffff, "Unknown ErrorCode (65535)"},
 	}
 

dcrec/secp256k1/schnorr/signature.go

@@ -66,24 +66,53 @@ func (sig Signature) Serialize() []byte {
 	return all
 }
 
-func parseSig(sigStr []byte) (*Signature, error) {
-	if len(sigStr) != SignatureSize {
-		return nil, fmt.Errorf("bad signature size; have %v, want %v",
-			len(sigStr), SignatureSize)
+// ParseSignature parses a signature according to the EC-Schnorr-DCRv0
+// specification and enforces the following additional restrictions specific to
+// secp256k1:
+//
+// - The r component must be in the valid range for secp256k1 field elements
+// - The s component must be in the valid range for secp256k1 scalars
+func ParseSignature(sig []byte) (*Signature, error) {
+	// The signature must be the correct length.
+	sigLen := len(sig)
+	if sigLen < SignatureSize {
+		str := fmt.Sprintf("malformed signature: too short: %d < %d", sigLen,
+			SignatureSize)
+		return nil, signatureError(ErrSigTooShort, str)
+	}
+	if sigLen > SignatureSize {
+		str := fmt.Sprintf("malformed signature: too long: %d > %d", sigLen,
+			SignatureSize)
+		return nil, signatureError(ErrSigTooLong, str)
 	}
 
-	rBytes := copyBytes(sigStr[0:32])
-	r := encodedBytesToBigInt(rBytes)
-	sBytes := copyBytes(sigStr[32:64])
-	s := encodedBytesToBigInt(sBytes)
+	// The signature is validly encoded at this point, however, enforce
+	// additional restrictions to ensure r is in the range [0, p-1], and s is in
+	// the range [0, n-1] since valid Schnorr signatures are required to be in
+	// that range per spec.
+	//
+	// Notice that rejecting these values here is not strictly required because
+	// they are also checked when verifying the signature, but there really
+	// isn't a good reason not to fail early here on signatures that do not
+	// conform to the spec.
+	var r secp256k1.FieldVal
+	if overflow := r.SetByteSlice(sig[0:32]); overflow {
+		str := "invalid signature: r >= field prime"
+		return nil, signatureError(ErrSigRTooBig, str)
+	}
+	var s secp256k1.ModNScalar
+	if overflow := s.SetByteSlice(sig[32:64]); overflow {
+		str := "invalid signature: s >= group order"
+		return nil, signatureError(ErrSigSTooBig, str)
+	}
 
-	return &Signature{r, s}, nil
-}
-
-// ParseSignature parses a signature in BER format for the curve type `curve'
-// into a Signature type, performing some basic sanity checks.
-func ParseSignature(sigStr []byte) (*Signature, error) {
-	return parseSig(sigStr)
+	// Return the signature.
+	var rBytes, sBytes [scalarSize]byte
+	r.PutBytes(&rBytes)
+	s.PutBytes(&sBytes)
+	rBig := encodedBytesToBigInt(&rBytes)
+	sBig := encodedBytesToBigInt(&sBytes)
+	return &Signature{rBig, sBig}, nil
 }
 
 // IsEqual compares this Signature instance to the one passed, returning true

dcrec/secp256k1/schnorr/signature_test.go

@@ -15,6 +15,69 @@ import (
 	"github.com/decred/dcrd/dcrec/secp256k1/v3"
 )
 
+// TestSignatureParsing ensures that signatures are properly parsed including
+// error paths.
+func TestSignatureParsing(t *testing.T) {
+	tests := []struct {
+		name string // test description
+		sig  string // hex encoded signature to parse
+		err  error  // expected error
+	}{{
+		name: "valid signature 1",
+		sig: "c6ec70969d8367538c442f8e13eb20ff0c9143690f31cd3a384da54dd29ec0aa" +
+			"4b78a1b0d6b4186195d42a85614d3befd9f12ed26542d0dd1045f38c98b4a405",
+		err: nil,
+	}, {
+		name: "valid signature 2",
+		sig: "adc21db084fa1765f9372c2021fb298720f3d13e6d844e2dff751a2d46a69277" +
+			"0b989e316f7faf308a5f4a7343c0569465287cf6bff457250d6dacbb361f6e63",
+		err: nil,
+	}, {
+		name: "empty",
+		sig:  "",
+		err:  ErrSigTooShort,
+	}, {
+		name: "too short by one byte",
+		sig: "adc21db084fa1765f9372c2021fb298720f3d13e6d844e2dff751a2d46a69277" +
+			"0b989e316f7faf308a5f4a7343c0569465287cf6bff457250d6dacbb361f6e",
+		err: ErrSigTooShort,
+	}, {
+		name: "too long by one byte",
+		sig: "adc21db084fa1765f9372c2021fb298720f3d13e6d844e2dff751a2d46a69277" +
+			"0b989e316f7faf308a5f4a7343c0569465287cf6bff457250d6dacbb361f6e6300",
+		err: ErrSigTooLong,
+	}, {
+		name: "r == p",
+		sig: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f" +
+			"181522ec8eca07de4860a4acdd12909d831cc56cbbac4622082221a8768d1d09",
+		err: ErrSigRTooBig,
+	}, {
+		name: "r > p",
+		sig: "fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc30" +
+			"181522ec8eca07de4860a4acdd12909d831cc56cbbac4622082221a8768d1d09",
+		err: ErrSigRTooBig,
+	}, {
+		name: "s == n",
+		sig: "4e45e16932b8af514961a1d3a1a25fdf3f4f7732e9d624c6c61548ab5fb8cd41" +
+			"fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141",
+		err: ErrSigSTooBig,
+	}, {
+		name: "s > n",
+		sig: "4e45e16932b8af514961a1d3a1a25fdf3f4f7732e9d624c6c61548ab5fb8cd41" +
+			"fffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364142",
+		err: ErrSigSTooBig,
+	}}
+
+	for _, test := range tests {
+		_, err := ParseSignature(hexToBytes(test.sig))
+		if !errors.Is(err, test.err) {
+			t.Errorf("%s mismatched err -- got %v, want %v", test.name, err,
+				test.err)
+			continue
+		}
+	}
+}
+
 // TestSchnorrSignAndVerify ensures the Schnorr signing function produces the
 // expected signatures for a selected set of private keys, messages, and nonces
 // that have been independently verified with the Sage computer algebra system.