-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
okhttp: support Conscrypt security provider #3971
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,6 +54,7 @@ public class OkHttpProtocolNegotiatorTest { | |
@Rule public final ExpectedException thrown = ExpectedException.none(); | ||
|
||
private final Provider fakeSecurityProvider = new Provider("GmsCore_OpenSSL", 1.0, "info") {}; | ||
private final Provider fakeConscrypt = new Provider("Conscrypt", 1.0, "info") {}; | ||
private final SSLSocket sock = mock(SSLSocket.class); | ||
private final Platform platform = mock(Platform.class); | ||
|
||
|
@@ -229,6 +230,31 @@ protected Class<?> findClass(String name) throws ClassNotFoundException { | |
assertEquals(TlsExtensionType.NPN, tlsExtensionType); | ||
} | ||
|
||
@Test | ||
public void pickTlsExtensionType_android41WithConscrypt() throws Exception { | ||
Security.removeProvider(fakeSecurityProvider.getName()); | ||
Security.addProvider(fakeConscrypt); | ||
ClassLoader cl = | ||
new ClassLoader(this.getClass().getClassLoader()) { | ||
@Override | ||
protected Class<?> findClass(String name) throws ClassNotFoundException { | ||
// Just don't throw. | ||
if ("android.app.ActivityOptions".equals(name)) { | ||
return null; | ||
} | ||
return super.findClass(name); | ||
} | ||
}; | ||
|
||
AndroidNegotiator.TlsExtensionType tlsExtensionType = | ||
AndroidNegotiator.pickTlsExtensionType(cl); | ||
|
||
assertEquals(TlsExtensionType.ALPN_AND_NPN, tlsExtensionType); | ||
|
||
// Clean up | ||
Security.removeProvider(fakeConscrypt.getName()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. try-finally or similar (could maybe remove it unconditionally in the |
||
} | ||
|
||
@Test | ||
public void pickTlsExtensionType_none() throws Exception { | ||
Security.removeProvider(fakeSecurityProvider.getName()); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,6 @@ | |
import java.util.List; | ||
import java.util.logging.Level; | ||
import java.util.logging.Logger; | ||
|
||
import javax.net.ssl.SSLContext; | ||
import javax.net.ssl.SSLSocket; | ||
import okio.Buffer; | ||
|
@@ -63,14 +62,26 @@ public class Platform { | |
public static final Logger logger = Logger.getLogger(Platform.class.getName()); | ||
|
||
/** | ||
* List of security providers to use in order of preference. | ||
* List of preferred security provider names, in order of preference. These will be used, if | ||
* present, followed by {@code SECONDARY_ANDROID_SECURITY_PROVIDER_CLASSES} if these are | ||
* unavailable. | ||
*/ | ||
private static final String[] ANDROID_SECURITY_PROVIDERS = | ||
new String[] { | ||
// See https://developer.android.com/training/articles/security-gms-provider.html | ||
"GmsCore_OpenSSL", | ||
"Conscrypt" | ||
}; | ||
/** | ||
* List of secondary security provider class names to use in order of preference, if the | ||
* preferred providers are unavailable. | ||
*/ | ||
private static final String[] ANDROID_SECURITY_PROVIDERS = new String[]{ | ||
// See https://developer.android.com/training/articles/security-gms-provider.html | ||
"com.google.android.gms.org.conscrypt.OpenSSLProvider", | ||
"com.android.org.conscrypt.OpenSSLProvider", | ||
"org.conscrypt.OpenSSLProvider", | ||
"org.apache.harmony.xnet.provider.jsse.OpenSSLProvider"}; | ||
private static final String[] SECONDARY_ANDROID_SECURITY_PROVIDER_CLASSES = | ||
new String[] { | ||
"com.android.org.conscrypt.OpenSSLProvider", | ||
"org.conscrypt.OpenSSLProvider", | ||
"org.apache.harmony.xnet.provider.jsse.OpenSSLProvider" | ||
}; | ||
|
||
private static final Platform PLATFORM = findPlatform(); | ||
|
||
|
@@ -199,11 +210,18 @@ private static Provider getAppEngineProvider() { | |
} | ||
|
||
/** | ||
* Select from the available security providers in preference order. If a preferred provider | ||
* is not found then warn but continue. | ||
* Select from the available security providers in preference order. If a preferred provider is | ||
* not found then warn but continue. | ||
*/ | ||
private static Provider getAndroidSecurityProvider() { | ||
for (String providerClassName : ANDROID_SECURITY_PROVIDERS) { | ||
for (String providerName : ANDROID_SECURITY_PROVIDERS) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should loop through the providers in order and choose the first we can use. That way the user has some way of configuring our behavior, and our behavior will better match expectations. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
Provider provider = Security.getProvider(providerName); | ||
if (provider != null) { | ||
logger.log(Level.FINE, "Found registered provider {0}", provider); | ||
return provider; | ||
} | ||
} | ||
for (String providerClassName : SECONDARY_ANDROID_SECURITY_PROVIDER_CLASSES) { | ||
Provider[] providers = Security.getProviders(); | ||
for (Provider availableProvider : providers) { | ||
if (providerClassName.equals(availableProvider.getClass().getName())) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should prefer insertProviderAt so that Conscrypt can be prioritized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done