Add an SSLConfigService implementation for Mac OS X

Add an SSLConfigService implementation for Mac OS X

BUG=19293
TEST=https://test-ssev.verisign.com/ has three links: one should work OK, the other should warn that the certificate is expired, the other that the certificate is revoked.

[hawk, 2009-09-03 21:10:34]

Hopefully a short code review, you'll find that the Mac code looks suspiciously
like the Windows code.

One issue for discussion: we honor the system wide setting for revocation
checking, which can be "off", "try", or "require". Our model, on the other hand,
is to set a bool flag requesting a check on input, and set status bits in the
result indicating whether revocation checking attempted and whether the
certificate was revoked on output. The issue is how to treat the "try" setting.
This CL treats it the same as "require", that is, it sets the request. Most
sites don't have revocation checkable certificates (EV certificate being a
notable exception), so if a user sets revocation checking to "try" we'll give
him a warning when he browses most https sites. This behavior is different from
Safari, which in this case will silently ignore the inability to check
revocation.

One more note of interest: the only options in the Mac OS X UI (Keychain
Access->Preferences->Certificates) are "off" and "try" (at least on several
10.5.7 and 10.5.8 systems I checked). To set to "require" you've got to drop to
the command line or edit the plist manually.

[wtc, 2009-09-03 22:22:50]

LGTM.

I don't know the definitions of "off", "try", and "require",
but if I guess Chromium's "revocation checking disabled" is
equivalent to "off" and "revocation checking enabled"
is equivalent to "try", and we don't support "require" as
it's too strict.

Two general comments.

1. Please add BUG=19293 and TEST= to this CL's description.

2. If it's not too much work, please use "svn cp" to
add the three new files, so that we can see how they differ
from ssl_config_service_win.* easily.  This would require
- save copies of the three new files
- "svn revert" on the three new files
- "svn cp" to re-add the new files based on the windows
  files

http://codereview.chromium.org/193009/diff/1/6
File net/base/ssl_config_service_mac.cc (right):

http://codereview.chromium.org/193009/diff/1/6#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
Nit: 2006-2008 => 2009

http://codereview.chromium.org/193009/diff/1/6#newcode28
Line 28: static CFStringRef kNoneRevocationValue = CFSTR("None");
Is "None" equivalent to "off" and "BestAttempt" equivalent
to "try" in the Keychain Access application?  Would be nice
to document that.

http://codereview.chromium.org/193009/diff/1/6#newcode30
Line 30: static CFStringRef kSSL2EnabledKey = CFSTR("org.chromium.ssl2");
Should we put these keys under "ssl"?  That is,
  org.chromium.ssl.ssl2
  org.chromium.ssl.ssl3
  org.chromium.ssl.tls1

