One comment made when I proposed the patch was that this functionality
is currently almost entirely unused. Therefore changing the expected
prototype is not likely to affect that many people, many of whom may
actually prefer having the argument used. It also cleans up a notable
inconsistency in the code (having placeholders for the unused argument),
which might be useful to do on a revision boundary.

It is true you can get to the SSL structure from the ex_data in the
X509_STORE_CTX (thanks very much for pointing that out, I had missed
it entirely). However, a) it's not easy to find (though I may just be
blinder than most :-), and b) while it's available through the ex_data
field, I don't think it's presence there is documented, and I as a user
of the code wouldn't be sure that it's presence there was meant to be
used by callbacks or something I could really rely on from version
to version of OpenSSL. In contrast, the argument to app_verify_callback
is easier to find, documented (though with the caveat that it doesn't
work), and cleaner.

It seems that there are 4 approaches to this patch:
1) punt, and leave things in their current inconsistent state
2) skip the patch to protect backwards compatibility in the function
    prototype; but also:
        a) remove the argument entirely from the code and documentation so
                as to skip having this discussion again
        b) document/comment the placement of the SSL pointer in the
                ex_data so that it's clear that it's a stable feature of
                the code and can be relied on by app_verify functions,
                and not there just for the current convenience of some
                unrelated piece of the library
3) add the patch, on the theory that at most a small number of people will
        be bitten by the change in callback prototype, and that it's worth
        it for both usability and modularity
4) keep the current prototype, and add yet a 3rd callback function that
        actually takes an argument.

Personally, I'd go for either 3 or 2+a+b. 4 might be nice, but would take
more time & planning.

--Diana

Dr S N Henson wrote:
> Bodo Moeller wrote:
> 
>>On Sat, Feb 16, 2002 at 11:16:23AM +0100, Richard Levitte - VMS Whacker wrote:
>>
>>
>>>I see no problem adding this patch.  Queued.
>>>
>>The problem is that the application callback prototype is incompatibly
>>changed.  Otherwise I would have added the argument instead of simply
>>adding comments pointing out the inconsistency.  To maintain backwards
>>compatibilty, it looks like we need a new function for registering
>>callback functions expecting this additional argument.
>>
>>
> 
> If its likely to be a problem then I suggest we don't bother with this
> patch at all. Its not needed because its possible to access application
> specific data in the verify callback via the relevant SSL structure as I
> indicated in another message.
> 
> Steve.
> 


______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to