Added CreateOriginBound method to x509_certificate.h.

net/base/x509_certificate_nss.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/x509_certificate.h"
 
 #include <cert.h>
 #include <cryptohi.h>
 #include <keyhi.h>
 #include <nss.h>
@@ skipping 737 matching lines @@
 
   // Save the signed result to the cert.
   cert->derCert = *result;
 
   X509Certificate* x509_cert =
       CreateFromHandle(cert, SOURCE_LONE_CERT_IMPORT, OSCertHandles());
   CERT_DestroyCertificate(cert);
   return x509_cert;
 }
 
+// static
+X509Certificate* X509Certificate::CreateOriginBound(

[wtc, 2011/08/04 00:37:53]

The CreateOriginBound function is identical to CreateSelfSigned
except that it has an additional block of code in the middle
(lines 807-871).  We should refactor to share code.

[mdietz, 2011/08/18 00:02:45]

The create origin bound code block in the middle can't be moved to anywhere
other than where it is now, and it doesn't make sense to overload
CreateSelfSigned with a parameter that allows it to create origin bound certs. 
So I split out the certificate creation and signing code blocks into new private
functions that both CreateOriginBound and CreateSelfSigned make use of.

+    crypto::RSAPrivateKey* key,
+    const std::string& subject,
+    const std::string& origin,
+    uint32 serial_number,
+    base::TimeDelta valid_duration) {
+  DCHECK(key);
+
+  // Create info about public key.
+  CERTSubjectPublicKeyInfo* spki =
+      SECKEY_CreateSubjectPublicKeyInfo(key->public_key());
+  if (!spki)
+    return NULL;
+
+  // Create the certificate request.
+  CERTName* subject_name =
+      CERT_AsciiToName(const_cast<char*>(subject.c_str()));
+  CERTCertificateRequest* cert_request =
+      CERT_CreateCertificateRequest(subject_name, spki, NULL);
+  SECKEY_DestroySubjectPublicKeyInfo(spki);
+
+  if (!cert_request) {
+    PRErrorCode prerr = PR_GetError();
+    LOG(ERROR) << "Failed to create certificate request: " << prerr;
+    CERT_DestroyName(subject_name);
+    return NULL;
+  }
+
+  PRTime now = PR_Now();
+  PRTime not_after = now + valid_duration.InMicroseconds();
+
+  // Note that the time is now in micro-second unit.
+  CERTValidity* validity = CERT_CreateValidity(now, not_after);
+  CERTCertificate* cert = CERT_CreateCertificate(serial_number, subject_name,
+                                                 validity, cert_request);
+  if (!cert) {
+    PRErrorCode prerr = PR_GetError();
+    LOG(ERROR) << "Failed to create certificate: " << prerr;
+  }
+
+  // Cleanup for resources used to generate the cert.
+  CERT_DestroyName(subject_name);
+  CERT_DestroyValidity(validity);
+  CERT_DestroyCertificateRequest(cert_request);
+
+  if (!cert)
+    return NULL;
+
+  /* Configure X509 library to handle the OB_CERT X509 Extension OID */

[wtc, 2011/08/04 00:37:53]

Use the C++ comment delimiter //.

The Style Guide recommends comments use proper punctuation:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Punctuation,_S...

So please make sure your comments have proper capitalization,
periods at the end, etc.

[mdietz, 2011/08/18 00:02:45]

Done.

+  static bool ob_cert_oid_tag_defined;
+  static SECOidTag ob_cert_oid_tag;
+
+  if (!ob_cert_oid_tag_defined) {

[wtc, 2011/08/04 00:37:53]

Chrome has a Singleton and LazyInstance class to do thread-safe
lazy one-time initialization.  Please use that.

[mdietz, 2011/08/18 00:02:45]

Implemented as a Sinlgeton wrapper around a SECOidTag.

+    // It's harmless if multiple threads enter this block concurrently.
+    static const uint8 kObCertOID[] =
+    // 1.3.6.1.4.1.11129.2.1.6
+    // (iso.org.dod.internet.private.enterprises.google.googleSecurity.
+    //  certificateExtensions.originBoundCertificate)
+      {0x2b, 0x06, 0x01, 0x04, 0x01, 0xd6, 0x79, 0x02, 0x01, 0x06};

[wtc, 2011/08/04 00:37:53]

I would format this as follows:
  static const uint8 kObCertOID[] = {
    0x2b, 0x06, 0x01, 0x04, 0x01, 0xd6, 0x79, 0x02, 0x01, 0x06
  };

[mdietz, 2011/08/18 00:02:45]

Done.

[mdietz, 2011/08/18 00:02:45]

Done.

+    SECOidData oid_data;
+    memset(&oid_data, 0, sizeof(oid_data));
+    oid_data.oid.data = const_cast<uint8*>(kObCertOID);
+    oid_data.oid.len = sizeof(kObCertOID);

[wtc, 2011/08/04 00:37:53]

Add
  oid_data.offset = SEC_OID_UNKNOWN;

This is a no-op because SEC_OID_UNKNOWN is 0, but most of the
code Ive seen does this.

[mdietz, 2011/08/18 00:02:45]

Done.

+    oid_data.desc = "Origin Bound Certificate";

[wtc, 2011/08/04 00:37:53]

Add
  oid_data.mechanism = CKM_INVALID_MECHANISM;

[mdietz, 2011/08/18 00:02:45]

Done.

+    oid_data.supportedExtension = SUPPORTED_CERT_EXTENSION;
+    ob_cert_oid_tag = SECOID_AddEntry(&oid_data);
+    DCHECK_NE(SEC_OID_UNKNOWN, ob_cert_oid_tag);

[wtc, 2011/08/04 00:37:53]

Remove this DCHECK_NE because it has proper error handling and
logging code.

This is the recommendation of the Chromium Coding Style page:
http://www.chromium.org/developers/coding-style#TOC-CHECK-vs-DCHECK-and-NOTRE...

[mdietz, 2011/08/18 00:02:45]

Done.

+    if(ob_cert_oid_tag == SEC_OID_UNKNOWN) {
+      LOG(ERROR) << "OB_CERT OID tag creation failed";
+      return NULL;
+    }
+    ob_cert_oid_tag_defined = true;
+  }
+
+  SECStatus ok;
+
+  // Create opaque handle to add extensions
+  void* cert_handle;
+  if((cert_handle = CERT_StartCertExtensions(cert)) == NULL){

[wtc, 2011/08/04 00:37:53]

Add a space after "if".

Add a space before the opening curly brace '{'.

[mdietz, 2011/08/18 00:02:45]

Done.

+    LOG(ERROR) << "Unable to get opaque handle for adding extensions";
+    return NULL;
+  }
+
+  // Create stack allocated SECItem for IA5String encoding
+  SECItem origin_string_item = {siAsciiString,
+                                (unsigned char*)origin.c_str(),
+                                origin.size()+1};

[wtc, 2011/08/04 00:37:53]

The terminating null should not be included.

I would format this as follows:
  SECItem origin_string_item = {
    siAsciiString,
    (unsigned char*)origin.data(),
    origin.size()
  };

Note the use of data() instead of c_str() (which avoids
the conversion to a C string in theory) and the absence
of +1.

Make the same change to x509_certificate_unittest.cc.

[mdietz, 2011/08/18 00:02:45]

Done.

Note to self: Apply same to unittest.

+
+  // IA5Encode and arena allocate SECItem
+  SECItem *asn1_origin_string;

[wtc, 2011/08/04 00:37:53]

Put * next to the type.

+  if((asn1_origin_string = SEC_ASN1EncodeItem(
+                                         cert->arena,
+                                         NULL,
+                                         &origin_string_item,
+                                         SEC_ASN1_GET(SEC_IA5StringTemplate)))

[wtc, 2011/08/04 00:37:53]

Indent the wrapped parameters by four spaces.

[mdietz, 2011/08/18 00:02:45]

Done.

This might exceed 80 lines on the last line.  Suggestions?

+     == NULL){
+    LOG(ERROR) << "Unable to get ASN1 encoding for origin in ob_cert extension";
+    return NULL;
+  }
+
+  // Add the extension to the opaque handle
+  if((ok = CERT_AddExtension(cert_handle, ob_cert_oid_tag, asn1_origin_string,
+                             PR_TRUE, PR_TRUE))
+      != SECSuccess){
+    LOG(ERROR) << "Unable to add origin bound cert extension to opaque handle";
+    return NULL;
+  }
+
+  // Copy extension into x509 cert
+  if((ok = CERT_FinishExtensions(cert_handle)) != SECSuccess){
+    LOG(ERROR) << "Unable to copy extension to X509 cert";
+    return NULL;
+  }
+
+  // Sign the cert here. The logic of this method references SignCert() in NSS
+  // utility certutil: http://mxr.mozilla.org/security/ident?i=SignCert.
+
+  // |arena| is used to encode the cert.
+  PRArenaPool* arena = cert->arena;
+  SECOidTag algo_id = SEC_GetSignatureAlgorithmOidTag(key->key()->keyType,
+                                                      SEC_OID_SHA1);
+  if (algo_id == SEC_OID_UNKNOWN) {
+    CERT_DestroyCertificate(cert);
+    return NULL;
+  }
+
+  SECStatus rv = SECOID_SetAlgorithmID(arena, &cert->signature, algo_id, 0);
+  if (rv != SECSuccess) {
+    CERT_DestroyCertificate(cert);
+    return NULL;
+  }
+
+  // Generate a cert of version 3.
+  *(cert->version.data) = 2;
+  cert->version.len = 1;
+
+  SECItem der;
+  der.len = 0;
+  der.data = NULL;
+
+  // Use ASN1 DER to encode the cert.
+  void* encode_result = SEC_ASN1EncodeItem(
+      arena, &der, cert, SEC_ASN1_GET(CERT_CertificateTemplate));
+  if (!encode_result) {
+    CERT_DestroyCertificate(cert);
+    return NULL;
+  }
+
+  // Allocate space to contain the signed cert.
+  SECItem* result = SECITEM_AllocItem(arena, NULL, 0);
+  if (!result) {
+    CERT_DestroyCertificate(cert);
+    return NULL;
+  }
+
+  // Sign the ASN1 encoded cert and save it to |result|.
+  rv = SEC_DerSignData(arena, result, der.data, der.len, key->key(), algo_id);
+  if (rv != SECSuccess) {
+    CERT_DestroyCertificate(cert);
+    return NULL;
+  }
+
+  // Save the signed result to the cert.
+  cert->derCert = *result;
+
+  X509Certificate* x509_cert =
+      CreateFromHandle(cert, SOURCE_LONE_CERT_IMPORT, OSCertHandles());
+  CERT_DestroyCertificate(cert);
+  return x509_cert;
+}
+
 void X509Certificate::GetDNSNames(std::vector<std::string>* dns_names) const {
   dns_names->clear();
 
   // Compare with CERT_VerifyCertName().
   GetCertSubjectAltNamesOfType(cert_handle_, certDNSName, dns_names);
 
   if (dns_names->empty())
     dns_names->push_back(subject_.common_name);
 }
 
@@ skipping 235 matching lines @@
 
 // static
 bool X509Certificate::WriteOSCertHandleToPickle(OSCertHandle cert_handle,
                                                 Pickle* pickle) {
   return pickle->WriteData(
       reinterpret_cast<const char*>(cert_handle->derCert.data),
       cert_handle->derCert.len);
 }
 
 }  // namespace net