New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 630151 link

Starred by 5 users
Status: Fixed
Owner:
davidben@chromium.org
Closed: Nov 2017
Cc:
█ svaldez@chromium.org
nhar...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 630147



Sign in to add a comment

Update SSLPrivateKey for RSA-PSS

Project Member Reported by davidben@chromium.org, Jul 21 2016 
TLS 1.3 changes the signing scheme from a split pre-hash / sign-digest to a uniform message-based signing. RSA-PSS is the first algorithm under this new model (though Ed25519 and general cleanliness was the true motivation).

We need to:

1. Switch us to using the new hooks in SSL_PRIVATE_KEY_METHOD by simply taking a hash and calling the old SSLPrivateKey API.

2. Push it down the stack and switch SSLPrivateKey to taking a 1.3-style sigalg value and reporting preferences accordingly.

3. Route up RSA-PSS on the OSs where we can. Where we can't, you don't get to do client auth with TLS 1.3.

4. Revise the CrOS certProvider API so it too works in this new world and route it up.

Comment 1 by davidben@chromium.org, Nov 2 2017

Owner: davidben@chromium.org
Status: Started (was: Untriaged)
Summary: Update SSLPrivateKey for RSA-PSS (was: Update SSLPrivateKey for message-based signing and RSA-PSS)
Punting message-based signing for now. We can do that part when Ed25519 comes along.

Comment 2 by davidben@chromium.org, Nov 2 2017 

Hrm. Actually, given the way the Android APIs work, we might not be able to punt message-based signing. We'll see...
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d

commit b9bafbe16eafc2b12a1618ffa5e38a8476437a2d
Author: David Benjamin <davidben@chromium.org>
Date: Tue Nov 07 21:41:38 2017

Replace SSLPrivateKey::Hash with a TLS-1.3-compatible scheme.

This finally switches from the old hash parameter bits to
SSL_SIGN_*. I haven't added RSA-PSS yet in this change, that will happen
in a follow-up. It also keeps the prehash bits for now, though that
too may need to change, going by how Android's RSA-PSS API looks
like.

Bug:  630151 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I93e06673611b41ae9970017aff235b94b0528f00
Reviewed-on: https://chromium-review.googlesource.com/752345
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514606}
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/chrome/browser/chromeos/certificate_provider/certificate_info.h
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/chrome/browser/chromeos/certificate_provider/certificate_provider_service.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/chrome/browser/chromeos/certificate_provider/certificate_provider_service.h
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/chrome/browser/chromeos/certificate_provider/certificate_provider_service_unittest.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/chrome/browser/extensions/api/certificate_provider/certificate_provider_api.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa_browsertest.mm
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/BUILD.gn
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/socket/ssl_client_socket_impl.h
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/client_cert_store_nss_unittest.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_client_auth_cache_unittest.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_android.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_android_unittest.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_mac.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_mac_unittest.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_nss.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_nss_unittest.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_win.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_platform_key_win_unittest.cc
[add] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_private_key.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_private_key.h
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/ssl_private_key_test_util.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/test_ssl_private_key.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/threaded_ssl_private_key.cc
[modify] https://crrev.com/b9bafbe16eafc2b12a1618ffa5e38a8476437a2d/net/ssl/threaded_ssl_private_key.h
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 7 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7b5f39db384e7f3fc53bc98a7577ee7b9684630

commit f7b5f39db384e7f3fc53bc98a7577ee7b9684630
Author: David Benjamin <davidben@chromium.org>
Date: Tue Nov 07 22:32:15 2017

Add RSA-PSS support to Linux, MacOS, and Windows SSLPrivateKey.

Android support will be done separately. It requires us to switch
from SignDigest to Sign.

