Drop support for the ASN.1-based ECDSA signature format in favor of

the simpler format which PKCS #11 uses, since we have to support the
latter in any case and it's not worth the complexity of supporting both.

7 files changed, 63 insertions, 175 deletions

diff --git a/asn1_internal.h b/asn1_internal.h
index bfe9372..18d4852 100644
--- a/asn1_internal.h
+++ b/asn1_internal.h
@@ -1,9 +1,12 @@
 /*
- * asn1.h
- * ------
- * Library internal header file for ASN.1 routines.
+ * asn1_internal.h
+ * ---------------
+ * Library internal header file for ASN.1 routines.  These functions
+ * are not part of the public libhal API.
  *
- * These functions are not part of the public libhal API.
+ * The only reason for not collapsing this header file into
+ * hal_internal.h is to maintain some isolation between the few
+ * modules which use libtfm and the rest of the library.
  *
  * More than 20 years after it was written, the best simple
  * introduction to ASN.1 is still Burt Kalski's "A Layman's Guide to a
@@ -40,8 +43,8 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef _HAL_ASN1_H_
-#define _HAL_ASN1_H_
+#ifndef _HAL_ASN1_INTERNAL_H_
+#define _HAL_ASN1_INTERNAL_H_
 
 #include <stdint.h>
 
@@ -100,7 +103,7 @@ extern hal_error_t hal_asn1_encode_integer(const fp_int * const bn,
 extern hal_error_t hal_asn1_decode_integer(fp_int *bn,
                                            const uint8_t * const der, size_t *der_len, const size_t der_max);
 
-#endif /* _HAL_ASN1_H_ */
+#endif /* _HAL_ASN1_INTERNAL_H_ */
 
 /*
  * Local variables:
diff --git a/ecdsa.c b/ecdsa.c
index 703849b..10c2ba2 100644
--- a/ecdsa.c
+++ b/ecdsa.c
@@ -1345,87 +1345,13 @@ static hal_error_t decode_signature_pkcs11(const ecdsa_curve_t * const curve,
 }
 
 /*
- * Encode a signature in ASN.1 format SEQUENCE { INTEGER r, INTEGER s }.
- */
-
-static hal_error_t encode_signature_asn1(const ecdsa_curve_t * const curve,
-                                         const fp_int * const r, const fp_int * const s,
-                                         uint8_t *signature, size_t *signature_len, const size_t signature_max)
-{
-  assert(curve != NULL && r != NULL && s != NULL);
-
-  size_t hlen, r_len, s_len;
-  hal_error_t err;
-
-  if ((err = hal_asn1_encode_integer(r, NULL, &r_len, 0)) != HAL_OK ||
-      (err = hal_asn1_encode_integer(s, NULL, &s_len, 0)) != HAL_OK)
-    return err;
-
-  const size_t vlen = r_len + s_len;
-
-  err = hal_asn1_encode_header(ASN1_SEQUENCE, vlen, signature, &hlen, signature_max);
-
-  if (signature_len != NULL)
-    *signature_len = hlen + vlen;
-
-  if (signature == NULL || err != HAL_OK)
-    return err;
-
-  uint8_t * const r_out = signature + hlen;
-  uint8_t * const s_out = r_out + r_len;
-
-  if ((err = hal_asn1_encode_integer(r, r_out, NULL, signature_max - (r_out - signature))) != HAL_OK ||
-      (err = hal_asn1_encode_integer(s, s_out, NULL, signature_max - (s_out - signature))) != HAL_OK)
-    return err;
-
-  return HAL_OK;
-}
-
-/*
- * Decode a signature from ASN.1 format SEQUENCE { INTEGER r, INTEGER s }.
- */
-
-static hal_error_t decode_signature_asn1(const ecdsa_curve_t * const curve,
-                                         fp_int *r, fp_int *s,
-                                         const uint8_t * const signature, const size_t signature_len)
-{
-  assert(curve != NULL && r != NULL && s != NULL);
-
-  if (signature == NULL)
-    return HAL_ERROR_BAD_ARGUMENTS;
-
-  size_t len1, len2;
-  hal_error_t err;
-
-  if ((err = hal_asn1_decode_header(ASN1_SEQUENCE, signature, signature_len, &len1, &len2)) != HAL_OK)
-    return err;
-
-  const uint8_t *       der     = signature + len1;
-  const uint8_t * const der_end = der       + len2;
-
-  if ((err = hal_asn1_decode_integer(r, der, &len1, der_end - der)) != HAL_OK)
-    return err;
-  der += len1;
-
-  if ((err = hal_asn1_decode_integer(s, der, &len1, der_end - der)) != HAL_OK)
-    return err;
-  der += len1;
-
-  if (der != der_end)
-    return HAL_ERROR_ASN1_PARSE_FAILED;
-
-  return HAL_OK;
-}
-
-/*
  * Sign a caller-supplied hash.
  */
 
 hal_error_t hal_ecdsa_sign(const hal_core_t *core,
                            const hal_ecdsa_key_t * const key,
                            const uint8_t * const hash, const size_t hash_len,
-                           uint8_t *signature, size_t *signature_len, const size_t signature_max,
-                           const hal_ecdsa_signature_format_t signature_format)
+                           uint8_t *signature, size_t *signature_len, const size_t signature_max)
 {
   if (key == NULL || hash == NULL || signature == NULL || signature_len == NULL || key->type != HAL_KEY_TYPE_EC_PRIVATE)
     return HAL_ERROR_BAD_ARGUMENTS;
@@ -1487,21 +1413,8 @@ hal_error_t hal_ecdsa_sign(const hal_core_t *core,
    * Encode the signature, then we're done.
    */
 
-  switch (signature_format) {
-
-  case HAL_ECDSA_SIGNATURE_FORMAT_ASN1:
-    if ((err = encode_signature_asn1(curve, r, s, signature, signature_len, signature_max)) != HAL_OK)
-      goto fail;
-    break;
-
-  case HAL_ECDSA_SIGNATURE_FORMAT_PKCS11:
-    if ((err = encode_signature_pkcs11(curve, r, s, signature, signature_len, signature_max)) != HAL_OK)
-      goto fail;
-    break;
-
-  default:
-    lose(HAL_ERROR_BAD_ARGUMENTS);
-  }
+  if ((err = encode_signature_pkcs11(curve, r, s, signature, signature_len, signature_max)) != HAL_OK)
+    goto fail;
 
   err = HAL_OK;
 
@@ -1518,8 +1431,7 @@ hal_error_t hal_ecdsa_sign(const hal_core_t *core,
 hal_error_t hal_ecdsa_verify(const hal_core_t *core,
                              const hal_ecdsa_key_t * const key,
                              const uint8_t * const hash, const size_t hash_len,
-                             const uint8_t * const signature, const size_t signature_len,
-                             const hal_ecdsa_signature_format_t signature_format)
+                             const uint8_t * const signature, const size_t signature_len)
 {
   assert(key != NULL && hash != NULL && signature != NULL);
 
@@ -1551,21 +1463,8 @@ hal_error_t hal_ecdsa_verify(const hal_core_t *core,
    * Start by decoding the signature.
    */
 
-  switch (signature_format) {
-
-  case HAL_ECDSA_SIGNATURE_FORMAT_ASN1:
-    if ((err = decode_signature_asn1(curve, r, s, signature, signature_len)) != HAL_OK)
-      return err;
-    break;
-
-  case HAL_ECDSA_SIGNATURE_FORMAT_PKCS11:
-    if ((err = decode_signature_pkcs11(curve, r, s, signature, signature_len)) != HAL_OK)
-      return err;
-    break;
-
-  default:
-    return HAL_ERROR_BAD_ARGUMENTS;
-  }
+  if ((err = decode_signature_pkcs11(curve, r, s, signature, signature_len)) != HAL_OK)
+    return err;
 
   /*
    * Check that r and s are in the allowed range, read the hash, then
diff --git a/hal.h b/hal.h
index 4a1ef75..3fcf813 100644
--- a/hal.h
+++ b/hal.h
@@ -435,8 +435,6 @@ extern hal_error_t hal_rsa_key_from_der(hal_rsa_key_t **key,
  * ECDSA.
  */
 
-typedef enum { HAL_ECDSA_SIGNATURE_FORMAT_ASN1, HAL_ECDSA_SIGNATURE_FORMAT_PKCS11 } hal_ecdsa_signature_format_t;
-
 typedef struct hal_ecdsa_key hal_ecdsa_key_t;
 
 extern const size_t hal_ecdsa_key_t_size;
@@ -495,14 +493,12 @@ extern hal_error_t hal_ecdsa_key_from_ecpoint(hal_ecdsa_key_t **key,
 extern hal_error_t hal_ecdsa_sign(const hal_core_t *core,
                                   const hal_ecdsa_key_t * const key,
                                   const uint8_t * const hash, const size_t hash_len,
-                                  uint8_t *signature, size_t *signature_len, const size_t signature_max,
-                                  const hal_ecdsa_signature_format_t signature_format);
+                                  uint8_t *signature, size_t *signature_len, const size_t signature_max);
 
 extern hal_error_t hal_ecdsa_verify(const hal_core_t *core,
                                     const hal_ecdsa_key_t * const key,
                                     const uint8_t * const hash, const size_t hash_len,
-                                    const uint8_t * const signature, const size_t signature_len,
-                                    const hal_ecdsa_signature_format_t signature_format);
+                                    const uint8_t * const signature, const size_t signature_len);
 
 /*
  * Higher level RPC-based mechanism for working with HSM at arm's
diff --git a/rpc_pkey.c b/rpc_pkey.c
index 8fece44..d286497 100644
--- a/rpc_pkey.c
+++ b/rpc_pkey.c
@@ -537,8 +537,7 @@ static hal_error_t sign_ecdsa(uint8_t *keybuf, const size_t keybuf_len,
     input = signature;
   }
 
-  if ((err = hal_ecdsa_sign(NULL, key, input, input_len, signature, signature_len, signature_max,
-                            HAL_ECDSA_SIGNATURE_FORMAT_PKCS11)) != HAL_OK)
+  if ((err = hal_ecdsa_sign(NULL, key, input, input_len, signature, signature_len, signature_max)) != HAL_OK)
     return err;
 
   return HAL_OK;
@@ -658,8 +657,7 @@ static hal_error_t verify_ecdsa(uint8_t *keybuf, const size_t keybuf_len,
     input = digest;
   }
 
-  if ((err = hal_ecdsa_verify(NULL, key, input, input_len, signature, signature_len,
-                            HAL_ECDSA_SIGNATURE_FORMAT_PKCS11)) != HAL_OK)
+  if ((err = hal_ecdsa_verify(NULL, key, input, input_len, signature, signature_len)) != HAL_OK)
     return err;
 
   return HAL_OK;
diff --git a/tests/test-ecdsa.c b/tests/test-ecdsa.c
index ae99616..98b3d70 100644
--- a/tests/test-ecdsa.c
+++ b/tests/test-ecdsa.c
@@ -150,13 +150,13 @@ static int test_against_static_vectors(const ecdsa_tc_t * const tc)
   uint8_t sig[tc->sig_len + 4];
   size_t  sig_len;
 
-  if ((err = hal_ecdsa_sign(NULL, key1, tc->H, tc->H_len, sig, &sig_len, sizeof(sig), HAL_ECDSA_SIGNATURE_FORMAT_ASN1)) != HAL_OK)
+  if ((err = hal_ecdsa_sign(NULL, key1, tc->H, tc->H_len, sig, &sig_len, sizeof(sig))) != HAL_OK)
     return printf("hal_ecdsa_sign() failed: %s\n", hal_error_string(err)), 0;
 
   if (sig_len != tc->sig_len || memcmp(sig, tc->sig, tc->sig_len) != 0)
     return printf("Signature mismatch\n"), 0;
 
-  if ((err = hal_ecdsa_verify(NULL, key2, tc->H, tc->H_len, sig, sig_len, HAL_ECDSA_SIGNATURE_FORMAT_ASN1)) != HAL_OK)
+  if ((err = hal_ecdsa_verify(NULL, key2, tc->H, tc->H_len, sig, sig_len)) != HAL_OK)
     return printf("hal_ecdsa_verify(private) failed: %s\n", hal_error_string(err)), 0;
 
   hal_ecdsa_key_clear(key2);
@@ -176,7 +176,7 @@ static int test_against_static_vectors(const ecdsa_tc_t * const tc)
                                        tc->Qx, tc->Qx_len, tc->Qy, tc->Qy_len)) != HAL_OK)
     return printf("hal_ecdsa_load_public() failed: %s\n", hal_error_string(err)), 0;
 
-  if ((err = hal_ecdsa_verify(NULL, key2, tc->H, tc->H_len, sig, sig_len, HAL_ECDSA_SIGNATURE_FORMAT_ASN1)) != HAL_OK)
+  if ((err = hal_ecdsa_verify(NULL, key2, tc->H, tc->H_len, sig, sig_len)) != HAL_OK)
     return printf("hal_ecdsa_verify(public) failed: %s\n", hal_error_string(err)), 0;
 
   uint8_t point[hal_ecdsa_key_to_ecpoint_len(key1)];
@@ -263,13 +263,12 @@ static int test_keygen_sign_verify(const hal_curve_name_t curve)
   printf("Signing\n");
 
   if ((err = hal_ecdsa_sign(NULL, key, hashbuf, sizeof(hashbuf),
-                            sigbuf, &siglen, sizeof(sigbuf), HAL_ECDSA_SIGNATURE_FORMAT_ASN1)) != HAL_OK)
+                            sigbuf, &siglen, sizeof(sigbuf))) != HAL_OK)
     return printf("hal_ecdsa_sign() failed: %s\n", hal_error_string(err)), 0;
 
   printf("Verifying\n");
 
-  if ((err = hal_ecdsa_verify(NULL, key, hashbuf, sizeof(hashbuf),
-                              sigbuf, siglen, HAL_ECDSA_SIGNATURE_FORMAT_ASN1)) != HAL_OK)
+  if ((err = hal_ecdsa_verify(NULL, key, hashbuf, sizeof(hashbuf), sigbuf, siglen)) != HAL_OK)
     return printf("hal_ecdsa_verify() failed: %s\n", hal_error_string(err)), 0;
 
   return 1;
diff --git a/tests/test-ecdsa.h b/tests/test-ecdsa.h
index a607d97..9fafe18 100644
--- a/tests/test-ecdsa.h
+++ b/tests/test-ecdsa.h
@@ -89,13 +89,13 @@ static const uint8_t p256_s[] = { /* 32 bytes */
   0x92, 0xdb, 0xea, 0xa1, 0xaf, 0x2b, 0xc3, 0x67
 };
 
-static const uint8_t p256_sig[] = { /* 70 bytes */
-  0x30, 0x44, 0x02, 0x20, 0x72, 0x14, 0xbc, 0x96, 0x47, 0x16, 0x0b, 0xbd,
-  0x39, 0xff, 0x2f, 0x80, 0x53, 0x3f, 0x5d, 0xc6, 0xdd, 0xd7, 0x0d, 0xdf,
-  0x86, 0xbb, 0x81, 0x56, 0x61, 0xe8, 0x05, 0xd5, 0xd4, 0xe6, 0xf2, 0x7c,
-  0x02, 0x20, 0x7d, 0x1f, 0xf9, 0x61, 0x98, 0x0f, 0x96, 0x1b, 0xda, 0xa3,
-  0x23, 0x3b, 0x62, 0x09, 0xf4, 0x01, 0x33, 0x17, 0xd3, 0xe3, 0xf9, 0xe1,
-  0x49, 0x35, 0x92, 0xdb, 0xea, 0xa1, 0xaf, 0x2b, 0xc3, 0x67
+static const uint8_t p256_sig[] = { /* 64 bytes */
+  0x72, 0x14, 0xbc, 0x96, 0x47, 0x16, 0x0b, 0xbd, 0x39, 0xff, 0x2f, 0x80,
+  0x53, 0x3f, 0x5d, 0xc6, 0xdd, 0xd7, 0x0d, 0xdf, 0x86, 0xbb, 0x81, 0x56,
+  0x61, 0xe8, 0x05, 0xd5, 0xd4, 0xe6, 0xf2, 0x7c, 0x7d, 0x1f, 0xf9, 0x61,
+  0x98, 0x0f, 0x96, 0x1b, 0xda, 0xa3, 0x23, 0x3b, 0x62, 0x09, 0xf4, 0x01,
+  0x33, 0x17, 0xd3, 0xe3, 0xf9, 0xe1, 0x49, 0x35, 0x92, 0xdb, 0xea, 0xa1,
+  0xaf, 0x2b, 0xc3, 0x67
 };
 
 static const uint8_t p256_u1[] = { /* 32 bytes */
@@ -223,16 +223,15 @@ static const uint8_t p384_s[] = { /* 48 bytes */
   0x67, 0xad, 0xad, 0xf1, 0x68, 0xeb, 0xbe, 0x80, 0x37, 0x94, 0xa4, 0x02
 };
 
-static const uint8_t p384_sig[] = { /* 103 bytes */
-  0x30, 0x65, 0x02, 0x31, 0x00, 0xa0, 0xc2, 0x7e, 0xc8, 0x93, 0x09, 0x2d,
-  0xea, 0x1e, 0x1b, 0xd2, 0xcc, 0xfe, 0xd3, 0xcf, 0x94, 0x5c, 0x81, 0x34,
-  0xed, 0x0c, 0x9f, 0x81, 0x31, 0x1a, 0x0f, 0x4a, 0x05, 0x94, 0x2d, 0xb8,
-  0xdb, 0xed, 0x8d, 0xd5, 0x9f, 0x26, 0x74, 0x71, 0xd5, 0x46, 0x2a, 0xa1,
-  0x4f, 0xe7, 0x2d, 0xe8, 0x56, 0x02, 0x30, 0x20, 0xab, 0x3f, 0x45, 0xb7,
-  0x4f, 0x10, 0xb6, 0xe1, 0x1f, 0x96, 0xa2, 0xc8, 0xeb, 0x69, 0x4d, 0x20,
-  0x6b, 0x9d, 0xda, 0x86, 0xd3, 0xc7, 0xe3, 0x31, 0xc2, 0x6b, 0x22, 0xc9,
-  0x87, 0xb7, 0x53, 0x77, 0x26, 0x57, 0x76, 0x67, 0xad, 0xad, 0xf1, 0x68,
-  0xeb, 0xbe, 0x80, 0x37, 0x94, 0xa4, 0x02
+static const uint8_t p384_sig[] = { /* 96 bytes */
+  0xa0, 0xc2, 0x7e, 0xc8, 0x93, 0x09, 0x2d, 0xea, 0x1e, 0x1b, 0xd2, 0xcc,
+  0xfe, 0xd3, 0xcf, 0x94, 0x5c, 0x81, 0x34, 0xed, 0x0c, 0x9f, 0x81, 0x31,
+  0x1a, 0x0f, 0x4a, 0x05, 0x94, 0x2d, 0xb8, 0xdb, 0xed, 0x8d, 0xd5, 0x9f,
+  0x26, 0x74, 0x71, 0xd5, 0x46, 0x2a, 0xa1, 0x4f, 0xe7, 0x2d, 0xe8, 0x56,
+  0x20, 0xab, 0x3f, 0x45, 0xb7, 0x4f, 0x10, 0xb6, 0xe1, 0x1f, 0x96, 0xa2,
+  0xc8, 0xeb, 0x69, 0x4d, 0x20, 0x6b, 0x9d, 0xda, 0x86, 0xd3, 0xc7, 0xe3,
+  0x31, 0xc2, 0x6b, 0x22, 0xc9, 0x87, 0xb7, 0x53, 0x77, 0x26, 0x57, 0x76,
+  0x67, 0xad, 0xad, 0xf1, 0x68, 0xeb, 0xbe, 0x80, 0x37, 0x94, 0xa4, 0x02
 };
 
 static const uint8_t p384_u1[] = { /* 48 bytes */
diff --git a/tests/test-ecdsa.py b/tests/test-ecdsa.py
index 1ecfef9..efd96e3 100644
--- a/tests/test-ecdsa.py
+++ b/tests/test-ecdsa.py
@@ -50,38 +50,31 @@ from pyasn1.codec.der.decoder   import decode as DER_Decode
 
 wrapper = TextWrapper(width = 78, initial_indent = " " * 2, subsequent_indent = " " * 2)
 
-def long_to_bytes(l):
+def long_to_bytes(number, order):
   #
   # This is just plain nasty.
   #
-  s = "%x" % l
-  return ("0" + s if len(s) & 1 else s).decode("hex")
+  s = "%x" % number
+  s = ("0" * (order/8 - len(s))) + s
+  return s.decode("hex")
 
-def bytes_to_bits(b):
+def bytes_to_bits(bytes):
   #
   # This, on the other hand, is not just plain nasty, this is fancy nasty.
   # This is nasty with raisins in it.
   #
-  bits = bin(long(b.encode("hex"), 16))[2:]
-  if len(bits) % 8:
-    bits = ("0" * (8 - len(bits) % 8)) + bits
-  return tuple(int(i) for i in bits)
+  s = bin(long(bytes.encode("hex"), 16))[2:]
+  if len(s) % 8:
+    s = ("0" * (8 - len(s) % 8)) + s
+  return tuple(int(i) for i in s)
 
 ###
 
-class ECDSA_Sig_Value(Sequence):
-  componentType = NamedTypes(
-    NamedType("r", Integer()),
-    NamedType("s", Integer()))
-
-def encode_sig(r, s):
-  sig = ECDSA_Sig_Value()
-  sig["r"] = r
-  sig["s"] = s
-  return DER_Encode(sig)
+def encode_sig(r, s, order):
+  return long_to_bytes(r, order) + long_to_bytes(s, order)
 
-p256_sig = encode_sig(p256_r, p256_s)
-p384_sig = encode_sig(p384_r, p384_s)
+p256_sig = encode_sig(p256_r, p256_s, 256)
+p384_sig = encode_sig(p384_r, p384_s, 384)
 
 ###
 
@@ -93,9 +86,9 @@ class ECPrivateKey(Sequence):
     OptionalNamedType("parameters", ObjectIdentifier().subtype(explicitTag = Tag(tagClassContext, tagFormatSimple, 0))),
     OptionalNamedType("publicKey", BitString().subtype(explicitTag = Tag(tagClassContext, tagFormatSimple, 1))))
 
-def encode_key(d, Qx, Qy, oid):
-  private_key = long_to_bytes(d)
-  public_key  = bytes_to_bits(chr(0x04) + long_to_bytes(Qx) + long_to_bytes(Qy))
+def encode_key(d, Qx, Qy, order, oid):
+  private_key = long_to_bytes(d, order)
+  public_key  = bytes_to_bits(chr(0x04) + long_to_bytes(Qx, order) + long_to_bytes(Qy, order))
   parameters = oid
   key = ECPrivateKey()
   key["version"]    = 1
@@ -104,8 +97,8 @@ def encode_key(d, Qx, Qy, oid):
   key["publicKey"]  = public_key
   return DER_Encode(key)
 
-p256_key = encode_key(p256_d, p256_Qx, p256_Qy, "1.2.840.10045.3.1.7")
-p384_key = encode_key(p384_d, p384_Qx, p384_Qy, "1.3.132.0.34")
+p256_key = encode_key(p256_d, p256_Qx, p256_Qy, 256, "1.2.840.10045.3.1.7")
+p384_key = encode_key(p384_d, p384_Qx, p384_Qy, 384, "1.3.132.0.34")
 
 ###
 
@@ -125,11 +118,12 @@ for name in dir():
 vars = sorted(vars)
 
 for curve in curves:
+  order = int(curve[1:])
   for var in vars:
     name = curve + "_" + var
     value = globals().get(name, None)
     if isinstance(value, (int, long)):
-      value = long_to_bytes(value)
+      value = long_to_bytes(value, order)
     if value is not None:
       print
       print "static const uint8_t %s[] = { /* %d bytes */" % (name, len(value))
@@ -138,14 +132,14 @@ for curve in curves:
     
 print
 print "typedef struct {"
-print "  hal_ecdsa_curve_t curve;"
+print "  hal_curve_name_t curve;"
 for var in vars:
   print "  const uint8_t *%8s; size_t %8s_len;" % (var, var)
 print "} ecdsa_tc_t;"
 print
 print "static const ecdsa_tc_t ecdsa_tc[] = {"
 for curve in curves:
-  print "  { HAL_ECDSA_CURVE_%s," % curve.upper()
+  print "  { HAL_CURVE_%s," % curve.upper()
   for var in vars:
     name = curve + "_" + var
     if name in globals():