activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eric White" <e...@ericwhite.ca>
Subject Re: [jira] Commented: (AMQ-1659) SSL Transport configured in wantClientAuth mode never asks for the client certificate during the SSL Handshake
Date Fri, 11 Apr 2008 08:25:33 GMT
On Thu, Apr 10, 2008 at 7:09 PM, David Jencks (JIRA) <jira@apache.org> wrote:
>
>
>     [ https://issues.apache.org/activemq/browse/AMQ-1659?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=42186#action_42186
]
>
>  David Jencks commented on AMQ-1659:
>  -----------------------------------
>
>  In the past I've fixed this bug (in other projects) like this:
>
>     public void bind() throws IOException {
>         super.bind();
>         if (needClientAuth) {
>             ((SSLServerSocket)this.serverSocket).setNeedClientAuth(true);
>         } else if (wantClientAuth) {
>             ((SSLServerSocket)this.serverSocket).setWantClientAuth(true);
>         }
>     }
>
>
>  which to me corresponds better to natural language usage like "I not only want client
auth, I need it!"
>
>  Is there some reason you think that setting both flags true should be disallowed?

The reason I though throwing an exception was a good idea was, in
reading the JDK API, these two options are really mutually exclusive,
if you set one the other is unset.  So, my thinking was URIs
containing: needClientAuth=true&wantClientAuth=true are technically incorrect.

That being said, I prefer the approach that you took.  Because from
the end users perspective it is very difficult to know that
needClientAuth and wantClientAuth override each other deep down inside
of the JDK.  So as you say if the user configuring ActiveMQ and sets
needClientAuth then that should take precedence over wantClientAuth.
This is because needClientAuth is more restrictive of the two.

I'm sorry I didn't run the tests, that was a lapse of judgment on my part.

Would it be possible to update this page:
http://activemq.apache.org/contributing.html
To include something like this in the "Submitting patches" section:

Quick Check List:
1. Does the patch apply clean to the version it is supposed to fix.
2. Does the resulting patched code compile
3. Do the Unit tests run cleanly

All of these are obvious, but it never hurts, to remind everyone.

Thank you very much for fixing this in time for ActiveMQ 4.1.2.  I
really appreciate the quick turn around.

Regards,
Eric

Mime
View raw message