rschmitt commented on PR #773:
URL: 
https://github.com/apache/httpcomponents-client/pull/773#issuecomment-3741157819

   I've been looking at this a bit today, comparing 
`SSLContexts.createDefault()` and `SSLContexts.createSystemDefault()`. It 
appears that the following system properties are _always_ respected:
   
   ```
   ssl.TrustManagerFactory.algorithm
   javax.net.ssl.trustStoreType
   javax.net.ssl.trustStore
   javax.net.ssl.trustStoreProvider
   javax.net.ssl.trustStorePassword
   ```
   
   I tested this by setting up a test server with a self-signed cert, along 
with a corresponding trust store. I call the server using a modified 
`ClientCustomSSL` from the `examples/` directory. What I found is that calls to 
the test endpoint succeed _if and only if_ I pass 
`-Djavax.net.ssl.trustStore=...` and so forth; even 
`SSLContexts.createDefault()` respects the trust store system properties. 
Additionally, both of these methods respect respect system-wide trust store 
configuration: I'm able to hit my company's internal endpoints with either 
`SSLContext`, and in the debugger I see the same trust store array contents 
with about ~140 certs.
   
   The practical difference between the two methods is in _key_ store 
configuration, i.e. mutual TLS:
   
   ```
   ssl.KeyManagerFactory.algorithm
   javax.net.ssl.keyStoreType
   javax.net.ssl.keyStore
   javax.net.ssl.keyStoreProvider
   javax.net.ssl.keyStorePassword
   ```
   
   I used logging breakpoints to verify that the JDK only reads these 
properties if `createSystemDefault()` is called.
   
   Finally, there are these oddball properties:
   
   ```
   http.keepAlive
   http.agent
   https.protocols
   https.cipherSuites
   ```
   
   These are properties for the JDK's built-in HTTP client that we manually 
reimplement; `SSLContext` doesn't look at them.
   
   This trust store behavior makes my other finding much less surprising: 
changing `systemProperties` to default to `true` doesn't seem to break any 
consumers. They all rebuild successfully. The change could still cause a 
runtime failure, or an integration test failure, but I still would have 
expected at least one unit test failure.
   
   Based on the above, I'm much more comfortable with changing the default 
behavior, in principle. My current thinking is:
   
   1. Use `SystemDefaultRoutePlanner` with the default `ProxySelector` by 
default. This brings proxy configuration behavior in line with trust store 
configuration behavior: use the JDK defaults (whether OS-wide or overridden by 
system property) unless programmatically configured not to do so (in this case, 
with a custom `HttpRoutePlanner`). The support for OS-specific, system-wide 
proxy configuration (gated behind `-Djava.net.useSystemProxies=true`) is worth 
noting because it's backed by a bunch of non-trivial native code (see for 
instance the [Windows 
implementation](https://github.com/openjdk/jdk/blob/master/src/java.base/windows/native/libnet/DefaultProxySelector.c)).
   2. Drop support for the `https.protocols` and `https.cipherSuites` system 
properties. We'll no longer check an arbitrary subset of the system properties 
respected by `HttpsURLConnection`.
   3. Use `SSLContexts.createSystemDefault()` by default, or change 
`SSLContexts.createDefault()` to delegate to it. Anyone using mTLS is either 
already enabling `systemProperties` or is programmatically setting a custom 
`SSLContext`, and I don't think that adding key stores has any effect on 
non-mTLS handshakes.
   4. Mark `useSystemProperties()` as `@Deprecated` and document that it is a 
no-op retained for compatibility.
   
   The only loose end here is these proxy auth system properties that are 
checked by `SystemDefaultCredentialsProvider`. I'm not sure what's going on 
there. I can't find any references in documentation to the `http.proxyUser` and 
`http.proxyPassword` system properties; it looks like they've been dropped from 
the JDK entirely, and were previously an undocumented implementation detail of 
JAX-WS. They don't look like standard properties in any sense.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to