-
Notifications
You must be signed in to change notification settings - Fork 533
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
Use custom validation callback for server certificates in HTTP handlers #6665
Changes from 56 commits
42c0dc5
cf11487
ad3faad
3637e2f
fb47fc2
478cc55
1579766
7976be8
6b466f2
e7f8303
2e96d0a
d19208b
2d102dc
f0dedda
73ac4b8
32b5ae3
728d0d8
077d51a
98f13da
d6f4099
bcf498a
82371c2
6d96d0a
25f81a4
f0265ed
c2a7454
d3acfd1
31daa4b
f23eaa0
2646442
b89c6b6
050e8f0
78778a0
4ed5bfc
46f9cd8
4c7b8a9
3afb880
c4e5816
cad31b7
ce85c3d
4d0575a
58105c0
040ab06
88a1d1f
e7aa149
3b9d675
6162112
ea45b2b
70e44bf
5d8644c
9940bc7
3760be0
3d4a5d7
8485733
6dbff42
9ef2ab7
0164fdd
271d708
ce29621
d05ba71
7c6da63
765c26e
fbb4b0a
b71dee3
ff7ddb1
a8392ab
d5b3ba6
d5fbd16
b11c904
491d1c1
266e308
89f7e3d
eb038d1
5da8631
4210916
2900b05
dc45876
90f4bf2
ac0a75c
cd03384
19fca32
0d61693
25f7961
a91b69c
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 |
---|---|---|
|
@@ -157,7 +157,7 @@ public CookieContainer CookieContainer | |
|
||
public bool CheckCertificateRevocationList { get; set; } = false; | ||
|
||
public Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool> ServerCertificateCustomValidationCallback { get; set; } | ||
public Func<HttpRequestMessage, X509Certificate2?, X509Chain?, SslPolicyErrors, bool>? ServerCertificateCustomValidationCallback { get; set; } | ||
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. If users never set this, would the changes here be a no-op? There is a lot of certificate loading here, and I'm wondering how that will impact performance. 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. This will be a no-op now. |
||
|
||
// See: https://developer.android.com/reference/javax/net/ssl/SSLSocket#protocols | ||
public SslProtocols SslProtocols { get; set; } = | ||
|
@@ -194,12 +194,12 @@ public int MaxAutomaticRedirections | |
/// </summary> | ||
/// <value>The pre authentication data.</value> | ||
public AuthenticationData? PreAuthenticationData { get; set; } | ||
|
||
/// <summary> | ||
/// If the website requires authentication, this property will contain data about each scheme supported | ||
/// by the server after the response. Note that unauthorized request will return a valid response - you | ||
/// need to check the status code and and (re)configure AndroidMessageHandler instance accordingly by providing | ||
/// both the credentials and the authentication scheme by setting the <see cref="PreAuthenticationData"/> | ||
/// both the credentials and the authentication scheme by setting the <see cref="PreAuthenticationData"/> | ||
/// property. If AndroidMessageHandler is not able to detect the kind of authentication scheme it will store an | ||
/// instance of <see cref="AuthenticationData"/> with its <see cref="AuthenticationData.Scheme"/> property | ||
/// set to <c>AuthenticationScheme.Unsupported</c> and the application will be responsible for providing an | ||
|
@@ -226,12 +226,12 @@ public bool RequestNeedsAuthorization { | |
/// <summary> | ||
/// <para> | ||
/// If the request is to the server protected with a self-signed (or otherwise untrusted) SSL certificate, the request will | ||
/// fail security chain verification unless the application provides either the CA certificate of the entity which issued the | ||
/// fail security chain verification unless the application provides either the CA certificate of the entity which issued the | ||
/// server's certificate or, alternatively, provides the server public key. Whichever the case, the certificate(s) must be stored | ||
/// in this property in order for AndroidMessageHandler to configure the request to accept the server certificate.</para> | ||
/// <para>AndroidMessageHandler uses a custom <see cref="KeyStore"/> and <see cref="TrustManagerFactory"/> to configure the connection. | ||
/// <para>AndroidMessageHandler uses a custom <see cref="KeyStore"/> and <see cref="TrustManagerFactory"/> to configure the connection. | ||
/// If, however, the application requires finer control over the SSL configuration (e.g. it implements its own TrustManager) then | ||
/// it should leave this property empty and instead derive a custom class from AndroidMessageHandler and override, as needed, the | ||
/// it should leave this property empty and instead derive a custom class from AndroidMessageHandler and override, as needed, the | ||
/// <see cref="ConfigureTrustManagerFactory"/>, <see cref="ConfigureKeyManagerFactory"/> and <see cref="ConfigureKeyStore"/> methods | ||
/// instead</para> | ||
/// </summary> | ||
|
@@ -328,7 +328,7 @@ string EncodeUrl (Uri url) | |
AssertSelf (); | ||
if (request == null) | ||
throw new ArgumentNullException (nameof (request)); | ||
|
||
if (!request.RequestUri.IsAbsoluteUri) | ||
throw new ArgumentException ("Must represent an absolute URI", "request"); | ||
|
||
|
@@ -625,7 +625,7 @@ internal Task WriteRequestContentToOutputInternal (HttpRequestMessage request, H | |
return ret; | ||
} | ||
|
||
HttpContent GetErrorContent (HttpURLConnection httpConnection, HttpContent fallbackContent) | ||
HttpContent GetErrorContent (HttpURLConnection httpConnection, HttpContent fallbackContent) | ||
{ | ||
var contentStream = httpConnection.ErrorStream; | ||
|
||
|
@@ -788,7 +788,7 @@ void CollectAuthInfo (HttpHeaderValueCollection <AuthenticationHeaderValue> head | |
|
||
RequestedAuthentication = authData.AsReadOnly (); | ||
} | ||
|
||
AuthenticationScheme GetAuthScheme (string scheme) | ||
{ | ||
if (String.Compare ("basic", scheme, StringComparison.OrdinalIgnoreCase) == 0) | ||
|
@@ -843,7 +843,7 @@ void CopyHeaders (HttpURLConnection httpConnection, HttpResponseMessage response | |
/// <summary> | ||
/// Configure the <see cref="HttpURLConnection"/> before the request is sent. This method is meant to be overriden | ||
/// by applications which need to perform some extra configuration steps on the connection. It is called with all | ||
/// the request headers set, pre-authentication performed (if applicable) but before the request body is set | ||
/// the request headers set, pre-authentication performed (if applicable) but before the request body is set | ||
/// (e.g. for POST requests). The default implementation in AndroidMessageHandler does nothing. | ||
/// </summary> | ||
/// <param name="request">Request data</param> | ||
|
@@ -897,9 +897,9 @@ internal Task SetupRequestInternal (HttpRequestMessage request, HttpURLConnectio | |
/// <summary> | ||
/// Create and configure an instance of <see cref="TrustManagerFactory"/>. The <paramref name="keyStore"/> parameter is set to the | ||
/// return value of the <see cref="ConfigureKeyStore"/> method, so it might be null if the application overrode the method and provided | ||
/// no key store. It will not be <c>null</c> when the default implementation is used. The application can return <c>null</c> from this | ||
/// no key store. It will not be <c>null</c> when the default implementation is used. The application can return <c>null</c> from this | ||
/// method in which case AndroidMessageHandler will create its own instance of the trust manager factory provided that the <see cref="TrustCerts"/> | ||
/// list contains at least one valid certificate. If there are no valid certificates and this method returns <c>null</c>, no custom | ||
/// list contains at least one valid certificate. If there are no valid certificates and this method returns <c>null</c>, no custom | ||
/// trust manager will be created since that would make all the HTTPS requests fail. | ||
/// </summary> | ||
/// <returns>The trust manager factory.</returns> | ||
|
@@ -922,7 +922,7 @@ void AppendEncoding (string encoding, ref List <string>? list) | |
return; | ||
list.Add (encoding); | ||
} | ||
|
||
async Task <HttpURLConnection> SetupRequestInternal (HttpRequestMessage request, URLConnection conn) | ||
{ | ||
if (conn == null) | ||
|
@@ -939,19 +939,19 @@ void AppendEncoding (string encoding, ref List <string>? list) | |
|
||
// SSL context must be set up as soon as possible, before adding any content or | ||
// headers. Otherwise Java won't use the socket factory | ||
SetupSSL (httpConnection as HttpsURLConnection); | ||
SetupSSL (httpConnection as HttpsURLConnection, request); | ||
if (request.Content != null) | ||
AddHeaders (httpConnection, request.Content.Headers); | ||
AddHeaders (httpConnection, request.Headers); | ||
|
||
List <string>? accept_encoding = null; | ||
|
||
decompress_here = false; | ||
if ((AutomaticDecompression & DecompressionMethods.GZip) != 0) { | ||
AppendEncoding (GZIP_ENCODING, ref accept_encoding); | ||
decompress_here = true; | ||
} | ||
|
||
if ((AutomaticDecompression & DecompressionMethods.Deflate) != 0) { | ||
AppendEncoding (DEFLATE_ENCODING, ref accept_encoding); | ||
decompress_here = true; | ||
|
@@ -970,7 +970,7 @@ void AppendEncoding (string encoding, ref List <string>? list) | |
if (!String.IsNullOrEmpty (cookieHeaderValue)) | ||
httpConnection.SetRequestProperty ("Cookie", cookieHeaderValue); | ||
} | ||
|
||
HandlePreAuthentication (httpConnection); | ||
await SetupRequest (request, httpConnection).ConfigureAwait (continueOnCapturedContext: false);; | ||
SetupRequestBody (httpConnection, request); | ||
|
@@ -997,10 +997,11 @@ void AppendEncoding (string encoding, ref List <string>? list) | |
internal SSLSocketFactory? ConfigureCustomSSLSocketFactoryInternal (HttpsURLConnection connection) | ||
=> ConfigureCustomSSLSocketFactoryInternal (connection); | ||
|
||
void SetupSSL (HttpsURLConnection? httpsConnection) | ||
void SetupSSL (HttpsURLConnection? httpsConnection, HttpRequestMessage requestMessage) | ||
{ | ||
if (httpsConnection == null) | ||
if (httpsConnection == null) { | ||
return; | ||
} | ||
|
||
var socketFactory = ConfigureCustomSSLSocketFactory (httpsConnection); | ||
if (socketFactory != null) { | ||
|
@@ -1017,37 +1018,51 @@ void SetupSSL (HttpsURLConnection? httpsConnection) | |
} | ||
#endif | ||
|
||
var keyStore = KeyStore.GetInstance (KeyStore.DefaultType); | ||
keyStore?.Load (null, null); | ||
bool gotCerts = TrustedCerts?.Count > 0; | ||
if (gotCerts) { | ||
for (int i = 0; i < TrustedCerts!.Count; i++) { | ||
Certificate cert = TrustedCerts [i]; | ||
if (cert == null) | ||
continue; | ||
keyStore?.SetCertificateEntry ($"ca{i}", cert); | ||
} | ||
} | ||
var keyStore = InitializeKeyStore (out bool gotCerts); | ||
keyStore = ConfigureKeyStore (keyStore); | ||
var kmf = ConfigureKeyManagerFactory (keyStore); | ||
var tmf = ConfigureTrustManagerFactory (keyStore); | ||
|
||
if (tmf == null) { | ||
// If there are no certs and no trust manager factory, we can't use a custom manager | ||
// because it will cause all the HTTPS requests to fail because of unverified trust | ||
// chain | ||
if (!gotCerts) | ||
if (tmf == null) | ||
{ | ||
// If there are no trusted certs, no custom trust manager factory or custom certificate validation callback | ||
// there is no point in changing the behavior of the default SSL socket factory | ||
if (!gotCerts && ServerCertificateCustomValidationCallback == null) | ||
return; | ||
|
||
tmf = TrustManagerFactory.GetInstance (TrustManagerFactory.DefaultAlgorithm); | ||
tmf?.Init (keyStore); | ||
tmf?.Init (gotCerts ? keyStore : null); // only use the custom key store if the user defined any trusted certs | ||
} | ||
|
||
ITrustManager[]? trustManagers = | ||
ServerCertificateCustomValidationCallback == null | ||
? tmf?.GetTrustManagers () | ||
: X509TrustManagerWithValidationCallback.Inject(tmf?.GetTrustManagers (), requestMessage, ServerCertificateCustomValidationCallback); | ||
|
||
var context = SSLContext.GetInstance ("TLS"); | ||
context?.Init (kmf?.GetKeyManagers (), tmf?.GetTrustManagers (), null); | ||
context?.Init (kmf?.GetKeyManagers (), trustManagers, null); | ||
httpsConnection.SSLSocketFactory = context?.SocketFactory; | ||
|
||
KeyStore? InitializeKeyStore (out bool gotCerts) | ||
{ | ||
var keyStore = KeyStore.GetInstance (KeyStore.DefaultType); | ||
keyStore?.Load (null, null); | ||
gotCerts = TrustedCerts?.Count > 0; | ||
|
||
if (gotCerts) { | ||
for (int i = 0; i < TrustedCerts!.Count; i++) { | ||
Certificate cert = TrustedCerts [i]; | ||
if (cert == null) | ||
continue; | ||
keyStore?.SetCertificateEntry ($"ca{i}", cert); | ||
} | ||
} | ||
|
||
return keyStore; | ||
} | ||
} | ||
|
||
|
||
|
||
void HandlePreAuthentication (HttpURLConnection httpConnection) | ||
{ | ||
var data = PreAuthenticationData; | ||
|
@@ -1093,7 +1108,7 @@ void AddHeaders (HttpURLConnection conn, HttpHeaders headers) | |
conn.SetRequestProperty (header.Key, header.Value != null ? String.Join (GetHeaderSeparator (header.Key), header.Value) : String.Empty); | ||
} | ||
} | ||
|
||
void SetupRequestBody (HttpURLConnection httpConnection, HttpRequestMessage request) | ||
{ | ||
if (request.Content == null) { | ||
|
@@ -1116,4 +1131,4 @@ void SetupRequestBody (HttpURLConnection httpConnection, HttpRequestMessage requ | |
} | ||
} | ||
|
||
} | ||
} |
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.
@simonrozsival: Why add a new app instead of adding the new tests to the existing
tests/Mono.Android-Tests
app? I don't understand the rationale.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.
I started with a test in
tests/Mono.Android-Tests
but I wasn't able to get it woring. I was gettingJava.Lang.ClassNotFoundException : xamarin.android.net.X509TrustManagerWithValidationCallback
because a java wrapper for the class I added,X509TrustManagerWithValidationCallback
, isn't being generated automatically. I got some tips from you and I instead created an apk project which allowed me to run the test successfully.I reverted the apk project. I think I somehow need to special-case the
X509TrustManagerWithValidationCallback
type so that it's built intomono.android.jar
to make the test work inside oftests/Mono.Android-Tests
.