From f73862ee3e87b61b4b2711c6bb4e9d2ef7d40bc1 Mon Sep 17 00:00:00 2001
From: Conor Patrick <conorpp@vt.edu>
Date: Sat, 26 May 2018 15:35:58 -0400
Subject: [PATCH] fix der encoding bug in u2f

---
 ctap.c | 43 ++++++++++++++++++++++++-------------------
 ctap.h |  3 +++
 u2f.c  | 31 ++++---------------------------
 3 files changed, 31 insertions(+), 46 deletions(-)

diff --git a/ctap.c b/ctap.c
index 6362af0..2c0c5b2 100644
--- a/ctap.c
+++ b/ctap.c
@@ -336,26 +336,11 @@ static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * au
     return auth_data_sz;
 }
 
-// require load_key prior to this
-// @data data to hash before signature
-// @clientDataHash for signature
-// @tmp buffer for hash.  (can be same as data if data >= 32 bytes)
-// @sigbuf location to deposit signature (must be 64 bytes)
-// @sigder location to deposit der signature (must be 72 bytes)
-// @return length of der signature
-int ctap_calculate_signature(uint8_t * data, int datalen, uint8_t * clientDataHash, uint8_t * hashbuf, uint8_t * sigbuf, uint8_t * sigder)
+
+int ctap_encode_der_sig(uint8_t * sigbuf, uint8_t * sigder)
 {
-    int i;
-    // calculate attestation sig
-    crypto_sha256_init();
-    crypto_sha256_update(data, datalen);
-    crypto_sha256_update(clientDataHash, CLIENT_DATA_HASH_SIZE);
-    crypto_sha256_final(hashbuf);
-
-    printf1(TAG_GREEN, "sha256: ");  dump_hex1(TAG_DUMP,hashbuf,32);
-    crypto_ecc256_sign(hashbuf, 32, sigbuf);
-
     // Need to caress into dumb der format ..
+    int i;
     int8_t lead_s = 0;  // leading zeros
     int8_t lead_r = 0;
     for (i=0; i < 32; i++)
@@ -382,10 +367,30 @@ int ctap_calculate_signature(uint8_t * data, int datalen, uint8_t * clientDataHa
     sigder[5 + 32 + pad_r - lead_r] = 0x20 + pad_s - lead_s;
     memmove(sigder + 6 + 32 + pad_r + pad_s - lead_r, sigbuf + 32 + lead_s, 32);
     //
-
     return 0x46 + pad_s + pad_r - lead_r - lead_s;
 }
 
+// require load_key prior to this
+// @data data to hash before signature
+// @clientDataHash for signature
+// @tmp buffer for hash.  (can be same as data if data >= 32 bytes)
+// @sigbuf location to deposit signature (must be 64 bytes)
+// @sigder location to deposit der signature (must be 72 bytes)
+// @return length of der signature
+int ctap_calculate_signature(uint8_t * data, int datalen, uint8_t * clientDataHash, uint8_t * hashbuf, uint8_t * sigbuf, uint8_t * sigder)
+{
+    // calculate attestation sig
+    crypto_sha256_init();
+    crypto_sha256_update(data, datalen);
+    crypto_sha256_update(clientDataHash, CLIENT_DATA_HASH_SIZE);
+    crypto_sha256_final(hashbuf);
+
+    printf1(TAG_GREEN, "sha256: ");  dump_hex1(TAG_DUMP,hashbuf,32);
+    crypto_ecc256_sign(hashbuf, 32, sigbuf);
+
+    return ctap_encode_der_sig(sigbuf,sigder);
+}
+
 uint8_t ctap_add_attest_statement(CborEncoder * map, uint8_t * sigder, int len)
 {
     int ret;
diff --git a/ctap.h b/ctap.h
index e37dd13..f747704 100644
--- a/ctap.h
+++ b/ctap.h
@@ -237,6 +237,9 @@ void ctap_response_init(CTAP_RESPONSE * resp);
 
 uint8_t ctap_handle_packet(uint8_t * pkt_raw, int length, CTAP_RESPONSE * resp);
 
+// Encodes R,S signature to 2 der sequence of two integers.  Sigder must be at least 72 bytes.
+// @return length of der signature
+int ctap_encode_der_sig(uint8_t * sigbuf, uint8_t * sigder);
 
 // Run ctap related power-up procedures (init pinToken, generate shared secret)
 void ctap_init();
diff --git a/u2f.c b/u2f.c
index 65f16e7..68973e5 100644
--- a/u2f.c
+++ b/u2f.c
@@ -104,33 +104,10 @@ static uint8_t get_signature_length(uint8_t * sig)
 
 static void dump_signature_der(uint8_t * sig)
 {
-    uint8_t pad_s = (sig[32] & 0x80) == 0x80;
-    uint8_t pad_r = (sig[0] & 0x80) == 0x80;
-    uint8_t i[] = {0x30, 0x44};
-    i[1] += (pad_s + pad_r);
-
-
-    // DER encoded signature
-    // write der sequence
-    // has to be minimum distance and padded with 0x00 if MSB is a 1.
-    u2f_response_writeback(i,2);
-    i[1] = 0;
-
-    // length of R value plus 0x00 pad if necessary
-    u2f_response_writeback("\x02",1);
-    i[0] = 0x20 + pad_r;
-    u2f_response_writeback(i,1 + pad_r);
-
-    // R value
-    u2f_response_writeback(sig, 32);
-
-    // length of S value plus 0x00 pad if necessary
-    u2f_response_writeback("\x02",1);
-    i[0] = 0x20 + pad_s;
-    u2f_response_writeback(i,1 + pad_s);
-
-    // S value
-    u2f_response_writeback(sig+32, 32);
+    uint8_t sigder[72];
+    int len;
+    len = ctap_encode_der_sig(sig, sigder);
+    u2f_response_writeback(sigder, len);
 }
 static int8_t u2f_load_key(struct u2f_key_handle * kh, uint8_t * appid)
 {