> On Sept. 19, 2012, 4:43 p.m., Rafael Schloming wrote: > > /proton/trunk/proton-c/src/ssl/openssl.c, line 385 > > <https://reviews.apache.org/r/7171/diff/1/?file=158481#file158481line385> > > > > For sasl I actually just return the existing one here. My thinking was > > that it would be convenient to be able to access the existing sasl layer > > given a transport, however I didn't want to have the transport's public API > > depend on sasl at all. However I can see how you might want the option to > > not automatically create the given security layer, so maybe the best of > > both worlds would be to have a separate accessor on the sasl side or a flag > > on the constructor that controls whether the layer is created > > automatically. Whatever we do we should probably be consistent between the > > two security layers (ssl & sasl). > > Kenneth Giusti wrote: > I'm a bit confused - my intent was to create a new ssl object if one did > not exist, else just return the existing one (on-demand create). The error > you highlight is a bit of an awkward check to catch the case where the ssl > object's mode is treated inconsistently by the caller (e.g. created as a > _client_, then queried again as a _server_, or vice versa). > > So, the way the code is currently designed, an ssl object of a given type > (client/server) is created on first access via the transport. And, once > created, you cannot change the type of the ssl object. > > Alternatively, we can create separate accessor and initialization > methods, like: > pn_ssl_t *pn_transport_ssl( pn_transport_t * ); // return existing > pn_ssl_t, or create a new one if none present > int pn_ssl_init( pn_ssl_t *, enum {PN_SSL_MODE_CLIENT, > PN_SSL_MODE_SERVER} ). // Set the role of this end of the SSL connection > > > What do you think?
Ah, clearly I was skimming a bit too hard. I like your suggestion modulo the name. I would call it pn_ssl rather than pn_transport_ssl as it is really more of a factory/constructor rather than a method. - Rafael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7171/#review11696 ----------------------------------------------------------- On Sept. 19, 2012, 4:11 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7171/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2012, 4:11 p.m.) > > > Review request for qpid and Rafael Schloming. > > > Description > ------- > > Patch to add SSL/TLS support to proton using OpenSSL. > > > This addresses bug PROTON-2. > https://issues.apache.org/jira/browse/PROTON-2 > > > Diffs > ----- > > /proton/trunk/examples/mailbox/README.txt 1387654 > /proton/trunk/examples/mailbox/fetch 1387654 > /proton/trunk/examples/mailbox/post 1387654 > /proton/trunk/examples/mailbox/server 1387654 > /proton/trunk/examples/mailbox/ssl-setup.sh PRE-CREATION > /proton/trunk/proton-c/CMakeLists.txt 1387654 > /proton/trunk/proton-c/bindings/php/php.i 1387654 > /proton/trunk/proton-c/bindings/python/python.i 1387654 > /proton/trunk/proton-c/bindings/ruby/ruby.i 1387654 > /proton/trunk/proton-c/include/proton/cproton.i 1387654 > /proton/trunk/proton-c/include/proton/driver.h 1387654 > /proton/trunk/proton-c/include/proton/ssl.h PRE-CREATION > /proton/trunk/proton-c/pn_config.h.in 1387654 > /proton/trunk/proton-c/src/driver.c 1387654 > /proton/trunk/proton-c/src/engine/engine-internal.h 1387654 > /proton/trunk/proton-c/src/engine/engine.c 1387654 > /proton/trunk/proton-c/src/ssl/openssl.c PRE-CREATION > /proton/trunk/proton-c/src/ssl/ssl-internal.h PRE-CREATION > /proton/trunk/proton-c/src/ssl/ssl_stub.c PRE-CREATION > > Diff: https://reviews.apache.org/r/7171/diff/ > > > Testing > ------- > > "proton-tests" still works :) > > Updated "mailbox" example to support SSL - see mailbox/README.txt for > instructions on how to set up and run with SSL. > > > Thanks, > > Kenneth Giusti > >
