[crypto] Reject non-canonical ECDSA signature data structures

An ECDSA signature value is a vector of two integers (r,s) modulo the
curve group order.  The ECDSA algorithm itself does not define the
encoding to be used for these two integers.  At least two different
standards exist for representing the vector (r,s): the ASN.1 structure
originally defined in RFC 3279 (which uses a SEQUENCE of two INTEGER
values) and the raw byte concatenation structure defined in IEEE
P1363.  A valid signature vector (r,s) may be freely converted between
these two formats.  Changing the format does not logically change the
validity of the signature.

Due to the mathematics underlying ECDSA, the vector (r,-s) is also
always a valid signature for the same content.

With the ASN.1 structure, there exists the possibility of adding extra
data that would currently be ignored by the parser: either objects
following the top-level SEQUENCE, or objects within the SEQUENCE
following the two INTEGER values.  Adding this data does not logically
change the validity of the signature, in the same way that converting
between ASN.1 and P1363 does not logically change the validity of the
signature.  However, some public test vector sets check for the
rejection of signatures containing inserted data.

Reject any ECDSA signature object that includes data following the
top-level SEQUENCE, or that includes data following the "r" and "s"
INTEGER values.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/crypto/ecdsa.c

diff --git a/src/crypto/ecdsa.c b/src/crypto/ecdsa.c
index 02a87ac..f4946e6 100644
--- a/src/crypto/ecdsa.c
+++ b/src/crypto/ecdsa.c
@@ -847,14 +847,30 @@
 
 	/* Enter sequence */
 	memcpy ( &cursor, signature, sizeof ( cursor ) );
+	asn1_shrink_any ( &cursor );
+	if ( cursor.len != signature->len ) {
+		DBGC ( &ctx, "ECDSA %p signature has multiple objects:\n",
+		       &ctx );
+		DBGC_HDA ( &ctx, 0, signature->data, signature->len );
+		rc = -EINVAL_SIGNATURE;
+		goto err_parse;
+	}
 	asn1_enter ( &cursor, ASN1_SEQUENCE );
 
 	/* Extract "r" and "s" values */
 	if ( ( rc = ecdsa_parse_signature ( &ctx, ctx.r0, &cursor ) ) != 0 )
-		goto err_r;
+		goto err_parse;
 	asn1_skip_any ( &cursor );
 	if ( ( rc = ecdsa_parse_signature ( &ctx, ctx.s0, &cursor ) ) != 0 )
-		goto err_s;
+		goto err_parse;
+	asn1_skip_any ( &cursor );
+	if ( cursor.len ) {
+		DBGC ( &ctx, "ECDSA %p signature has extra objects:\n",
+		       &ctx );
+		DBGC_HDA ( &ctx, 0, signature->data, signature->len );
+		rc = -EINVAL_SIGNATURE;
+		goto err_parse;
+	}
 
 	/* Verify signature */
 	if ( ( rc = ecdsa_verify_rs ( &ctx ) ) != 0 )
@@ -866,8 +882,7 @@
 	return 0;
 
  err_verify:
- err_s:
- err_r:
+ err_parse:
 	ecdsa_free ( &ctx );
  err_init:
 	return rc;