On Wed, Feb 20, 2002 at 10:18:55AM -0800, D. K. Smetters wrote: > 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,
Well, maybe you and me are actually the only ones who have used SSL_CTX_set_cert_verify_callback() so far. If so, I agree to the change -- but only under the condition that your patch updates demos/easy_tls accordingly :-) Also the SSL_CTX_set_cert_verify_callback() prototype should be updated to include the argument list of the actual callback. It should also be included in the 'app_verify_arg' declaration for struct ssl_ctx_st in ssl.h. If this is changed, the POD manpages in doc/ssl/ must be updated. > 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. The app_verify_callback argument certainly would be much nicer than having to use ex_data. (Everything is nicer than ex_data.) The only impediment to changing this is backwards compatibility. > 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. There aren't that many occurrences of 'verify_callback' in ssl/*.c, so approach #4 should be managable. But if noone else uses SSL_CTX_set_cert_verify_call() anyway, then changing the interface is fine with me. -- Bodo M�ller <[EMAIL PROTECTED]> PGP http://www.informatik.tu-darmstadt.de/TI/Mitarbeiter/moeller/0x36d2c658.html * TU Darmstadt, Theoretische Informatik, Alexanderstr. 10, D-64283 Darmstadt * Tel. +49-6151-16-6628, Fax +49-6151-16-6036 ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [EMAIL PROTECTED] Automated List Manager [EMAIL PROTECTED]