Bug:  630151 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Idaeb84e9c10e93efce634170e4cea528d6cbe109
Reviewed-on: https://chromium-review.googlesource.com/753827
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514627}
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/chrome/browser/ui/cocoa/ssl_client_certificate_selector_cocoa_browsertest.mm
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/client_cert_store_nss_unittest.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_android.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_android_unittest.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_mac.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_mac_unittest.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_nss.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_nss_unittest.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_win.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_platform_key_win_unittest.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_private_key.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_private_key.h
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/ssl_private_key_test_util.cc
[modify] https://crrev.com/f7b5f39db384e7f3fc53bc98a7577ee7b9684630/net/ssl/test_ssl_private_key.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 10 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3

commit 9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3
Author: David Benjamin <davidben@chromium.org>
Date: Fri Nov 10 19:01:53 2017

Replace SSLPrivateKey::SignDigest with Sign.

This is so we can support Ed25519 in the future (in my dreams...) and so
we can call into RSA-PSS on Android. The Java APIs for RSA-PSS only take
take the unhashed input, so go ahead and make this change now. Existing
SSLPrivateKey implementations can use SSL_get_signature_algorithm_digest
and EVP_Digest to have BoringSSL perform the hash for them.

While I'm here, change StringPiece to span<const uint8_t> and avoid a
bunch of casts.

Bug:  630151 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ib83e182608c036ec273309d79fa63541ff0cad6f
Reviewed-on: https://chromium-review.googlesource.com/754583
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515621}
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/chrome/browser/chromeos/certificate_provider/certificate_provider_service.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/chrome/browser/chromeos/certificate_provider/certificate_provider_service.h
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/chrome/browser/chromeos/certificate_provider/certificate_provider_service_unittest.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/chrome/browser/extensions/api/certificate_provider/certificate_provider_apitest.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/android/keystore.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/android/keystore.h
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/ssl_client_auth_cache_unittest.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/ssl_platform_key_android.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/ssl_platform_key_mac.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/ssl_platform_key_nss.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/ssl_platform_key_win.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/ssl_private_key.h
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/ssl_private_key_test_util.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/test_ssl_private_key.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/threaded_ssl_private_key.cc
[modify] https://crrev.com/9ba36b0da5c50ee90afb6ad75f8e8f7de4f9fce3/net/ssl/threaded_ssl_private_key.h
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 10 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff

commit 5b4410e59b62eea4449fdb3e8b00f94af02ae9ff
Author: David Benjamin <davidben@chromium.org>
Date: Fri Nov 10 21:50:23 2017

Implement RSA-PSS SSLPrivateKey for Android >= N.

Starting Android N, RSA-PSS may be used. Prior to that, it doesn't work.
If we ever need to, we could hack around this with RSA/ECB/NoPadding and
performing the padding ourselves. For now, this CL leaves Android
platform keys on M and below without RSA-PSS. As with PSS requiring
macOS 10.13 or higher on Mac or PSS support in smartcards, client
certificate deployments must ensure the client base supports the
relevant algorithms when deploying TLS 1.3.

So this doesn't give a strange ERR_SSL_VERSION_INTERFERENCE error, map
SSL_R_NO_COMMON_SIGNATURE_ALGORITHMS to a corresponding //net error code
rather than ERR_SSL_PROTOCOL_ERROR.

Bug:  630151 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I5a434dde2f4721369d0d0ebff9dc85ad60930408
Reviewed-on: https://chromium-review.googlesource.com/755922
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515697}
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/android/java/src/org/chromium/net/AndroidKeyStore.java
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/android/keystore.cc
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/android/keystore.h
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/base/net_error_list.h
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/base/net_errors.cc
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/socket/ssl_client_socket_impl.cc
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/ssl/ssl_platform_key_android.cc
[modify] https://crrev.com/5b4410e59b62eea4449fdb3e8b00f94af02ae9ff/net/ssl/ssl_platform_key_android_unittest.cc

Comment 8 by davidben@chromium.org, Nov 10 2017

Status: Fixed (was: Started)
►

Sign in to add a comment