Hello Thomas, I did not have the global variable in mind :)
I thought about your initial suggestion of specific private key engine, and it has value, so I added a new option. I propose the following [1], the problem is that I cannot test this out. While looking on the current engine implementation, there is a need to pass the pre/post strings, so there is more work to be done here, I will do this later. I also don't understand the need of using the dynamic in try_load_engine, but this will be resolved by supporting pre/post strings. Please review and test, should reach the same functionality as you require. Regards, Alon. [1] https://github.com/alonbl/openvpn/compare/master...engine On Mon, Jun 18, 2012 at 12:45 AM, Thomas Habets <tho...@habets.se> wrote: > Those questions are why I'd prefer to reuse the already loaded ENGINE > (engine_persist in crypto_openssl), but it didn't appear to be > exported from the crypto backend (crypto_backend.h), which is why my > previous patch added exporting of it (by means of the init function). > > All versions of the patch works with non-static modules. The TPM one > I'm using is a .so file. > http://packages.debian.org/squeeze/amd64/libengine-tpm-openssl/filelist > > Do you want a separate ENGINE struct or not? > If same, how should it be exported from crypto_openssl to ssl_openssl? > I don't see any non-static accessor. > If different, call setup_engine() from ssl_openssl.c (meaning turning > setup_engine() non-static)? > > I was hesitant to create new non-statics in crypto_openssl, but one > easy solution is of course to just make > crypto_openssl.c::engine_persist non-static and use it directly, as > seen in attached patch. > > Seems I don't need to call ENGINE_init() at all. The attached patch > works, at least. > > I appreciate the code discipline. Really I do. :-) > > Regards, > Thomas > > On 17 June 2012 22:04, Alon Bar-Lev <alon.bar...@gmail.com> wrote: >> Yes, almost :) >> >> Won't it better to call ENGINE_init at setup_engine() or at >> try_load_engine() instead of at tls_ctx_load_priv_file()? It is just >> that tls_ctx_load_priv_file() can be called more than once, while the >> init should be called once, right? >> Are you sure all works well if engine is not statically linked? >> What about ENGINE_finish() at proper place? >> >> Thank you for your patience, >> Alon Bar-Lev. >> >> >> On Sun, Jun 17, 2012 at 11:53 PM, Thomas Habets <tho...@habets.se> wrote: >>> Hi. >>> >>> Need? No. I thought you preferred reusing the loaded/inited ENGINE >>> struct cached by existing code instead of creating a new one. >>> >>> Is the attached patch what you had in mind? >>> >>> (same description/sign-off) >>> >>> Regards, >>> Thomas >>> >>> >>> On 17 June 2012 12:12, Alon Bar-Lev <alon.bar...@gmail.com> wrote: >>>> Hi, >>>> >>>> Why do we need to crypto_init_lib_engine() twice? Can you please take >>>> a look at init_crypto_pre:: init_crypto_pre()? >>>> >>>> I also think crypto_init_lib_engine() should not return the engine... >>>> as won't it simpler to use ENGINE_by_id() at >>>> ssl_openssl.c::tls_ctx_load_priv_file()? >>>> >>>> Alon. >>>> >>>> On Sun, Jun 17, 2012 at 1:02 PM, Thomas Habets <tho...@habets.se> wrote: >>>>> Hi. >>>>> >>>>> Ah yes, I first made the patch to an older version where some of these >>>>> things don't apply, and then forward-ported it. >>>>> >>>>> How about this? >>>>> --------- >>>>> Add support for SSL engine loading the private key. >>>>> >>>>> Option 'engine' is used to specify the name of the engine that >>>>> will load the private key. >>>>> >>>>> For example this can be "tpm" to use the OpenSSL TPM engine module >>>>> (libengine-tpm-openssl in Debian). >>>>> >>>>> It defaults to the built-in UI methods because openssl-tpm-engine >>>>> doesn't yet support user data being sent to the callback functions. >>>>> A patch for that on its way to them. >>>>> >>>>> Some more details: >>>>> http://blog.habets.pp.se/2012/02/TPM-backed-SSL >>>>> >>>>> Signed-off-by: Thomas Habets <hab...@google.com> >>>>> >>>>> >>>>> >>>>> On 17 June 2012 01:11, Alon Bar-Lev <alon.bar...@gmail.com> wrote: >>>>>> Hello, >>>>>> >>>>>> It is a good idea. >>>>>> But first, please remove the emacs stuff. >>>>>> >>>>>> Now, I see that the ENGINE_load_builtin_engines() is already called at >>>>>> crypto_openssl.c::crypto_init_lib_engine, is there any require to >>>>>> duplicate this? >>>>>> >>>>>> There is already "engine" option, available only to polarssl, it can >>>>>> easily and correct way be used also for openssl, instead of having >>>>>> another option. >>>>>> >>>>>> What do you think? >>>>>> Alon. >>>>>> >>>>>> >>>>>> On Sun, Jun 17, 2012 at 2:50 AM, Thomas Habets <tho...@habets.se> wrote: >>>>>>> Patch attached. >>>>>>> >>>>>>> Add support for SSL engine loading the private key. >>>>>>> >>>>>>> Added option 'key-engine' specifying the name of the engine that >>>>>>> will load the private key. >>>>>>> >>>>>>> For example this can be "tpm" to use the OpenSSL TPM engine module >>>>>>> (libengine-tpm-openssl in Debian). >>>>>>> >>>>>>> It defaults to the built-in UI methods because openssl-tpm-engine >>>>>>> doesn't yet support user data being sent to the callback functions. >>>>>>> A patch for that on its way to them. >>>>>>> >>>>>>> Some more details: >>>>>>> http://blog.habets.pp.se/2012/02/TPM-backed-SSL >>>>>>> >>>>>>> Signed-off-by: Thomas Habets <hab...@google.com> >>>>>>> >>>>>>> -- >>>>>>> typedef struct me_s { >>>>>>> char name[] = { "Thomas Habets" }; >>>>>>> char email[] = { "tho...@habets.pp.se" }; >>>>>>> char kernel[] = { "Linux" }; >>>>>>> char *pgpKey[] = { "http://www.habets.pp.se/pubkey.txt"; }; >>>>>>> char pgp[] = { "A8A3 D1DD 4AE0 8467 7FDE 0945 286A E90A AD48 E854" }; >>>>>>> char coolcmd[] = { "echo '. ./_&. ./_'>_;. ./_" }; >>>>>>> } me_t; >>>>>>> >>>>>>> ------------------------------------------------------------------------------ >>>>>>> Live Security Virtual Conference >>>>>>> Exclusive live event will cover all the ways today's security and >>>>>>> threat landscape has changed and how IT managers can respond. >>>>>>> Discussions >>>>>>> will include endpoint security, mobile security and the latest in >>>>>>> malware >>>>>>> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ >>>>>>> _______________________________________________ >>>>>>> Openvpn-devel mailing list >>>>>>> Openvpn-devel@lists.sourceforge.net >>>>>>> https://lists.sourceforge.net/lists/listinfo/openvpn-devel >>>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> typedef struct me_s { >>>>> char name[] = { "Thomas Habets" }; >>>>> char email[] = { "tho...@habets.pp.se" }; >>>>> char kernel[] = { "Linux" }; >>>>> char *pgpKey[] = { "http://www.habets.pp.se/pubkey.txt"; }; >>>>> char pgp[] = { "A8A3 D1DD 4AE0 8467 7FDE 0945 286A E90A AD48 E854" }; >>>>> char coolcmd[] = { "echo '. ./_&. ./_'>_;. ./_" }; >>>>> } me_t; > > > > -- > typedef struct me_s { > char name[] = { "Thomas Habets" }; > char email[] = { "tho...@habets.pp.se" }; > char kernel[] = { "Linux" }; > char *pgpKey[] = { "http://www.habets.pp.se/pubkey.txt"; }; > char pgp[] = { "A8A3 D1DD 4AE0 8467 7FDE 0945 286A E90A AD48 E854" }; > char coolcmd[] = { "echo '. ./_&. ./_'>_;. ./_" }; > } me_t;
