QCBOR: Make bstr wrapping compatible with COSE signing

QCBOREncode_CloseBstrWrap() now additionally returns the
wrapping bstr in the wrapped CBOR it returns. This is one
of a few fixes to make t_cose correctly compatible with
COSE-C and the COSE standard.

Change-Id: I58bcda54f9399817cd84f19b34d8d0adf88f2c06
Signed-off-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/lib/ext/qcbor/inc/qcbor.h b/lib/ext/qcbor/inc/qcbor.h
index 998645d..8da2f39 100644
--- a/lib/ext/qcbor/inc/qcbor.h
+++ b/lib/ext/qcbor/inc/qcbor.h
@@ -43,6 +43,7 @@
 
  when               who             what, where, why
  --------           ----            ---------------------------------------------------
+ 4/6/19             llundblade      Wrapped bstr returned now includes the wrapping bstr
  02/16/19           llundblade      Redesign MemPool to fix memory access alignment bug
  12/18/18           llundblade      Move decode malloc optional code to separate repository
  12/13/18           llundblade      Documentatation improvements
@@ -213,6 +214,7 @@
 // Must not conflict with any of the official CBOR types
 #define CBOR_MAJOR_NONE_TYPE_RAW  9
 #define CBOR_MAJOR_NONE_TAG_LABEL_REORDER 10
+#define CBOR_MAJOR_NONE_TYPE_BSTR_LEN_ONLY 11
 
 
 /* ===========================================================================
@@ -1539,15 +1541,16 @@
 /**
  @brief Close a wrapping bstr.
 
- @param[in] pCtx The context to add to.
- @param[out] pWrappedCBOR UsefulBufC containing wrapped bytes
+ @param[in]  pCtx          The context to add to.
+ @param[out] pWrappedCBOR  UsefulBufC containing wrapped bytes.
 
  The closes a wrapping bstr opened by QCBOREncode_BstrWrap(). It reduces
  nesting level by one.
 
- A pointer and length of the enclosed encoded CBOR is returned in
- *pWrappedCBOR if it is not NULL. The main purpose of this is so this
- data can be hashed (e.g., with SHA-256) as part of a COSE (RFC 8152)
+ A pointer and length of the wrapped and encoded CBOR is returned in
+ *pWrappedCBOR if it is not NULL. This includes the wrapping bstr
+ itself. The main purpose of this is so this data can be hashed
+ (e.g., with SHA-256) as part of a COSE (RFC 8152)
  implementation. **WARNING**, this pointer and length should be used
  right away before any other calls to QCBOREncode_xxxx() as they will
  move data around and the pointer and length will no longer be to the
@@ -2197,6 +2200,35 @@
 void  QCBOREncode_AddType7(QCBOREncodeContext *pCtx, size_t uSize, uint64_t uNum);
 
 
+/**
+ @brief Semi-private method to add only the type and length of a byte string.
+
+ @param[in] pCtx    The context to initialize.
+ @param[in] Bytes   Pointer and length of the input data.
+
+ This is the same as QCBOREncode_AddBytes() except it only adds the
+ CBOR encoding for the type and the length. It doesn't actually add
+ the bytes. You can't actually produce correct CBOR with this and the
+ rest of this API. It is only used for a special case where
+ the valid CBOR is created manually by putting this type and length in
+ and then adding the actual bytes. In particular, when only a hash of
+ the encoded CBOR is needed, where the type and header are hashed
+ separately and then the bytes is hashed. This makes it possible to
+ implement COSE Sign1 with only one copy of the payload in the output
+ buffer, rather than two, roughly cutting memory use in half.
+
+ This is only used for this odd case, but this is a supported
+ tested function.
+*/
+static inline void QCBOREncode_AddBytesLenOnly(QCBOREncodeContext *pCtx, UsefulBufC Bytes);
+
+static inline void QCBOREncode_AddBytesLenOnlyToMap(QCBOREncodeContext *pCtx, const char *szLabel, UsefulBufC Bytes);
+
+static inline void QCBOREncode_AddBytesLenOnlyToMapN(QCBOREncodeContext *pCtx, int64_t nLabel, UsefulBufC Bytes);
+
+
+
+
 static inline void QCBOREncode_AddInt64ToMap(QCBOREncodeContext *pCtx, const char *szLabel, int64_t uNum)
 {
    QCBOREncode_AddBuffer(pCtx, CBOR_MAJOR_TYPE_TEXT_STRING, UsefulBuf_FromSZ(szLabel)); // AddSZString not defined yet
@@ -2310,6 +2342,22 @@
    QCBOREncode_AddBytes(pCtx, Bytes);
 }
 
