Hello!
On Fri, Aug 01, 2014 at 09:20:02AM +0400, Dmitrii Pichulin wrote:
> On 31.07.2014 17:49, Maxim Dounin wrote:
> >> + if (engine == NULL) {
> >> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> >> + "ENGINE_by_id(\"%s\") failed", p);
> >> + return NGX_ERROR;
> >> + }
> >> +
> >> + p[last - p] = ':';
> >> +
> >> + if (passwords) {
> >> + pwd = passwords->elts;
> >> +
> >> + ngx_cpystrn(pwd_buf, pwd->data, pwd->len + 1);
> >> +
> >> + pwd_data.password = pwd_buf;
> >> + } else {
> >> + pwd_data.password = NULL;
> >> + }
> >> + pwd_data.prompt_info = NULL;
> >> +
> >> + last++;
> >> +
> >> + private_key = ENGINE_load_private_key(engine, last, 0,
> >> + (void *) &pwd_data);
> > I don't see how it's expected to work. You only pass private data
> > for UI callbacks, but not callbacks itself.
> >
> > Anyway, proper implementation of passing key passwords into an engine
> > seems to be rather big, and as per my reading of the code under
> > crypto/engine won't work with most of the engines anyway. It
> > might be better idea to don't try to do this for now.
>
> Maxim, our vision is based on 2 implementations of engines as previously
> noted:
> 1) gost_capi — doesn't support external passwords
> 2) opensc — with such code from get_pin function
> (https://github.com/OpenSC/engine_pkcs11/blob/master/src/engine_pkcs11.c):
>
> /* either get the pin code from the supplied callback data, or get the pin
> * via asking our self. In both cases keep a copy of the pin code in the
> * pin variable (strdup'ed copy). */
> static int get_pin(UI_METHOD * ui_method, void *callback_data)
> {
> UI *ui;
> struct {
> const void *password;
> const char *prompt_info;
> } *mycb = callback_data;
>
> /* pin in the call back data, copy and use */
> if (mycb != NULL && mycb->password) {
> pin = (char *)calloc(MAX_PIN_LENGTH, sizeof(char));
> if (!pin)
> return 0;
> strncpy(pin,mycb->password,MAX_PIN_LENGTH);
> pin_length = MAX_PIN_LENGTH;
> return 1;
> }
> ...
>
> As you can see, there's no need for ui_method if a password is present.
The code in the get_pin() function provided looks utterly wrong -
it tries to access private data of UI_METHOD, assuming some
structure. This may work if stucture match (it seems to be
copied from PW_CB_DATA used by openssl apps, see apps/apps.[ch]),
but will result in segmentation fault if it doesn't.
Proper engine behaviour would be to use provided UI_METHOD's
callbacks to ask for a password, using the
UI_INPUT_FLAG_DEFAULT_PWD flag, see engines/e_chil.c for an
example. This seems to be understood at least by openssl itself
and curl, see here:
http://curl.haxx.se/mail/lib-2013-08/0265.html
> We suggest to implement something like this:
>
> typedef struct {
> const void *password;
> const char *prompt_info;
> ngx_array_t *passwords;
> ngx_uint_t position;
> } ngx_openssl_pw_cb_data_ex;
>
> In this case, our ui_read implementation can run through all passwords,
> while supporting the basics.
There is no need to provide complex structure as there should be a
loop to call ENGINE_load_private_keyi() multiple times anyway.
> Or it would be better to pass nothing for now?
Given the fact how crappy underlying code in engines is, I
seriously doubt it worth supporting. Better to don't try for now.
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
