Fix imported server certs being distrusted in NSS 3.13.

net/base/cert_database.h

 // Copyright (c) 2012 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.
 
 #ifndef NET_BASE_CERT_DATABASE_H_
 #define NET_BASE_CERT_DATABASE_H_
 #pragma once
 
 #include <string>
 #include <vector>
@@ skipping 62 matching lines @@
 
   // Constants that define which usages a certificate is trusted for.
   // They are used in combination with CertType to specify trust for each type
   // of certificate.
   // For a CA_CERT, they specify that the CA is trusted for issuing server and
   // client certs of each type.
   // For SERVER_CERT, only TRUSTED_SSL makes sense, and specifies the cert is
   // trusted as a server.
   // For EMAIL_CERT, only TRUSTED_EMAIL makes sense, and specifies the cert is
   // trusted for email.
+  // For non-root certs, TRUST_TERMINAL_RECORD specifies that the cert should
+  // not inherit trust from the issuer cert chain, and the cert will be trusted
+  // or not based only on which TRUSTED_* flags are set.
   // NOTE: The actual constants are defined using an enum instead of static
   // consts due to compilation/linkage constraints with template functions.
   typedef uint32 TrustBits;
   enum {
-    UNTRUSTED        =      0,
-    TRUSTED_SSL      = 1 << 0,
-    TRUSTED_EMAIL    = 1 << 1,
-    TRUSTED_OBJ_SIGN = 1 << 2,
+    UNTRUSTED             =      0,
+    TRUSTED_SSL           = 1 << 0,
+    TRUSTED_EMAIL         = 1 << 1,
+    TRUSTED_OBJ_SIGN      = 1 << 2,
+    TRUST_TERMINAL_RECORD = 1 << 3,

[Ryan Sleevi, 2012/03/29 23:35:13]

This is unfortunately very specific to NSS, which I think makes it hard to see
this as a generic interface.

While I realize this is an existing issue, it might be worth considering moving
this into an #ifdef for NSS.

[wtc, 2012/03/30 22:00:50]

We need to think this through.  The low-level NSS trust flags
are hard to understand.  Ideally we should create a more
intuitive set of trust flags here and map them to the NSS
trust flags internally.

   };
 
   CertDatabase();
 
   // Check whether this is a valid user cert that we have the private key for.
   // Returns OK or a network error code such as ERR_CERT_CONTAINS_ERRORS.
   int CheckUserCert(X509Certificate* cert);
 
   // Store user (client) certificate. Assumes CheckUserCert has already passed.
   // Returns OK, or ERR_ADD_USER_CERT_FAILED if there was a problem saving to
@@ skipping 54 matching lines @@
 
   // Import server certificate.  The first cert should be the server cert.  Any
   // additional certs should be intermediate/CA certs and will be imported but
   // not given any trust.
   // Any certificates that could not be imported will be listed in
   // |not_imported|.
   // Returns false if there is an internal error, otherwise true is returned and
   // |not_imported| should be checked for any certificates that were not
   // imported.
   bool ImportServerCert(const CertificateList& certificates,
+                        TrustBits trust_bits,

[Ryan Sleevi, 2012/03/29 23:35:13]

OpenSSL and Android have no such concept.

Android: http://developer.android.com/reference/android/security/KeyChain.html

[wtc, 2012/03/30 22:00:50]

One solution might be to make ImportCACerts and
ImportServerCert do nothing but importing the certs, and
require a separate SetCertTrust call to set the trust.
This allows us to implement ImportCACerts and
ImportServerCert (with neutral trust) with OpenSSL and
Android.

[mattm, 2012/03/30 22:16:56]

Looking at the link Ryan posted, I'm not sure android should  use these
functions at all.  It looks like the only way to import a cert there is through
an Intent (does it involve user confirmation even if the app supplies the cert
to add?).  Synchronous import functions like these might not really be usable
for android?

[Ryan Sleevi, 2012/03/30 22:42:36]

Correct. And they'd like to fix that (and work is in progress somewhat).

I took a second look through cert_database_openssl, and it seems the entire
thing is NotImplemented. So really, we should remove the USE_OPENSSL from the
#if defined() on line 108 and accept that this is a huge NSS-specialization for
now. I'm fine with that. I think the only reason it was added was because of all
the NSS-specifics in the CertificateManagerModel that wanted to be re-used for
OpenSSL.

The only thing about this that is generic (today) is CheckUserCert/AddUserCert,
and both of those need to be made non-blocking callbacks at some point that run
on worker threads.

(AddObserver/RemoveObserver are questionably so - we want to implement them
correctly on other platforms, but as noticed in issues like
http://crbug.com/97462, this requires some UI-thread hopping for OS X, and FILE
thread for Win, so this is not really the same across platforms)

                         ImportCertFailureList* not_imported);
 
   // Get trust bits for certificate.
   TrustBits GetCertTrust(const X509Certificate* cert, CertType type) const;
 
   // IsUntrusted returns true if |cert| is specifically untrusted. These
   // certificates are stored in the database for the specific purpose of
   // rejecting them.
   bool IsUntrusted(const X509Certificate* cert) const;
 
@@ skipping 26 matching lines @@
   static void NotifyObserversOfUserCertAdded(const X509Certificate* cert);
   static void NotifyObserversOfUserCertRemoved(const X509Certificate* cert);
   static void NotifyObserversOfCertTrustChanged(const X509Certificate* cert);
 
   DISALLOW_COPY_AND_ASSIGN(CertDatabase);
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_CERT_DATABASE_H_