On 04/11/17 20:34, Selva wrote:
> 
> 
> On Sat, Nov 4, 2017 at 1:58 PM, Gert Doering <g...@greenie.muc.de
> <mailto:g...@greenie.muc.de>> wrote:
> 
>     Hi,
> 
>     On Wed, Nov 01, 2017 at 07:24:02PM +0100, Steffan Karger wrote:
>     > This looks like it should use our user query wrappers from (e.g.)
>     > console.h.  David, you're the expert here, what should James use to
>     > query for passwords?
> 
>     The mechanics are "query_user_...()", most conveniently
> 
>     /**
>      * A plain "make Gert happy" wrapper.  Same arguments as @query_user_add
>      */
>     static inline bool
>     query_user_SINGLE(char *prompt, size_t prompt_len,
>                       char *resp, size_t resp_len,
>                       bool echo)
> 
>     (console.h)
> 
>     ... most notably this is important because it can use "other mechanisms"
>     if console input is not available, for example, systemd querying.
> 
> 
> Shouldn't this  happen automatically in this particular case as the
> patch uses
> SSL_CTX_get_default_passwd_cb_userdata() which would result in openssl using
> to the password callback previously set in ssl_openssl.c ? And that
> callback is
> get_userpass() which should know whether to query the management, console or
> something else.

All the hoops, loops and twists we take to retrieve a simple password
from a user ....

TL;DR: Yes, Selva is correct.

From the OpenSSL docs:

   SSL_CTX_get_default_passwd_cb() returns a function pointer to the
   password callback currently set in ctx.  If no callback was
   explicitly set, the NULL pointer is returned.

   SSL_CTX_get_default_passwd_cb_userdata() returns a pointer to the
   userdata currently set in ctx.   If no userdata was explicitly set,
   the NULL pointer is returned"

   (This is set using SSL_CTX_set_default_password_cb_userdata())

In tls_ctx_set_options() [ssl_openssl.c:267], we call:

   SSL_CTX_set_default_passwd_cb(ctx->ctx, pem_password_callback);

The pem_password_callback() again calls pem_password_setup()
[ssl.c:374-396] and the latter one calls get_user_pass() [misc.h:244]
which calls get_user_pass_cr() [misc.c:869].  This function will call
query_user_SINGLE() or using the management interface if that's what the
OpenVPN process have been told to use.

So when James' patch calls SSL_CTX_get_default_passwd_cb(ctx), a
function pointer to pem_password_callback() is returned, which then is
called with the cb() call a few lines further down.

I don't see us using the userdata part of this call chain (I don't see
traces of SSL_set_default_passwd_cb_userdata() in our code).  But having
the support for it makes no harm.  Until we start using it, it will be NULL.

And when seeing this part of the patch.  It makes me wonder if similar
approaches should be adopted a few other places too.  But that's a
completely different discussion.

I need to spend a bit more time to fully grasp the UI get/set calls and
the related implementation.  But what is done in regards to password
retrieving in ui_read() makes sense to me.


-- 
kind regards,

David Sommerseth
OpenVPN, Inc


Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to