> On Dec. 11, 2012, 2 p.m., Rafael Schloming wrote: > > /proton/trunk/proton-c/include/proton/ssl.h, line 261 > > <https://reviews.apache.org/r/8449/diff/1/?file=236662#file236662line261> > > > > Maybe I'm missing something here, but isn't this backwards? Isn't the > > wildcard pattern actually contained in the wildcard certificate? Or is this > > something distinct from wildcard certificates, in which case, how does this > > interact with them?
I think I misunderstood this comment against the original bug: https://issues.apache.org/jira/browse/PROTON-161?focusedCommentId=13504688&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13504688 I thought this was a request for client-side wildcards to avoid the need for a callback to the client application to perform any special-case matching. There's a request for a "pluggable" hostname check mechanism, which wouldn't be possible for any of the swig'ed languages. In any case, wildcards _in the certificate_ must be supported and I missed that in this patch. I'll work on that. > On Dec. 11, 2012, 2 p.m., Rafael Schloming wrote: > > /proton/trunk/proton-c/include/proton/ssl.h, line 262 > > <https://reviews.apache.org/r/8449/diff/1/?file=236662#file236662line262> > > > > The API is a bit odd here. It seems like there is overlap, e.g. > > sometimes the match is against the hostname, and sometimes the match is > > against the hostname_match thingy. > > > > I'm also confused about the scenario in which we would ever need to > > supply a pattern against which to do a non-exact match. It seems like > > wildcard certificates would eliminate that need. I can see how maybe if you > > have a virtual server farm and you don't know which certificate you're > > going to get you might want to supply a pattern as the client, but I > > haven't ever heard of that as a standard pattern. (I'm not saying it isn't, > > I could just be ignorant here.) If it's not a standard pattern then maybe > > we should wait until a use case comes up to offer it? I think the API may be trying to do too much here. There are three separate aspects it's trying to cover: 1) Sending SNI to the server (see: http://en.wikipedia.org/wiki/Server_Name_Indication) 2) validating the name contained in the certificate sent by the peer, and 3) support for "custom" client side name checking. #1 is pretty straightforward - we need to send the server the name we're expecting to authenticate so the server can select the correct certificate. This is provided by the set_peer_hostname call. Configuration for #2 should be simplified by using the name given by #1 by default. And I think this would cover most use cases. The API becomes more complex if we want to: o) support #3, which includes overriding the hostname set by #1 o) allow SNI, but disable host name check (#2) Both of these features led to the addition of the "set_peer_hostname_pattern" api. As far as wildcard certificates are concerned: I think this is a separate issue from the client configuration. Yes, we need to support them for #2 regardless of how/if we allow #3 [and the current patch does *not* - I need to fix that]. - Kenneth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8449/#review14290 ----------------------------------------------------------- On Dec. 10, 2012, 10:06 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8449/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2012, 10:06 p.m.) > > > Review request for qpid and Rafael Schloming. > > > Description > ------- > > Adds an API call to set the hostname which is used for Server Name Indication > as well as Common Name matching. Right now only support exact text match - > no wildcarding. > > > This addresses bug proton-161. > https://issues.apache.org/jira/browse/proton-161 > > > Diffs > ----- > > /proton/trunk/proton-c/bindings/python/proton.py 1419512 > /proton/trunk/proton-c/include/proton/ssl.h 1419512 > /proton/trunk/proton-c/src/ssl/openssl.c 1419512 > /proton/trunk/tests/proton_tests/ssl.py 1419512 > /proton/trunk/tests/proton_tests/ssl_db/README.txt 1419512 > /proton/trunk/tests/proton_tests/ssl_db/ca-certificate.pem 1419512 > /proton/trunk/tests/proton_tests/ssl_db/ca-private-key.pem PRE-CREATION > /proton/trunk/tests/proton_tests/ssl_db/client-certificate.pem 1419512 > /proton/trunk/tests/proton_tests/ssl_db/client-private-key.pem 1419512 > /proton/trunk/tests/proton_tests/ssl_db/server-certificate.pem 1419512 > /proton/trunk/tests/proton_tests/ssl_db/server-private-key.pem 1419512 > > Diff: https://reviews.apache.org/r/8449/diff/ > > > Testing > ------- > > Updated ssl unit tests. > > > Thanks, > > Kenneth Giusti > >
