Damien Miller <[EMAIL PROTECTED]>:
> Holger Reif:

>> There has been a discussion on this recently. The conclusion was
>> it should be changed. I think it was even aggred on how to do
>> this. But I think nobody took the task of actually implementing
>> it. (Please correct me if I'm wrong!)
>> 
>> AFAIR it was the idea to do it the same you you did, only the
>> additional parameter would be void* instead of char*.
>>
>> YOu can do the change and submit a patch to this dev list for
>> inclusion into further versions. Please keep in mind that 
>> some apps (like rsa, x509) are using this callback and need
>> changes as well.

> Done :)
> 
> Please find attached a patch against openssl-SNAP-19990718 which
> modifies the pem/* stuff to take an extra argument of type void* 
> to all the functions which use a password callback. It also 
> modifies the apps/* to use the new interface.
> 
> I haven't added backwards compatability functions, but I am 
> happy to do so if required.

Last time this came up, there was a suggestion to name the new
functions <whatever>_ex, because otherwise adding backwards
compatibility is hard to do: You cannot have two functions with the
same name and a different number of arguments; a macro that takes
three arguments and extends to a call to the function of the same name
with four arguments (the fourth one being NULL) would be possible, but
awkward (you wouldn't be able to call the four-parameter version
without #undef'ing the macro first).  Now it should be easy to patch
your patch to change this (no need to go through all the code in the
library again) and add compatibility functions with the old names if
that ..._ex suggestion is what we want to do.

However, it would be a compatibility hack and not a clean solution
(unless the functions that call such callbacks know which kind of
callback they are facing and use "if" statements to do appropriate
function calls, which would blow things up a lot); existing programs
that have a callback fitting the old prototype

     int pem_password_cb(char *buf, int size, int rwflag);

would have that called as

     int pem_password_cb(char *buf, int size, int rwflag, void *user_data);

with an unexpected extra parameter, which should work for usual C
calling conventions, but is not guaranteed to work.  (Given that until
recently the callback used to be called without even having a
parameter list declaration, it's probably a safe bet that no-one uses
them on systems with non-standard calling conventions now -- but still
relying on this would be an ugly thing to do.)

So, is backwards compatibility an important issue here and is it worth
this kind of evil hack, or should we just add the parameters?
Or rename everything to ..._ex and put a bunch of compatibility macros
in one file where it doesn't impurify all of the library?

> I also modified the ssl_ctx data structure to include a
> default_pem_password_callback_user_data field to supplement the
> default_pem_password_callback.

Makes sense.  You have

-void SSL_CTX_set_default_passwd_cb(SSL_CTX *ctx, pem_password_cb *);
+void SSL_CTX_set_default_passwd_cb(SSL_CTX *ctx, pem_password_cb *cb, void *u);

-- I'd prefer to split that into two functions.
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to