When I specify --enable-password-save to ./configure askpass is able
to read the password from a file.
Seems despite what the --help says it actually defaults to off. :-(
Shouldn't it on be the default?
I'm not an OpenSSL ninja, but this looks suspicious:
------
if (!ENGINE_init(e))
msg (M_SSLERR, "Cannot initialize engine '%s'", engine);
ENGINE_free(e); /* free +1 reference count of the above two */
[... more uses of e ...]
------
Seems all the M_SSLERR lines there should set e=NULL and stop
processing, to keep the same behaviour.
But yes, it works.
On 18 June 2012 21:40, Alon Bar-Lev <alon.bar...@gmail.com> wrote:
> Hello Thomas,
>
> Thank you for your comments and help.
> I've updated the branch based on your work, but with changes...
> The password is secret, and there is a standard mechanism in openvpn
> to handle password...
> So I tried to use it.
>
> For the conditionals, I wanted to get rid of the openssl engine
> conditionals... anyway, the conditionals is not for the ENGINE type as
> it does exists in 0.9.7 initial release. So I think that reducing use
> of conditionals is better for reading the code.
>
> Can you please re-test and comment?
>
> Thanks!
> Alon.
>
> On Mon, Jun 18, 2012 at 10:12 PM, Thomas Habets <tho...@habets.se> wrote:
>> I can confirm that it works. I need to specify both engine and
>> engine-pvk in the config though. If "engine" is not specified then
>> ENGINE_load_builtin_engines() is never called. If you had this in mind
>> then I think "engine-pvk" should require "engine". (just putting
>> "engine" in the config file is enough, without parameter)
>>
>> Other nits:
>> The call to ENGINE_load_private_key() should probably be surrounded by
>> #ifdef HAVE_OPENSSL_ENGINE.
>>
>> Presumably without HAVE_OPENSSL_ENGINE the ENGINE struct and
>> openssl/engine.h doesn't exist, so they should be #ifdeffed too.
>>
>> Fixed this, and added SRK password option, in attached patch to be
>> applied on top of your engine branch. If option is not given then the
>> default UI functions are called (for linux at console that means
>> prompt the user).
>>
>> (the SRK password is not really a secret. If you have it then you're
>> allowed to do operations with the TPM chip. But you have to supply
>> the tpm-sealed data when doing the operation. I've seen several
>> recommendations to just leave it blank)
>>
>> On 18 June 2012 15:22, Alon Bar-Lev <alon.bar...@gmail.com> wrote:
>>> Oh...
>>> And I forgot mentioning that the UI method should be solved, using the
>>> default is not something that is usable for openvpn.
>>> Can you please take care of this?
>>>
>>> Alon.
>>>
>>> On Mon, Jun 18, 2012 at 3:25 PM, Alon Bar-Lev <alon.bar...@gmail.com> wrote:
>>>> 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;
>>
>>
>>
>> --
>> 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;