+static inline void QCBOREncode_AddBytesLenOnly(QCBOREncodeContext *pCtx, UsefulBufC Bytes)
+{
+    QCBOREncode_AddBuffer(pCtx, CBOR_MAJOR_NONE_TYPE_BSTR_LEN_ONLY, Bytes);
+}
+
+static inline void QCBOREncode_AddBytesLenOnlyToMap(QCBOREncodeContext *pCtx, const char *szLabel, UsefulBufC Bytes)
+{
+    QCBOREncode_AddSZString(pCtx, szLabel);
+    QCBOREncode_AddBytesLenOnly(pCtx, Bytes);
+}
+
+static inline void QCBOREncode_AddBytesLenOnlyToMapN(QCBOREncodeContext *pCtx, int64_t nLabel, UsefulBufC Bytes)
+{
+    QCBOREncode_AddInt64(pCtx, nLabel);
+    QCBOREncode_AddBytesLenOnly(pCtx, Bytes);
+}
 
 static inline void QCBOREncode_AddBinaryUUID(QCBOREncodeContext *pCtx, UsefulBufC Bytes)
 {
diff --git a/lib/ext/qcbor/src/qcbor_encode.c b/lib/ext/qcbor/src/qcbor_encode.c
index 460dd85..c652f79 100644
--- a/lib/ext/qcbor/src/qcbor_encode.c
+++ b/lib/ext/qcbor/src/qcbor_encode.c
@@ -42,6 +42,7 @@
 
  when               who             what, where, why
  --------           ----            ---------------------------------------------------
+ 4/6/19             llundblade      Wrapped bstr returned now includes the wrapping bstr
  12/30/18           llundblade      Small efficient clever encode of type & argument.
  11/29/18           llundblade      Rework to simpler handling of tags and labels.
  11/9/18            llundblade      Error codes are now enums.
@@ -392,26 +393,41 @@
 
 
 /*
- Semi-private function. It is exposed to user of the interface,
- but they will usually call one of the inline wrappers rather than this.
+ Semi-private function. It is exposed to user of the interface, but
+ they will usually call one of the inline wrappers rather than this.
 
  See header qcbor.h
 
- Does the work of adding some bytes to the CBOR output. Works for a
- byte and text strings, which are the same in in CBOR though they have
- different major types.  This is also used to insert raw
- pre-encoded CBOR.
+ Does the work of adding actual strings bytes to the CBOR output (as
+ opposed to numbers and opening / closing aggregate types).
+
+ There are four use cases:
+   CBOR_MAJOR_TYPE_BYTE_STRING -- Byte strings
+   CBOR_MAJOR_TYPE_TEXT_STRING -- Text strings
+   CBOR_MAJOR_NONE_TYPE_RAW -- Already-encoded CBOR
+   CBOR_MAJOR_NONE_TYPE_BSTR_LEN_ONLY -- Special case
+
+ The first two add the type and length plus the actual bytes. The
+ third just adds the bytes as the type and length are presumed to be
+ in the bytes. The fourth just adds the type and length for the very
+ special case of QCBOREncode_AddBytesLenOnly().
  */
 void QCBOREncode_AddBuffer(QCBOREncodeContext *me, uint8_t uMajorType, UsefulBufC Bytes)
 {
    if(me->uError == QCBOR_SUCCESS) {
       // If it is not Raw CBOR, add the type and the length
       if(uMajorType != CBOR_MAJOR_NONE_TYPE_RAW) {
-         AppendEncodedTypeAndNumber(me, uMajorType, Bytes.len);
+         uint8_t uRealMajorType = uMajorType;
+         if(uRealMajorType == CBOR_MAJOR_NONE_TYPE_BSTR_LEN_ONLY) {
+            uRealMajorType = CBOR_MAJOR_TYPE_BYTE_STRING;
+         }
+         AppendEncodedTypeAndNumber(me, uRealMajorType, Bytes.len);
       }
 
-      // Actually add the bytes
-      UsefulOutBuf_AppendUsefulBuf(&(me->OutBuf), Bytes);
+      if(uMajorType != CBOR_MAJOR_NONE_TYPE_BSTR_LEN_ONLY) {
+         // Actually add the bytes
+         UsefulOutBuf_AppendUsefulBuf(&(me->OutBuf), Bytes);
+      }
 
       // Update the array counting if there is any nesting at all
       me->uError = Nesting_Increment(&(me->nesting));
@@ -541,12 +557,11 @@
          // Return pointer and length to the enclosed encoded CBOR. The intended
          // use is for it to be hashed (e.g., SHA-256) in a COSE implementation.
          // This must be used right away, as the pointer and length go invalid
-         // on any subsequent calls to this function because of the
-         // InsertEncodedTypeAndNumber() call that slides data to the right.
+         // on any subsequent calls to this function because there might be calls to
+         // InsertEncodedTypeAndNumber() that slides data to the right.
          if(pWrappedCBOR) {
             const UsefulBufC PartialResult = UsefulOutBuf_OutUBuf(&(me->OutBuf));
-            const size_t uBstrLen = UsefulOutBuf_GetEndPosition(&(me->OutBuf)) - uEndPosition;
-            *pWrappedCBOR = UsefulBuf_Tail(PartialResult, uInsertPosition+uBstrLen);
+            *pWrappedCBOR = UsefulBuf_Tail(PartialResult, uInsertPosition);
          }
          Nesting_Decrease(&(me->nesting));
       }
@@ -573,7 +588,7 @@
    }
 
    if(UsefulOutBuf_GetError(&(me->OutBuf))) {
-      // Stuff didn't fit in the buffer.
+      // Items didn't fit in the buffer.
       // This check catches this condition for all the appends and inserts
       // so checks aren't needed when the appends and inserts are performed.
       // And of course UsefulBuf will never overrun the input buffer given
diff --git a/lib/ext/qcbor/test/qcbor_encode_tests.c b/lib/ext/qcbor/test/qcbor_encode_tests.c
index 2a22cf1..89db5e5 100644
--- a/lib/ext/qcbor/test/qcbor_encode_tests.c
+++ b/lib/ext/qcbor/test/qcbor_encode_tests.c
@@ -1370,6 +1370,8 @@
 
 
 /*
+ The expected encoding for first test in BstrWrapTest()
+
  82           # array(2)
    19 01C3   # unsigned(451)
    43        # bytes(3)
@@ -1378,12 +1380,19 @@
 static const uint8_t spExpectedBstrWrap[] = {0x82, 0x19, 0x01, 0xC3, 0x43, 0x19, 0x01, 0xD2};
 
 /*
+ 81   #array(1)
+ 0x58  0x25  # string of length 37 (length of "This is longer than twenty four bytes")
+ */
+static const uint8_t spExpectedTypeAndLen[] = {0x81, 0x58, 0x25};
+
+/*
  Very basic bstr wrapping test
  */
 int BstrWrapTest()
 {
    QCBOREncodeContext EC;
 
+   // First test - make some wrapped CBOR and see that it is as expected
    QCBOREncode_Init(&EC, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
 
    QCBOREncode_OpenArray(&EC);
@@ -1406,7 +1415,9 @@
       return -2;
    }
 
-   /* Another test; see about handling length calculation */
+   // Second test - see if the length of the wrapped
+   // bstr is correct. Also tests bstr wrapping
+   // in length calculation only mode.
    QCBOREncode_Init(&EC, (UsefulBuf){NULL, INT32_MAX});
    QCBOREncode_OpenArray(&EC);
    QCBOREncode_BstrWrap(&EC);
@@ -1415,11 +1426,25 @@
    QCBOREncode_CloseArray(&EC);
    UsefulBufC BStr;
    QCBOREncode_CloseBstrWrap(&EC, &BStr);
-   // 2 is one byte for an array of length 1 and 1 byte for a NULL
-   if(BStr.ptr != NULL || BStr.len != 2) {
+   // 3 is one byte for the wrapping bstr, 1 for an array of length 1, and 1 byte for a NULL
+   if(BStr.ptr != NULL || BStr.len != 3) {
       return -5;
    }
 
+   // Third, test QCBOREncode_AddBytesLenOnly() here as it is part of the
+   // bstr wrapping use cases.
+   UsefulBuf_MAKE_STACK_UB(StuffBuf, 50);
+   QCBOREncode_Init(&EC, StuffBuf);
+   QCBOREncode_OpenArray(&EC);
+   QCBOREncode_AddBytesLenOnly(&EC, UsefulBuf_FROM_SZ_LITERAL("This is longer than twenty four bytes"));
+   QCBOREncode_CloseArray(&EC);
+   if(QCBOREncode_Finish(&EC, &Encoded)) {
+      return -6;
+   }
+   if(CheckResults(Encoded, spExpectedTypeAndLen)) {
+      return -7;
+   }
+
    return 0;
 }
 
@@ -1761,7 +1786,7 @@
 }
 
 
-static const uint8_t spSignature[] = {
+static const uint8_t spCoseSign1Signature[] = {
    0x8e, 0xb3, 0x3e, 0x4c, 0xa3, 0x1d, 0x1c, 0x46, 0x5a, 0xb0,
    0x5a, 0xac, 0x34, 0xcc, 0x6b, 0x23, 0xd5, 0x8f, 0xef, 0x5c,
    0x08, 0x31, 0x06, 0xc4, 0xd2, 0x5a, 0x91, 0xae, 0xf0, 0xb0,
@@ -1782,9 +1807,16 @@
       54                                # bytes(20)
          546869732069732074686520636F6E74656E742E # "This is the content."
       58 40                             # bytes(64)
-         8EB33E4CA31D1C465AB05AAC34CC6B23D58FEF5C083106C4D25A91AEF0B0117E2AF9A291AA32E14AB834DC56ED2A223444547E01F11D3B0916E5A4C345CACB36 # "\x8E\xB3>L\xA3\x1D\x1CFZ\xB0Z\xAC4\xCCk#\xD5\x8F\xEF\\\b1\x06\xC4\xD2Z\x91\xAE\xF0\xB0\x11~*\xF9\xA2\x91\xAA2\xE1J\xB84\xDCV\xED*\"4DT~\x01\xF1\x1D;\t\x16\xE5\xA4\xC3E\xCA\xCB6"
+         8EB33E4CA31D1C465AB05AAC34CC6B23D58FEF5C083106C4D25
+         A91AEF0B0117E2AF9A291AA32E14AB834DC56ED2A223444547E
+         01F11D3B0916E5A4C345CACB36     # "\x8E\xB3>L\xA3\x1D\x1CFZ\xB0Z\xAC4
+                                           \xCCk#\xD5\x8F\xEF\b1\x06\xC4\xD2Z
+                                           \x91\xAE\xF0\xB0\x11~*\xF9\xA2\x91
+                                           \xAA2\xE1J\xB84\xDCV\xED*\"4DT~\x01
+                                           \xF1\x1D;\t\x16\xE5\xA4\xC3E\xCA
+                                           \xCB6"
  */
-static const uint8_t spExpected[] = {
+static const uint8_t spCoseSign1TBSExpected[] = {
    0xD2, 0x84, 0x43, 0xA1, 0x01, 0x26, 0xA1, 0x04, 0x42, 0x31,
    0x31, 0x54, 0x54, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20,
    0x74, 0x68, 0x65, 0x20, 0x63, 0x6F, 0x6E, 0x74, 0x65, 0x6E,
@@ -1796,27 +1828,24 @@
    0x22, 0x34, 0x44, 0x54, 0x7E, 0x01, 0xF1, 0x1D, 0x3B, 0x09,
    0x16, 0xE5, 0xA4, 0xC3, 0x45, 0xCA, 0xCB, 0x36};
 
+static const uint8_t pProtectedHeaders[] = {0xa1, 0x01, 0x26};
+
+
 /*
- this corresponds exactly to the example in RFC 8152
- section C.2.1. This doesn't actually verify the signature
- though that would be nice as it would make the test
- really good. That would require bring in ECDSA crypto
- to this test.
+ This corresponds exactly to the example in RFC 8152 section
+ C.2.1. This doesn't actually verify the signature though that would
+ be nice as it would make the test really good. That would require
+ bring in ECDSA crypto to this test.
  */
 int CoseSign1TBSTest()
 {
    // All of this is from RFC 8152 C.2.1
-   const char *szKid = "11";
-   const UsefulBufC Kid = UsefulBuf_FromSZ(szKid);
-   const char *szPayload = "This is the content.";
-   const UsefulBufC Payload = UsefulBuf_FromSZ(szPayload);
-   static const uint8_t pProtectedHeaders[] = {0xa1, 0x01, 0x26};
-   const UsefulBufC ProtectedHeaders = UsefulBuf_FROM_BYTE_ARRAY_LITERAL(pProtectedHeaders);
-
-   // It would be good to compare this to the output from
-   // a COSE implementation like COSE-C. It has been checked
-   // against the CBOR playground.
-   const UsefulBufC Signature = UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spSignature);
+   const char          *szKid     = "11";
+   const UsefulBufC     Kid       = UsefulBuf_FromSZ(szKid);
+   const char          *szPayload = "This is the content.";
+   const UsefulBufC     Payload   = UsefulBuf_FromSZ(szPayload);
+   const UsefulBufC     ProtectedHeaders = UsefulBuf_FROM_BYTE_ARRAY_LITERAL(pProtectedHeaders);
+   const UsefulBufC     Signature        = UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spCoseSign1Signature);
 
    QCBOREncodeContext EC;
    QCBOREncode_Init(&EC, UsefulBuf_FROM_BYTE_ARRAY(spBigBuf));
@@ -1836,11 +1865,15 @@
    // The payload
    UsefulBufC WrappedPayload;
    QCBOREncode_BstrWrap(&EC);
-   QCBOREncode_AddEncoded(&EC, Payload); // Payload is not actually CBOR in example C.2.1
+   // Payload is not actually CBOR in example C.2.1 like it would be
+   // for a CWT or EAT. It is just a text string.
+   QCBOREncode_AddEncoded(&EC, Payload);
    QCBOREncode_CloseBstrWrap(&EC, &WrappedPayload);
 
    // Check we got back the actual payload expected
-   if(UsefulBuf_Compare(WrappedPayload, Payload)) {
+   // The extra "T" is 0x54, which is the initial byte a bstr of length 20.
+   if(UsefulBuf_Compare(WrappedPayload,
+                        UsefulBuf_FROM_SZ_LITERAL("TThis is the content."))) {
       return -1;
    }
 
@@ -1859,7 +1892,10 @@
       return -3;
    }
 
-   if(CheckResults(COSE_Sign1, spExpected)) {
+   // It would be good to compare this to the output from a COSE
+   // implementation like COSE-C. This has been checked against the
+   // CBOR playground.
+   if(CheckResults(COSE_Sign1, spCoseSign1TBSExpected)) {
       return -4;
    }