Bug 1061021, Part 7: Stop using PLArenaPool for SignedData encoding, r=keeler
authorBrian Smith <brian@briansmith.org>
Sat, 30 Aug 2014 17:40:41 -0700
changeset 14678 bc91f4793d5ad18741284d523d2a27d268be0951
parent 14677 90d09ed5c83b018b9f953e6697cf73a6b5452141
child 14679 cbd4132642d4c8df7359f071aa3d5243e002dac7
push id3202
push userfranziskuskiefer@gmail.com
push dateMon, 01 Oct 2018 08:30:12 +0000
reviewerskeeler
bugs1061021
Bug 1061021, Part 7: Stop using PLArenaPool for SignedData encoding, r=keeler
lib/mozpkix/test/lib/pkixtestutil.cpp
--- a/lib/mozpkix/test/lib/pkixtestutil.cpp
+++ b/lib/mozpkix/test/lib/pkixtestutil.cpp
@@ -144,16 +144,27 @@ TLV(uint8_t tag, const ByteString& value
   } else {
     assert(false);
     return ENCODING_FAILED;
   }
   result.append(value);
   return result;
 }
 
+static SECItem*
+ArenaDupByteString(PLArenaPool* arena, const ByteString& value)
+{
+  SECItem* result = SECITEM_AllocItem(arena, nullptr, value.length());
+  if (!result) {
+    return nullptr;
+  }
+  memcpy(result->data, value.data(), value.length());
+  return result;
+}
+
 class Output
 {
 public:
   Output()
   {
   }
 
   // Makes a shallow copy of the input item. All input items must have a
@@ -231,18 +242,18 @@ OCSPResponseContext::OCSPResponseContext
   , certStatus(good)
   , revocationTime(0)
   , thisUpdate(time)
   , nextUpdate(time + 10)
   , includeNextUpdate(true)
 {
 }
 
-static SECItem* ResponseBytes(OCSPResponseContext& context);
-static SECItem* BasicOCSPResponse(OCSPResponseContext& context);
+static ByteString ResponseBytes(OCSPResponseContext& context);
+static ByteString BasicOCSPResponse(OCSPResponseContext& context);
 static SECItem* ResponseData(OCSPResponseContext& context);
 static ByteString ResponderID(OCSPResponseContext& context);
 static ByteString KeyHash(OCSPResponseContext& context);
 static SECItem* SingleResponse(OCSPResponseContext& context);
 static SECItem* CertID(OCSPResponseContext& context);
 static ByteString CertStatus(OCSPResponseContext& context);
 
 static SECItem*
@@ -451,97 +462,79 @@ YMDHMS(int16_t year, int16_t month, int1
 
   uint64_t totalSeconds = days * Time::ONE_DAY_IN_SECONDS;
   totalSeconds += hour * 60 * 60;
   totalSeconds += minutes * 60;
   totalSeconds += seconds;
   return TimeFromElapsedSecondsAD(totalSeconds);
 }
 
-static SECItem*
-SignedData(PLArenaPool* arena, const SECItem* tbsData,
+static ByteString
+SignedData(const SECItem* tbsData,
            SECKEYPrivateKey* privKey,
            SignatureAlgorithm signatureAlgorithm,
            bool corrupt, /*optional*/ SECItem const* const* certs)
 {
-  assert(arena);
   assert(tbsData);
   assert(privKey);
-  if (!arena || !tbsData || !privKey) {
-    return nullptr;
+  if (!tbsData || !privKey) {
+    return ENCODING_FAILED;
   }
 
   SECOidTag signatureAlgorithmOidTag;
-  Input signatureAlgorithmDER;
+  ByteString signatureAlgorithmDER;
   switch (signatureAlgorithm) {
     case SignatureAlgorithm::rsa_pkcs1_with_sha256:
       signatureAlgorithmOidTag = SEC_OID_PKCS1_SHA256_WITH_RSA_ENCRYPTION;
-      if (signatureAlgorithmDER.Init(alg_sha256WithRSAEncryption,
-                                     sizeof(alg_sha256WithRSAEncryption))
-            != Success) {
-        return nullptr;
-      }
+      signatureAlgorithmDER.assign(alg_sha256WithRSAEncryption,
+                                   sizeof(alg_sha256WithRSAEncryption));
       break;
     default:
-      return nullptr;
+      return ENCODING_FAILED;
   }
 
-  // SEC_SignData doesn't take an arena parameter, so we have to manage
-  // the memory allocated in signature.
   SECItem signature;
   if (SEC_SignData(&signature, tbsData->data, tbsData->len, privKey,
                    signatureAlgorithmOidTag) != SECSuccess)
   {
     return nullptr;
   }
   // TODO: add ability to have signatures of bit length not divisible by 8,
   // resulting in unused bits in the bitstring encoding
   ByteString signatureNested(BitString(ByteString(signature.data, signature.len),
                                        corrupt));
   SECITEM_FreeItem(&signature, false);
   if (signatureNested == ENCODING_FAILED) {
     return nullptr;
   }
 
-  SECItem* certsNested = nullptr;
+  ByteString certsNested;
   if (certs) {
-    Output certsOutput;
+    ByteString certsSequenceValue;
     while (*certs) {
-      certsOutput.Add(*certs);
+      certsSequenceValue.append(ByteString((*certs)->data, (*certs)->len));
       ++certs;
     }
-    SECItem* certsSequence = certsOutput.Squash(arena, der::SEQUENCE);
-    if (!certsSequence) {
-      return nullptr;
+    ByteString certsSequence(TLV(der::SEQUENCE, certsSequenceValue));
+    if (certsSequence == ENCODING_FAILED) {
+      return ENCODING_FAILED;
     }
-    certsNested = EncodeNested(arena,
-                               der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0,
-                               certsSequence);
-    if (!certsNested) {
-      return nullptr;
+    certsNested = TLV(der::CONTEXT_SPECIFIC | der::CONSTRUCTED | 0,
+                      certsSequence);
+    if (certsNested == ENCODING_FAILED) {
+      return ENCODING_FAILED;
     }
   }
 
-  Output output;
-  if (output.Add(tbsData) != Success) {
-    return nullptr;
-  }
-
-  SECItem sigantureAlgorithmDERItem =
-    UnsafeMapInputToSECItem(signatureAlgorithmDER);
-  if (output.Add(&sigantureAlgorithmDERItem) != Success) {
-    return nullptr;
-  }
-  output.Add(signatureNested);
-  if (certsNested) {
-    if (output.Add(certsNested) != Success) {
-      return nullptr;
-    }
-  }
-  return output.Squash(arena, der::SEQUENCE);
+  ByteString value;
+  value.append(ByteString(tbsData->data, tbsData->len));
+  value.append(signatureAlgorithmDER);
+  value.append(signatureNested);
+  value.append(certsNested);
+  return TLV(der::SEQUENCE, value);
 }
 
 // Extension  ::=  SEQUENCE  {
 //      extnID      OBJECT IDENTIFIER,
 //      critical    BOOLEAN DEFAULT FALSE,
 //      extnValue   OCTET STRING
 //                  -- contains the DER encoding of an ASN.1 value
 //                  -- corresponding to the extension type identified
@@ -581,43 +574,37 @@ Extension(PLArenaPool* arena, Input extn
   }
   if (output.Add(extnValue) != Success) {
     return nullptr;
   }
 
   return output.Squash(arena, der::SEQUENCE);
 }
 
-SECItem*
-MaybeLogOutput(SECItem* result, const char* suffix)
+void
+MaybeLogOutput(const ByteString& result, const char* suffix)
 {
   assert(suffix);
 
-  if (!result) {
-    return nullptr;
-  }
-
   // This allows us to more easily debug the generated output, by creating a
   // file in the directory given by MOZILLA_PKIX_TEST_LOG_DIR for each
   // NOT THREAD-SAFE!!!
   const char* logPath = getenv("MOZILLA_PKIX_TEST_LOG_DIR");
   if (logPath) {
     static int counter = 0;
     ScopedPtr<char, PR_smprintf_free>
       filename(PR_smprintf("%u-%s.der", counter, suffix));
     ++counter;
     if (filename) {
       ScopedFILE file(OpenFile(logPath, filename.get(), "wb"));
       if (file) {
-        (void) fwrite(result->data, result->len, 1, file.get());
+        (void) fwrite(result.data(), result.length(), 1, file.get());
       }
     }
   }
-
-  return result;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // Key Pairs
 
 Result
 GenerateKeyPair(/*out*/ ScopedSECKEYPublicKey& publicKey,
                 /*out*/ ScopedSECKEYPrivateKey& privateKey)
@@ -706,27 +693,29 @@ CreateEncodedCertificate(PLArenaPool* ar
   SECItem* tbsCertificate(TBSCertificate(arena, version, serialNumber,
                                          signature, issuerNameDER, notBefore,
                                          notAfter, subjectNameDER,
                                          publicKey.get(), extensions));
   if (!tbsCertificate) {
     return nullptr;
   }
 
-  SECItem*
-    result(MaybeLogOutput(SignedData(arena, tbsCertificate,
-                                     issuerPrivateKey ? issuerPrivateKey
-                                                      : privateKeyTemp.get(),
-                                     signatureAlgorithm, false, nullptr),
-                          "cert"));
-  if (!result) {
+  ByteString result(SignedData(tbsCertificate,
+                               issuerPrivateKey ? issuerPrivateKey
+                                                : privateKeyTemp.get(),
+                               signatureAlgorithm, false, nullptr));
+  if (result == ENCODING_FAILED) {
     return nullptr;
   }
+
+  MaybeLogOutput(result, "cert");
+
   privateKeyResult = privateKeyTemp.release();
-  return result;
+
+  return ArenaDupByteString(arena, result);
 }
 
 // TBSCertificate  ::=  SEQUENCE  {
 //      version         [0]  Version DEFAULT v1,
 //      serialNumber         CertificateSerialNumber,
 //      signature            AlgorithmIdentifier,
 //      issuer               Name,
 //      validity             Validity,
@@ -983,103 +972,91 @@ CreateEncodedOCSPResponse(OCSPResponseCo
   //    successful          (0),  -- Response has valid confirmations
   //    malformedRequest    (1),  -- Illegal confirmation request
   //    internalError       (2),  -- Internal error in issuer
   //    tryLater            (3),  -- Try again later
   //                              -- (4) is not used
   //    sigRequired         (5),  -- Must sign the request
   //    unauthorized        (6)   -- Request unauthorized
   // }
-  SECItem* responseStatus = SECITEM_AllocItem(context.arena, nullptr, 3);
-  if (!responseStatus) {
+  ByteString reponseStatusValue;
+  reponseStatusValue.push_back(context.responseStatus);
+  ByteString responseStatus(TLV(der::ENUMERATED, reponseStatusValue));
+  if (responseStatus == ENCODING_FAILED) {
     return nullptr;
   }
-  responseStatus->data[0] = der::ENUMERATED;
-  responseStatus->data[1] = 1;
-  responseStatus->data[2] = context.responseStatus;
 
-  SECItem* responseBytesNested = nullptr;
+  ByteString responseBytesNested;
   if (!context.skipResponseBytes) {
-    SECItem* responseBytes = ResponseBytes(context);
-    if (!responseBytes) {
+    ByteString responseBytes(ResponseBytes(context));
+    if (responseBytes == ENCODING_FAILED) {
       return nullptr;
     }
 
-    responseBytesNested = EncodeNested(context.arena,
-                                       der::CONSTRUCTED |
-                                       der::CONTEXT_SPECIFIC,
-                                       responseBytes);
-    if (!responseBytesNested) {
+    responseBytesNested = TLV(der::CONSTRUCTED | der::CONTEXT_SPECIFIC,
+                              responseBytes);
+    if (responseBytesNested == ENCODING_FAILED) {
       return nullptr;
     }
   }
 
-  Output output;
-  if (output.Add(responseStatus) != Success) {
+  ByteString value;
+  value.append(responseStatus);
+  value.append(responseBytesNested);
+  ByteString result(TLV(der::SEQUENCE, value));
+  if (result == ENCODING_FAILED) {
     return nullptr;
   }
-  if (responseBytesNested) {
-    if (output.Add(responseBytesNested) != Success) {
-      return nullptr;
-    }
-  }
-  return MaybeLogOutput(output.Squash(context.arena, der::SEQUENCE), "ocsp");
+
+  MaybeLogOutput(result, "ocsp");
+
+  return ArenaDupByteString(context.arena, result);
 }
 
 // ResponseBytes ::= SEQUENCE {
 //    responseType            OBJECT IDENTIFIER,
 //    response                OCTET STRING }
-SECItem*
+ByteString
 ResponseBytes(OCSPResponseContext& context)
 {
   // Includes tag and length
   static const uint8_t id_pkix_ocsp_basic_encoded[] = {
     0x06, 0x09, 0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x30, 0x01, 0x01
   };
-  SECItem id_pkix_ocsp_basic = {
-    siBuffer,
-    const_cast<uint8_t*>(id_pkix_ocsp_basic_encoded),
-    sizeof(id_pkix_ocsp_basic_encoded)
-  };
-  SECItem* response = BasicOCSPResponse(context);
-  if (!response) {
+  ByteString response(BasicOCSPResponse(context));
+  if (response == ENCODING_FAILED) {
     return nullptr;
   }
-  SECItem* responseNested = EncodeNested(context.arena, der::OCTET_STRING,
-                                         response);
-  if (!responseNested) {
+  ByteString responseNested = TLV(der::OCTET_STRING, response);
+  if (responseNested == ENCODING_FAILED) {
     return nullptr;
   }
 
-  Output output;
-  if (output.Add(&id_pkix_ocsp_basic) != Success) {
-    return nullptr;
-  }
-  if (output.Add(responseNested) != Success) {
-    return nullptr;
-  }
-  return output.Squash(context.arena, der::SEQUENCE);
+  ByteString value;
+  value.append(id_pkix_ocsp_basic_encoded,
+               sizeof(id_pkix_ocsp_basic_encoded));
+  value.append(responseNested);
+  return TLV(der::SEQUENCE, value);
 }
 
 // BasicOCSPResponse ::= SEQUENCE {
 //   tbsResponseData          ResponseData,
 //   signatureAlgorithm       AlgorithmIdentifier,
 //   signature                BIT STRING,
 //   certs                [0] EXPLICIT SEQUENCE OF Certificate OPTIONAL }
-SECItem*
+ByteString
 BasicOCSPResponse(OCSPResponseContext& context)
 {
   SECItem* tbsResponseData = ResponseData(context);
   if (!tbsResponseData) {
     return nullptr;
   }
 
   // TODO(bug 980538): certs
-  return SignedData(context.arena, tbsResponseData,
-                    context.signerPrivateKey.get(),
+  return SignedData(tbsResponseData, context.signerPrivateKey.get(),
                     SignatureAlgorithm::rsa_pkcs1_with_sha256,
                     context.badSignature, context.certs);
 }
 
 // Extension ::= SEQUENCE {
 //   id               OBJECT IDENTIFIER,
 //   critical         BOOLEAN DEFAULT FALSE
 //   value            OCTET STRING