COSE: Correct compatibility with COSE standard

Corrections to construction of Sig_structure (TBS bytes). These require
an updated QCBOR. Verified against COSE-C implementation (Bug fixes
and substantial change to COSE-C was required so it is not possible
to test t_cose against COSE-C in GitHub)

Change-Id: I477dcf15192ad5df0310ea123bed8d48b3744740
Signed-off-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/lib/t_cose/src/t_cose_util.c b/lib/t_cose/src/t_cose_util.c
index 4228c2b..6026c6e 100644
--- a/lib/t_cose/src/t_cose_util.c
+++ b/lib/t_cose/src/t_cose_util.c
@@ -20,6 +20,8 @@
  *
  * \brief Implementation of t_cose utility functions.
  *
+ * These are some functions common to signing and
+ * verification.
  */
 
 
@@ -28,17 +30,16 @@
  */
 int32_t hash_alg_id_from_sig_alg_id(int32_t cose_sig_alg_id)
 {
-    /* if other hashes, particularly those that output bigger hashes
+    /* If other hashes, particularly those that output bigger hashes
      * are added here, various other parts of this code have to be
      * changed to have larger buffers.
      */
     switch(cose_sig_alg_id) {
+    case COSE_ALGORITHM_ES256:
+        return COSE_ALG_SHA256_PROPRIETARY;
 
-        case COSE_ALGORITHM_ES256:
-            return COSE_ALG_SHA256_PROPRIETARY;
-
-        default:
-            return INT32_MAX;
+    default:
+        return INT32_MAX;
     }
 }
 
@@ -55,21 +56,30 @@
  *    external_aad : bstr,
  *    payload : bstr
  * ]
+ *
+ * body_protected refers to the protected headers from the
+ * main COSE_Sign1 structure. This is a little hard to
+ * to understand in the spec.
+ *
+ * sign_protected is not used with COSE_Sign1 since
+ * there is no signer chunk.
+ *
+ * external_aad allows external data to be covered
+ * by the hash, but is not supported by this implementation.
  */
 
 
 /**
  * This is the size of the first part of the CBOR encoded TBS
- * bytes. It is around 20 bytes. See create_tbs_hash().
+ * bytes. It is around 30 bytes. See create_tbs_hash().
  */
 #define T_COSE_SIZE_OF_TBS \
     1 + /* For opening the array */ \
     sizeof(COSE_SIG_CONTEXT_STRING_SIGNATURE1) + /* "Signature1" */ \
     2 + /* Overhead for encoding string */ \
     T_COSE_SIGN1_MAX_PROT_HEADER + /* entire protected headers */ \
-    3 * ( /* 3 NULL bstrs for fields not used */ \
-        1 /* size of a NULL bstr */  \
-    )
+    1 + /* Empty bstr for absent external_aad */ \
+    9 /* The max CBOR length encoding for start of payload */
 
 
 /*
@@ -79,11 +89,15 @@
                                   struct q_useful_buf buffer_for_hash,
                                   struct q_useful_buf_c *hash,
                                   struct q_useful_buf_c protected_headers,
+                                  enum t_cose_tbs_hash_mode_t payload_mode,
                                   struct q_useful_buf_c payload)
 {
     /* approximate stack use on 32-bit machine:
-     * local use: 320
-     * with calls: 360
+     * local use: 210
+     * total with calls: 250
+     * Can be another 128 bytes or so depending on
+     * t_cose_crypto_hash implementation. It sometimes
+     * includes the full hashing context.
      */
     enum t_cose_err_t           return_value;
     QCBOREncodeContext          cbor_encode_ctx;
@@ -92,6 +106,7 @@
     QCBORError                  qcbor_result;
     struct t_cose_crypto_hash   hash_ctx;
     int32_t                     hash_alg_id;
+    size_t                      bytes_to_omit;
 
     /* This builds the CBOR-format to-be-signed bytes */
     QCBOREncode_Init(&cbor_encode_ctx, buffer_for_TBS_first_part);
@@ -102,19 +117,37 @@
     /* body_protected */
     QCBOREncode_AddBytes(&cbor_encode_ctx,
                          protected_headers);