http://codereview.chromium.org/193009/diff/1/6#newcode42
Line 42: if (CFStringCompare(kNoneRevocationValue, style,
I guess this is where we consider "try" and "require"
as the same?  This subtle point deserves a comment.

http://codereview.chromium.org/193009/diff/1/6#newcode52
Line 52: if (CFPreferencesGetAppBooleanValue(key,
kCFPreferencesCurrentApplication,
The CFPreferencesGetAppBooleanValue documentation seems to
suggest that we should check exists_and_valid before checking
the return value of CFPreferencesGetAppBooleanValue, i.e.,
  Boolean exists_and_valid;
  Boolean rv = CFPreferencesGetAppBooleanValue(..., &exists_and_valid);
  if (!exists_and_valid) {
    DCHECK(!rv);
    return default_value;
  }
  return rv;

http://codereview.chromium.org/193009/diff/1/6#newcode128
Line 128: // This method is provided for use by the unit tests. These settings
Is this why you're not calling CFPreferencesAppSynchronize?

Our Options menu has a checkbox for "check for server
certificate revocation".  Does this mean we're not going
to implement that checkbox on the Mac?

http://codereview.chromium.org/193009/diff/1/6#newcode132
Line 132: enabled ? kBestAttemptRevocationValue : kNoneRevocationValue,
Yes, our "revocation checking enabled" is equivalent to
"BestAttempt".

http://codereview.chromium.org/193009/diff/1/6#newcode134
Line 134: kCFPreferencesAnyHost);
Just curious: what does "host" mean here?

http://codereview.chromium.org/193009/diff/1/4
File net/base/ssl_config_service_mac.h (right):

http://codereview.chromium.org/193009/diff/1/4#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
Nit: 2006-2009 => 2009

http://codereview.chromium.org/193009/diff/1/4#newcode16
Line 16: // We think the SSL configuration settings should apply to all
applications
Delete this entire paragraph. Maybe keep the first sentence.

http://codereview.chromium.org/193009/diff/1/4#newcode50
Line 50: // We store the IE SSL config and the time that we fetched it.
IE => system

http://codereview.chromium.org/193009/diff/1/5
File net/base/ssl_config_service_mac_unittest.cc (right):

http://codereview.chromium.org/193009/diff/1/5#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
Nit: 2006-2008 => 2009

[hawk, 2009-09-08 20:06:06]

http://codereview.chromium.org/193009/diff/1/6
File net/base/ssl_config_service_mac.cc (right):

http://codereview.chromium.org/193009/diff/1/6#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
On 2009/09/03 22:22:50, wtc wrote:
> Nit: 2006-2008 => 2009

Done.

http://codereview.chromium.org/193009/diff/1/6#newcode28
Line 28: static CFStringRef kNoneRevocationValue = CFSTR("None");
On 2009/09/03 22:22:50, wtc wrote:
> Is "None" equivalent to "off" and "BestAttempt" equivalent
> to "try" in the Keychain Access application?  Would be nice
> to document that.

Everywhere in comments where I've included something in quotes is a paraphrase
-- the actual values that may appear in the plist are:

None
BestAttempt
RequireIfPresent
RequireForAll

http://codereview.chromium.org/193009/diff/1/6#newcode30
Line 30: static CFStringRef kSSL2EnabledKey = CFSTR("org.chromium.ssl2");
On 2009/09/03 22:22:50, wtc wrote:
> Should we put these keys under "ssl"?  That is,
>   org.chromium.ssl.ssl2
>   org.chromium.ssl.ssl3
>   org.chromium.ssl.tls1
>   

Done.

http://codereview.chromium.org/193009/diff/1/6#newcode42
Line 42: if (CFStringCompare(kNoneRevocationValue, style,
On 2009/09/03 22:22:50, wtc wrote:
> I guess this is where we consider "try" and "require"
> as the same?  This subtle point deserves a comment.

We treat anything other than None as the same, which is commented below.

http://codereview.chromium.org/193009/diff/1/6#newcode52
Line 52: if (CFPreferencesGetAppBooleanValue(key,
kCFPreferencesCurrentApplication,
On 2009/09/03 22:22:50, wtc wrote:
> The CFPreferencesGetAppBooleanValue documentation seems to
> suggest that we should check exists_and_valid before checking
> the return value of CFPreferencesGetAppBooleanValue, i.e.,
>   Boolean exists_and_valid;
>   Boolean rv = CFPreferencesGetAppBooleanValue(..., &exists_and_valid);
>   if (!exists_and_valid) {
>     DCHECK(!rv);
>     return default_value;
>   }
>   return rv;

Done.

http://codereview.chromium.org/193009/diff/1/6#newcode128
Line 128: // This method is provided for use by the unit tests. These settings
On 2009/09/03 22:22:50, wtc wrote:
> Is this why you're not calling CFPreferencesAppSynchronize?

Yes. We don't want to override the system settings, and bool is less expressive
than what the system settings expect.

> Our Options menu has a checkbox for "check for server
> certificate revocation".  Does this mean we're not going
> to implement that checkbox on the Mac?

Yes. SecTrustEvaluate(), which we use for certificate validation, uses the
system wide settings for revocation checking.

http://codereview.chromium.org/193009/diff/1/6#newcode132
Line 132: enabled ? kBestAttemptRevocationValue : kNoneRevocationValue,
On 2009/09/03 22:22:50, wtc wrote:
> Yes, our "revocation checking enabled" is equivalent to
> "BestAttempt".

There's a subtle difference: we make a distinction between "certificate
revocation check succeeded and the certificate is not revoked" and "couldn't
perform revocation checking (no AIA extension, or no CRL, or OCSP responder is
down, etc.)". Under BestAttempt, Mac OS X provides no such distinction; the
result is simply noErr.

http://codereview.chromium.org/193009/diff/1/6#newcode134
Line 134: kCFPreferencesAnyHost);
On 2009/09/03 22:22:50, wtc wrote:
> Just curious: what does "host" mean here?

In the case where a user has a mobile home directory or some other method of
preference synchronization (e.g., MobileMe) across multiple machines, you can
specify whether you want the user's setting for a specific machine or the
generic setting.

http://codereview.chromium.org/193009/diff/1/4
File net/base/ssl_config_service_mac.h (right):

http://codereview.chromium.org/193009/diff/1/4#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
On 2009/09/03 22:22:50, wtc wrote:
> Nit: 2006-2009 => 2009

Done.

http://codereview.chromium.org/193009/diff/1/4#newcode16
Line 16: // We think the SSL configuration settings should apply to all
applications
On 2009/09/03 22:22:50, wtc wrote:
> Delete this entire paragraph. Maybe keep the first sentence.

Done.

http://codereview.chromium.org/193009/diff/1/4#newcode50
Line 50: // We store the IE SSL config and the time that we fetched it.
On 2009/09/03 22:22:50, wtc wrote:
> IE => system

Done.

http://codereview.chromium.org/193009/diff/1/5
File net/base/ssl_config_service_mac_unittest.cc (right):

http://codereview.chromium.org/193009/diff/1/5#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
On 2009/09/03 22:22:50, wtc wrote:
> Nit: 2006-2008 => 2009

Done.

[wtc, 2009-09-09 20:32:49]

LGTM.