On 10/12/12 15:30, Bill Burke wrote:
>
>
> On 12/8/2012 5:45 PM, Sergey Beryozkin wrote:
>> On 07/12/12 14:11, Bill Burke wrote:
>>>
>>>
>>> On 12/7/2012 5:46 AM, Marek Potociar wrote:
>>>> Hi Bill,
>>>> Thank you for writing this up. It's been sitting on my plate for too
>>>> long.This is a great starting point. A few thoughts/questions:
>>>>
>>>
>>> I'm open to anything here as I'm not sure what the optimal solution is
>>> yet.
>>>
>>>> IIUC, you suggest we re-introduce a "client builder" concept that
>>>> would be represented by ClientFactory. I'm fine with that.
>>>
>>> Ya, I'm sorry you had something similar way back when and I fought
>>> against it. I fought mainly because we had no configuration to build the
>>> client with.
>>>
>>>> With KeyStore/TrustManager ClientFactory builder methods in place,
>>>> should we expose similar methods via client-side configuration? Or
>>>> does the proposal mean that users would not be able to modify
>>>> keystore/truststore etc. on any other level than Client (i.e. not in
>>>> WebTarget or Invocation.Builder)? Are there any common use cases that
>>>> we would be ignoring? IOW, do you see a need to change
>>>> keystore/truststore configuration for different resource targets?
>>>> Also, I think we need to expose a method that would accept a custom
>>>> SecureRandom instance.
>>>>
>>>
>>> I don't know about Jersey, but Resteasy's client defaults to Apache Http
>>> Client 4.x and it is by far the most popular. The reason I put this
>>> stuff in a ClientBuilder is because of how Apache Http Client works. You
>>> need the truststore and client-cert keystore available when you create a
>>> SocketFactory in AHC 4.1. If we allowed SSL configuration at the Client
>>> and WebTarget level, you'd have to manage the lifecycle of multiple
>>> HttpClients (and socket factories). java.io.* implementations pretty
>>> much have a one-to-one relationship between stores and SocketFactorys. I
>>> don't think java.nio.* has the same problems, but I haven't done a lot
>>> of SSL + NIO.
>>>
>>> Also, I was thinking of having these additional methods:
>>>
>>> Probably need a way to set properties on the Client builder/factory.
>>> property(String name, Object value);
>>>
>>> disableTrustManager(). Basically will not do cert checks for HTTPS
>>> calls. This is useful when testing or for clients that don't care about
>>> trust. IMO, though, trust should not be disabled by default as disabling
>>> it is a bad thing usually.
>>>
>>> hostNameVerification(HostNameVerificationPolicy verification);
>>>
>>> enum HostNameVerificationPolicy
>>> {
>>> ANY,
>>> BROWSER, // default
>>> STRICT
>>> }
>>>
>>> This checks the CN attribute of the X509PrincipalName of the server's
>>> CERT to make sure it is the host you are actually querying. ANY will not
>>> do the check, BROWSER allows wildcards *.foo.com. Strict requires the CN
>>> to be exactly the same as the requested host.
>>>
>>>
>>>> As for the Authentication, I cannot really decide. On one hand I am
>>>> inclined to prefer a composition-based solution, e.g. something like
>>>>
>>>> target.register(new BasicAuthFilter(user, pwd)) // or
>>>> BasicAuthFeature....
>>>>
>>>
>>> Again, Resteasy at least is based on Apache HTTP Client. While BasicAuth
>>> is very easy to implement, Digest is not as trivial so we delegate to
>>> Apache HTTP Client for the implementation of this.
>>>
>>>
>>> We could also just ditch authentication() and have defined properties:
>>>
>>> webTarget.configuration().setProperty(SecurityProperties.USERNAME,
>>> "bburke");
>>> webTarget.configuration().setProperty(SecurityProperties.PASSWORD,
>>> "geheim");
>>>
>>> In this case, we require client implementations to support basic and
>>> digest auth. How they implement it is up to them.
>>>
>>>
>>>
>>>> but at the same time, your proposal works well with the fluent API and
>>>> with careful javadocing can be extensible in the future too. The
>>>> disadvantage of the above is also the need to provide the
>>>> implementation in the spec and fixing any bugs can become a nightmare
>>>> after we release. So I guess I would like to hear from others what
>>>> they think before I make up my mind.
>>>>
>>>
>>> For authentication, is there a possibility of a bug? Basic and Digest
>>> auth just need username/password and thats basically all we're doing at
>>> the moment.
>>
>> Can we restrict it .basic() only ?
>>
>
> IMO, we should support whatever the servlet spec requires: basic,
> digest, client-cert, and login-form.
>
And where is this requirement to match the servlet spec is coming from ?
> For Form i was thinking:
>
> authentication().loginform(URI actionUri);
>
> The servlet spec defines what the form parameter names should be. The
> only problem is that the spec will return 200 with a error page on auth
> failure. Also, the jax-rs client would have to keep track of session
> cookies.
>
IMHO it does not make sense to model at the client API level the
authentication mechanism targeted at human users; it is not about "yes,
we can" but about "why we should do it"
>
>> Who even uses digest ? At least basic is a well-known mainstream scheme,
>> digest is no more secure than digest without TLS and is more obscure I
>> would say - so I wonder if it should promoted at the API level...
>>
>
> I agree not many use digest, but servlet spec supports Digest, so we
> should too.
>
See above. Supporting basic() (via Authenticator, properties), would be
good IMHO, further API-level requirements can be added seamlessly on
demand.
>> Lets keep a basic shortcut for now, in terms of Authenticator.basic()
>> only.
>>
>> One more note, would it make sense to somehow make setting client key
>> store and non-TLS authentication mutually exclusive ? It does not make
>> sense to use two-way TLS and say basic authentication, though may be ite
>> is not a big deal
>>
>
> I used to think the same, but it does make sense to do both two-way TLS
> and basic-auth. A client-cert basically an auth of the machine you're
> calling from. A password is an auth of the human that is using the
> machine. So, if somebody gets your laptop, they still can't login
> without your password, while the client-cert protects against MITM,
> phishing, and redirect attacks.
>
Yes, good point
Cheers, Sergey
> Bill
>