On Feb 1, 2013, at 11:57 PM, Bill Burke <bburke_at_redhat.com> wrote:
> I am very disappointed in what you've come up with, so much so, I don't think it should even be included. I'll be completely blunt. What you've provided is almost completely worthless. I can almost guarantee you users will be forced to use vendor-specific APIs any time they use the JAX-RS client API.
>
> For example, Turning a keystore and a truststore into an SSLContext is a real pain and you're missing a big opportunity to simplify things for 90% of SSL use cases. For example, this is what it looks like:
>
>
>
> KeyStore keystore = ...;
> KeyStore truststore = ...;
>
> String alg=KeyManagerFactory.getDefaultAlgorithm();
> KeyManagerFactory kmFact=KeyManagerFactory.getInstance(alg);
>
> kmFact.init(keystore, trustStorePassword.toCharArray());
> KeyManager[] keyManagers = kmFact.getKeyManagers();
> alg=TrustManagerFactory.getDefaultAlgorithm();
> TrustManagerFactory tmFact=TrustManagerFactory.getInstance(alg);
> tmFact.init(truststore );
> TrustManager[] trust = tmFact.getTrustManagers();
>
> SSLContext context = SSLContext.getInstance("SSL");
> context.init(keyManagers, trust, null);
>
>
> What is wrong with the additional methods I had in Resteasy's builder? i.e. being able to pass in the truststore and keystore? What is so bad about setting a connection TTL? Pool size? All things we're all gonna want to have in all our implementations.
>
> https://github.com/resteasy/Resteasy/blob/3.0-beta-2/jaxrs/resteasy-client/src/main/java/org/jboss/resteasy/client/jaxrs/AbstractClientBuilder.java
>
> I'd at least like to get an explanation of why 90% of my proposal was ignored/ditched.
I believe I did provide an explanation.
There's a lot that can be configured in SSL and we do not have enough cycles to properly design it and implement all the concepts in RI in time. For example, your key/trust store setters only cover the most basic config scenario. In order to provide a full coverage, we would need a lot more methods. And rather than exposing lots of new setters in the client builder API I'd like to see if there is a chance we could provide a separate "SSL configuration utility" instead that would produce the SSL context - something similar to what Grizzly provides (see bellow). Etc. But I do not have enough cycles right now to explore all these options and to convert them into a polished API proposal.
As for non-SSL configuration (TTL, pool size, HTTP auth, ...), I understand that all of it may be easy to implement with Grizzly client or AHC, but RI has to provide default implementation based on HttpURLConnection. And again, we do not have enough cycles for that, not to mention the fact that these other config settings have NOTHING to do with security configuration and, as I have mentioned multiple times in this forum, now is not the right time for sneaking in new features. We're past PRD, approaching final release. If we were not able to add these features to the client API in the last 2 years, we may need to wait until MR is out. Sorry.
> Finally, IMO, ClientFactory should be ditched and set property methods should be added to ClientBuilder. This allows implementation code to construct the client based on configuration properties rather than having to worry about concurrency or partial initialized Client instances that could be reconfigured if a property is set intermittently.
>
> Anyways, -1000 for what you've got. I'd rather not have it at all.
That's always an option...
We can certainly ditch the whole thing. Such decision would at least give me more time to focus on resolving issues in RI and existing API features. Still, I would prefer we go over the list of features that did not get in, prioritise them and maybe try to squeeze in one or two additional feats from the list that you and other EG members find most important.
In case you change your mind, here's the my contribution to the list of features for consideration:
- host name verification policy
- ClientBuilder implements Configurable
- KeyStore & TrustStore setting (is overridden if SSLContext has been set); even though I'd still prefer to keep it in a separate utility class, similar to Grizzly SSLContextConfigurator (
http://java.net/projects/grizzly/sources/git/content/modules/grizzly/src/main/java/org/glassfish/grizzly/ssl/SSLContextConfigurator.java)
Marek
> On 2/1/2013 11:40 AM, Marek Potociar wrote:
>> Hello experts,
>>
>> I'd like you to review a proposal for the basic client security
>> configuration support in JAX-RS 2.0. After a careful deliberation and
>> taking into account recent schedule adjustments, I'd like to propose a
>> following resolution:
>>
>> *1. SSL configuration:*
>> In light of all the possible configuration aspects around TLS[1], we
>> propose to agree upon the absolute minimum, that is providing a custom
>> SSLConfig instance into the client run-time.
>>
>> New ClientBuilder interface has been introduced that supports setting
>> SSLConfig:
>> https://github.com/mpotociar/jax-rs/blob/master/src/jax-rs-api/src/main/java/javax/ws/rs/client/ClientBuilder.java
>>
>> The newly introduced client builder is supposed to be provided by a new
>> static ClientFactory method. To facilitate the change, ClientFactory has
>> been updated:
>> https://github.com/mpotociar/jax-rs/blob/master/src/jax-rs-api/src/main/java/javax/ws/rs/client/ClientFactory.java
>>
>> A new SSL config getter is introduce in the Client API:
>> https://github.com/mpotociar/jax-rs/blob/master/src/jax-rs-api/src/main/java/javax/ws/rs/client/Client.java
>>
>>
>> *2. HTTP Authentication:*
>> We do not have enough cycles to provide a well-designed authentication
>> API and RI implementation. There is also a potential of a collision with
>> a Java SE HTTP client API (currently work in progress). The proposal is
>> therefore to defer HTTP auth support to a maintenance release and align
>> it with the new Java SE HTTP client API.
>>
>> We're looking forward to your feedback. Please send us your feedback by
>> no later then *Wed, Feb 06, 2013 CoB*!
>>
>> Thank you,
>> Marek & Santiago
>>
>> [1] A deeper analysis of SSLContext creation tree shows the following
>> configuration aspects that are involved:
>>
>> - SSL engine (SSLEngine: created using SSLContext)
>> - [*] enabled cipher suites (String[])
>> - [*] enabled protocols (String[])
>> - *SSL context* (SSLContext: created from provided data)
>> - *protocol* (String)
>> - *provider* (String/Provider)
>> - *secure random generator source* (SecureRandom)
>> - key managers (KeyManager[]: provided by KeyManagerFactory)
>> - key manager factory (KeyManagerFactory)
>> - *algorithm* (String)
>> - *provider* (String/Provider)
>> - *key store password* (char[])
>> - *key store key password* (char[])
>> - *key store* (KeyStore)
>> - type (String)
>> - provider (String/Provider)
>> - store data (URL/InputStream/byte[]/...)
>> - trust managers (TrustManager[])
>> - trust manager factory (TrustManagerFactory)
>> - *algorithm* (String)
>> - *provider* (String/Provider)
>> - *trust store password* (char[])
>> - *trust store* (KeyStore)
>> - type (String)
>> - provider (String/Provider)
>> - store data (URL/InputStream/byte[]/...)
>> -------
>> Legend:
>>
>> [*] = optional config
>>
>> The *bold black* items indicate the configuration properties related to
>> full configuration of a SSL context. For those of you who use pure-text
>> email clients, these properties include:
>>
>> /context protocol, context protocol provider, secure randomness source,
>> keystore algorithm, keystore algorithm provider, keystore password,
>> keystore key password, keystore data, truststore algorithm, truststore
>> alg. provider, truststore password, truststore data/
>>
>> As you can see, it's quite a lot of configuration, and all of it is
>> already captured in Java SE APIs that assist in creation of SSLContext.
>> So given our timeframe, we propose to focus really only on the most
>> essential configuration, which is setting the pre-configured SSLContext
>> instance. We suggest to revisit any additional convenience SSL config
>> methods as part of a maintenance release.
>
> --
> Bill Burke
> JBoss, a division of Red Hat
> http://bill.burkecentral.com