Per the other thread, the review for this has mostly been ignored. I have no wish to waste time banging my head against a wall, so I am:
-1 on the addition of the SSL code, r415639 et al, on the basis that: a) the API is undocumented in basic ways for key functions; e.g. it is unspecified to what purpose the certs, keys, and digest are passed in creating a "factory". b) providing an SSL client implementation without any way to verify that the server identity matches the presented cert is a security issue. (CVE names have been allocated for manifestations of this problem in other software) c) it is a leaky abstraction; the way errors are handled is by exposing the OpenSSL native error codes to the caller, which the caller can only interpret by knowing which SSL toolkit the code is linked against d) there seems to be no demonstration that the abstraction is even *possible* to implement on anything other than OpenSSL; the WinSock code is mostly commented out or stubbed. e) it has systematic error handling problems, returning '-1' as an apr_status_t value in many places. f) systematically breaks APR code style by doing argument validation g) the OpenSSL implementation plainly does not meet the documented (copy'n'pasted) API constraints in at least the handling of non-blocking I/O. and I am also -1 on the addition of the EVP code, r597209 et al, on the basis that: h) the undocumented/unspecified API (e.g. in formats of cert/key files, naming of ciphers) is another leaky abstraction requiring the caller to know it using OpenSSL underneath; and hence may as well code to OpenSSL directly i) apr_evp_factory_create represents unnecessarily bad API design; requiring a single entry point for a dual-purpose function which ignores half its arguments depending on a "purpose" switch. j) it has unnecessary dependencies on SSL_* interfaces in code purporting to do purely crypto k) it makes use of interfaces from unreleased versions of OpenSSL (which may or may not change before release; who knows) - for one of the two major modes of operation, no less. l) again, no demonstration that non-OpenSSL-based implementations are even possible, if an abstraction is the intent. ---ends--- I hope that is considered sufficient technical justification for the vetos. Note that most of the points above are not new and have been posted on this list six or twelve months back. I know that the veto is a horrible uncivilised blunt instrument. I am happy to see this code branched off somewhere where those interested in developing it further can do so. I am also happy to do the grunt work of reversion if the authors are still unwilling to resolve these issues and don't want to (or don't have time to) do that themselves. Regards, joe