-    /* sign_protected */
+    /* sign_protected is not used for Sign1 */
+
+    /* external_aad. There is none so an empty bstr */
     QCBOREncode_AddBytes(&cbor_encode_ctx, NULL_Q_USEFUL_BUF_C);
-    /* external_aad */
-    QCBOREncode_AddBytes(&cbor_encode_ctx, NULL_Q_USEFUL_BUF_C);
-    /* fake payload */
-    QCBOREncode_AddBytes(&cbor_encode_ctx, NULL_Q_USEFUL_BUF_C);
+
+    /* The short fake payload. */
+    if(payload_mode == T_COSE_TBS_PAYLOAD_IS_BSTR_WRAPPED) {
+        /* Fake payload is just an empty bstr. It is here only
+         * to make the array count right. It must be ommitted
+         * in the actual hashing below
+         */
+        bytes_to_omit = 1;
+        QCBOREncode_AddBytes(&cbor_encode_ctx, NULL_Q_USEFUL_BUF_C);
+    } else {
+        /* Fake payload is the type and length of the wrapping
+         * bstr. It gets hashed with the first part, so no
+         * bytes to omit.
+         */
+        bytes_to_omit = 0;
+        QCBOREncode_AddBytesLenOnly(&cbor_encode_ctx, payload);
+    }
+    /* Cleverness only works because the payload is last in the array */
+
+    /* Close of the array */
     QCBOREncode_CloseArray(&cbor_encode_ctx);
 
-    /* get the result and convert it to struct q_useful_buf_c representation */
+    /* get the encoded results, except for payload */
     qcbor_result = QCBOREncode_Finish(&cbor_encode_ctx, &tbs_first_part);
     if(qcbor_result) {
         /* Mainly means that the protected_headers were too big
-         (which should never happen) */
+         * (which should never happen) */
         return_value = T_COSE_ERR_SIG_STRUCT;
         goto Done;
     }
@@ -122,32 +155,43 @@
     /* Start the hashing */
     hash_alg_id = hash_alg_id_from_sig_alg_id(cose_alg_id);
     /* Don't check hash_alg_id for failure. t_cose_crypto_hash_start()
-     will handle it properly
+     * will handle it properly.
      */
     return_value = t_cose_crypto_hash_start(&hash_ctx, hash_alg_id);
     if(return_value) {
         goto Done;
     }
 
-    /* Hash the first part of the TBS. Take all but the last two
-     * bytes. The last two bytes are the fake payload from above. It
-     * is replaced by the real payload which is hashed next. The fake
-     * payload is needed so the array count is right. This is one of
-     * the main things that make it possible to implement with one
-     * buffer for the whole cose sign1.
+    /* This structure is hashed in two parts. The first part is
+     * the CBOR-formatted array with protected headers and such.
+     * The last part is the actual bytes of the payload. Doing it
+     * this way avoids having to allocate a big buffer to hold
+     * these two parts together.  It avoids having two copies of
+     * the payload in the implementaiton as the payload as formatted
+     * in the output buffer can be what is hashed. They payload
+     * is the largest memory use, so this saves a lot.
+     *
+     * This is further complicated because the the payload
+     * does have to be wrapped in a bstr. It is done one way
+     * when signing and another when verifying.
+     */
+
+    /* This is hashing of the first part, all the CBOR
+     * except the payload.
      */
     t_cose_crypto_hash_update(&hash_ctx,
                               q_useful_buf_head(tbs_first_part,
-                                                tbs_first_part.len - 2));
+                                        tbs_first_part.len - bytes_to_omit));
 
-    /* Hash the payload */
+    /* Hash the payload, the second part. This may or may not
+     * have the bstr wrapping. If not, it was hashed above.
+     */
     t_cose_crypto_hash_update(&hash_ctx, payload);
 
     /* Finish the hash and set up to return it */
     return_value = t_cose_crypto_hash_finish(&hash_ctx,
                                              buffer_for_hash,
                                              hash);
-
 Done:
     return return_value;
 }