apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Reid <da...@jetnet.co.uk>
Subject Re: veto on addition of ssl, evp code
Date Mon, 14 Apr 2008 20:46:03 GMT
Joe Orton wrote:
> 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.

Joe,

I've repeatedly said I'm happy to have the code removed. I've offered to
move it into a branch.

Do with the code as you will. No objections from me.

david

Mime
View raw message