net: comment/TODO cleanup and clarification, and don't raise ERR_SSL_PROTOCOL_ERROR for user_cancelled TLS alerts on Mac.

TBR=wtc
BUG=none
TEST=none

Review URL: http://codereview.chromium.org/3938002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63199 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 25 insertions, 18 deletions

diff --git a/net/socket/ssl_client_socket_mac.cc b/net/socket/ssl_client_socket_mac.cc
index 918b6b8..6a8270e 100644
--- a/net/socket/ssl_client_socket_mac.cc
+++ b/net/socket/ssl_client_socket_mac.cc
@@ -164,7 +164,6 @@ int NetErrorFromOSStatus(OSStatus status) {
     case errSSLPeerHandshakeFail:  // Received a handshake_failure alert.
     case errSSLPeerNoRenegotiation:  // Received a no_renegotiation alert
     case errSSLPeerUnexpectedMsg:  // Received an unexpected_message alert.
-    case errSSLPeerUserCancelled:  // Received a user_cancelled alert.
     case errSSLProtocol:
     case errSSLRecordOverflow:
       return ERR_SSL_PROTOCOL_ERROR;
@@ -186,12 +185,17 @@ int NetErrorFromOSStatus(OSStatus status) {
 
     // (Note that all errSSLPeer* codes indicate errors reported by the peer,
     // so the cert-related ones refer to my _client_ cert.)
+    // TODO(wtc): Add fine-grained error codes for client certificate errors
+    // reported by the server using the following SSL/TLS alert messages:
+    //   access_denied
+    //   bad_certificate
+    //   unsupported_certificate
+    //   certificate_expired
+    //   certificate_revoked
+    //   certificate_unknown
+    //   unknown_ca
     case errSSLPeerCertUnknown...errSSLPeerBadCert:
     case errSSLPeerUnknownCA:
-    // TODO(rsleevi): Add a new error code for access_denied - the peer has
-    // accepted the certificate as valid, but denied access to the requested
-    // resource. Returning ERR_BAD_SSL_CLIENT_AUTH simply gives the user a
-    // chance to select a new certificate, if they have one, and try again.
     case errSSLPeerAccessDenied:
       LOG(WARNING) << "Server rejected client cert (OSStatus=" << status << ")";
       return ERR_BAD_SSL_CLIENT_AUTH_CERT;
diff --git a/net/socket/ssl_client_socket_win.cc b/net/socket/ssl_client_socket_win.cc
index 6fccad1..7e8c29e 100644
--- a/net/socket/ssl_client_socket_win.cc
+++ b/net/socket/ssl_client_socket_win.cc
@@ -35,20 +35,23 @@ static int MapSecurityError(SECURITY_STATUS err) {
   // There are numerous security error codes, but these are the ones we thus
   // far find interesting.
   switch (err) {
-    case SEC_E_WRONG_PRINCIPAL:  // Schannel
+    case SEC_E_WRONG_PRINCIPAL:  // Schannel - server certificate error.
     case CERT_E_CN_NO_MATCH:  // CryptoAPI
       return ERR_CERT_COMMON_NAME_INVALID;
-    case SEC_E_UNTRUSTED_ROOT:  // Schannel - unknown_ca alert
+    case SEC_E_UNTRUSTED_ROOT:  // Schannel - server certificate error or
+                                // unknown_ca alert.
     case CERT_E_UNTRUSTEDROOT:  // CryptoAPI
       return ERR_CERT_AUTHORITY_INVALID;
-    case SEC_E_CERT_EXPIRED:  // Schannel - certificate_expired alert
+    case SEC_E_CERT_EXPIRED:  // Schannel - server certificate error or
+                              // certificate_expired alert.
     case CERT_E_EXPIRED:  // CryptoAPI
       return ERR_CERT_DATE_INVALID;
     case CRYPT_E_NO_REVOCATION_CHECK:
       return ERR_CERT_NO_REVOCATION_MECHANISM;
     case CRYPT_E_REVOCATION_OFFLINE:
       return ERR_CERT_UNABLE_TO_CHECK_REVOCATION;
-    case CRYPT_E_REVOKED:  // CryptoAPI and Schannel certificate_revoked alert
+    case CRYPT_E_REVOKED:  // CryptoAPI and Schannel server certificate error,
+                           // or certificate_revoked alert.
       return ERR_CERT_REVOKED;
 
     // We received one of the following alert messages from the server:
@@ -82,10 +85,6 @@ static int MapSecurityError(SECURITY_STATUS err) {
     // been omitted to prevent masking them as protocol errors.
       return ERR_SSL_PROTOCOL_ERROR;
 
-    // TODO(rsleevi): Add a new error code for access_denied - the peer has
-    // accepted the certificate as valid, but denied access to the requested
-    // resource. Returning ERR_BAD_SSL_CLIENT_AUTH simply gives the user a
-    // chance to select a new certificate, if they have one, and try again.
     case SEC_E_LOGON_DENIED:  // Received a access_denied alert.
       return ERR_BAD_SSL_CLIENT_AUTH_CERT;
 
@@ -1014,11 +1013,15 @@ int SSLClientSocketWin::DidCallInitializeSecurityContext() {
     // InitializeSecurityContext must be referring to the bad or missing
     // client certificate.
     if (IsCertificateError(result)) {
-      // TODO(wtc): Add new error codes for client certificate errors reported
-      // by the server using SSL/TLS alert messages.  See the MSDN page
-      // "Schannel Error Codes for TLS and SSL Alerts", which maps TLS alert
-      // messages to Windows error codes:
-      // http://msdn.microsoft.com/en-us/library/dd721886%28VS.85%29.aspx
+      // TODO(wtc): Add fine-grained error codes for client certificate errors
+      // reported by the server using the following SSL/TLS alert messages:
+      //   access_denied
+      //   bad_certificate
+      //   unsupported_certificate
+      //   certificate_expired
+      //   certificate_revoked
+      //   certificate_unknown
+      //   unknown_ca
       return ERR_BAD_SSL_CLIENT_AUTH_CERT;
     }
     return result;