You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by David Reid <da...@jetnet.co.uk> on 2007/03/28 12:40:30 UTC

future of ssl code

A while back there was a comment on this list that the ssl code should
be "ripped out" before the next release. There was no additional
information as to why, but that's OK.

Maybe it should be removed into a seperate module along the same lines
as apr-iconv? Additionally we should look at what really needs to go
into it. There are a few bits of code that I'd like to think about
moving into a library with apr's platform independance, but the bloat
that is apr-util no longer seems like the ideal landing site.

So, thoughts? Comments? Suggestions?

FWIW, this will have ramifications for some other code that I plan on
working on soon(ish).

david

Re: future of ssl code

Posted by Mads Toftum <ma...@toftum.dk>.
On Wed, Mar 28, 2007 at 11:40:30AM +0100, David Reid wrote:
> A while back there was a comment on this list that the ssl code should
> be "ripped out" before the next release. There was no additional
> information as to why, but that's OK.
> 
It would be nice to get the reason for that. Having SSL support in apr
would imho be a nice extension to the general networking bits. Wrapping
SSL-C in the same lib might be good as well.

> Maybe it should be removed into a seperate module along the same lines
> as apr-iconv? Additionally we should look at what really needs to go
> into it. There are a few bits of code that I'd like to think about
> moving into a library with apr's platform independance, but the bloat
> that is apr-util no longer seems like the ideal landing site.
> 
> So, thoughts? Comments? Suggestions?
> 
Big +1 from the peanut gallery. I'd like to see support for both crypto
and ssl bits - perhaps abstracting away some of the more unpleasant bits
of the openssl api.

vh

Mads Toftum
-- 
http://soulfood.dk

Re: future of ssl code

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Mar 28, 2007 at 05:19:17PM +0100, David Reid wrote:
> Joe Orton wrote:
> > - prototypes lack parameter names in many cases, hard to read
> 
> Docs all in place and give explanations right above the prototypes.

The right place for paramater names is in the C function declaration, 
not in a doxygen comment.  If you get the function declaration right, 
the doxygen @fn stuff is unnecessary.

> > - having a "factory" keyed by "private key, cert, digest type" is 
> > obscure. digest type of what?  Requiring "no pkey/cert == server" is 
> > also weird:  clients can have private keys and certs too.
> 
> Fixed.

Question remains: digest type of what?

> > - no cert verification if used for a client => SSL without 
> > the "security"; this needs API help to pass in the server name
> 
> Care to explain a bit more?

It's a fundamental part of SSL that clients must verify server identity 
in some way.  See e.g. RFC 2818.

> > - lots of argument validation (APR_EINVAL returns) which is non-standard 
> > for APR
> 
> I don't see why this is a huge issue.

It's not huge, but it's an issue.

> > - does nothing really to support non-blocking writes as API claims
> 
> Again, care to explain a bit more?

Well, using the defined API, how do I differentiate a write-which-failed 
from a write-which-blocked?  Partly this is just lack of error 
abstraction.

joe

Re: future of ssl code

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
David Reid wrote:
> 
>> - lots of argument validation (APR_EINVAL returns) which is non-standard 
>> for APR
> 
> I don't see why this is a huge issue.

Slows things down.  Input validation is the caller's responsibility.

Re: future of ssl code

Posted by David Reid <da...@jetnet.co.uk>.
Joe Orton wrote:
> On Wed, Mar 28, 2007 at 11:40:30AM +0100, David Reid wrote:
>> A while back there was a comment on this list that the ssl code should
>> be "ripped out" before the next release. There was no additional
>> information as to why, but that's OK.
> 
> Are you saying it *is* in a state ready for a 1.3 release, with the 
> API/ABI constraints that entails?  It went in as a (quote) "first dump", 
> and has seen little improvement since then.
> 
> Some API problems:
> - error handling is broken. lots of places return "-1" as an 
> apr_status_t.

Corrected.

> - apr_ssl_socket_raw_error() breaks the abstraction and exposes the ABI
> of the SSL implementation

Part of a larger issue with the error handling.

> - prototypes lack parameter names in many cases, hard to read

Docs all in place and give explanations right above the prototypes.

> - attempting to wrap every apr_socket_* function seems futile; why not 
> just expose the apr_socket_t * pointer and let users deal with it directly?

To allow for different implementations that need to not use the
underlying sockets (assuming such things exist).

> - having a "factory" keyed by "private key, cert, digest type" is 
> obscure. digest type of what?  Requiring "no pkey/cert == server" is 
> also weird:  clients can have private keys and certs too.

Fixed.

> - no cert verification if used for a client => SSL without 
> the "security"; this needs API help to pass in the server name

Care to explain a bit more?

> 
> Some implementation problems:
> - is the OpenSSL code intended to be Unix-specific?  It assumes 
> apr_os_sock_t is an fd I guess, by passing it to SSL_set_fd

True. Have to look at how we fix this.

> - lots of argument validation (APR_EINVAL returns) which is non-standard 
> for APR

I don't see why this is a huge issue.

> - does nothing really to support non-blocking writes as API claims

Again, care to explain a bit more?

david

Re: future of ssl code

Posted by David Reid <da...@jetnet.co.uk>.
Joe Orton wrote:
> On Wed, Mar 28, 2007 at 11:40:30AM +0100, David Reid wrote:
>> A while back there was a comment on this list that the ssl code should
>> be "ripped out" before the next release. There was no additional
>> information as to why, but that's OK.
> 
> Are you saying it *is* in a state ready for a 1.3 release, with the 
> API/ABI constraints that entails?  It went in as a (quote) "first dump", 
> and has seen little improvement since then.

I wasn't commenting one way or the other.

> 
> Some API problems:
> - error handling is broken. lots of places return "-1" as an 
> apr_status_t.
> - apr_ssl_socket_raw_error() breaks the abstraction and exposes the ABI
> of the SSL implementation
> - prototypes lack parameter names in many cases, hard to read
> - attempting to wrap every apr_socket_* function seems futile; why not 
> just expose the apr_socket_t * pointer and let users deal with it directly?
> - having a "factory" keyed by "private key, cert, digest type" is 
> obscure. digest type of what?  Requiring "no pkey/cert == server" is 
> also weird:  clients can have private keys and certs too.
> - no cert verification if used for a client => SSL without 
> the "security"; this needs API help to pass in the server name

OK, I'll work through these. It was the lack of any form of feedback and
the blanket "it should be yanked" that had me stumped. lets face it,
there hasn't been much discussion of the code.

> Some implementation problems:
> - is the OpenSSL code intended to be Unix-specific?  It assumes 
> apr_os_sock_t is an fd I guess, by passing it to SSL_set_fd
> - lots of argument validation (APR_EINVAL returns) which is non-standard 
> for APR
> - does nothing really to support non-blocking writes as API claims

OK, good points and something I can at least work towards.

I'm more than willing to try and fix these, but when faced with just
negative comments it was hard to know what people felt needed fixed.

Thanks for clarifying Joe.

> 
> joe
> 
> !DSPAM:16,460a56311814816939239!
> 
> 


Re: future of ssl code

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Mar 28, 2007 at 11:40:30AM +0100, David Reid wrote:
> A while back there was a comment on this list that the ssl code should
> be "ripped out" before the next release. There was no additional
> information as to why, but that's OK.

Are you saying it *is* in a state ready for a 1.3 release, with the 
API/ABI constraints that entails?  It went in as a (quote) "first dump", 
and has seen little improvement since then.

Some API problems:
- error handling is broken. lots of places return "-1" as an 
apr_status_t.
- apr_ssl_socket_raw_error() breaks the abstraction and exposes the ABI
of the SSL implementation
- prototypes lack parameter names in many cases, hard to read
- attempting to wrap every apr_socket_* function seems futile; why not 
just expose the apr_socket_t * pointer and let users deal with it directly?
- having a "factory" keyed by "private key, cert, digest type" is 
obscure. digest type of what?  Requiring "no pkey/cert == server" is 
also weird:  clients can have private keys and certs too.
- no cert verification if used for a client => SSL without 
the "security"; this needs API help to pass in the server name

Some implementation problems:
- is the OpenSSL code intended to be Unix-specific?  It assumes 
apr_os_sock_t is an fd I guess, by passing it to SSL_set_fd
- lots of argument validation (APR_EINVAL returns) which is non-standard 
for APR
- does nothing really to support non-blocking writes as API claims

joe

Re: future of ssl code

Posted by David Reid <da...@jetnet.co.uk>.
Joe Orton wrote:
> On Wed, Mar 28, 2007 at 10:09:43AM -0500, William Rowe wrote:
>>> Given Joe's stance (which I'm taking as a veto) I think removing it and
>>> starting a seperate "module" within apr's repo would make the most sense
>>> and should remove the veto from 1.3 - making everyone happy.
>> I definately don't want to see this handled piecemeal.  Please leave it in
>> place, and Joe please reconsider your veto.
> 
> I have not vetoed anything.  In principle I have no objection to putting 
> the SSL code in apr-util.  As I said originally: I would prefer to see 
> the code develop in a branch until it reaches a suitable state for 
> inclusion in the trunk/a release.

I assume my suggestion to remove the code into a separate module would
also satisfy you?

Re: future of ssl code

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> On Wed, Mar 28, 2007 at 10:09:43AM -0500, William Rowe wrote:
>>> Given Joe's stance (which I'm taking as a veto) I think removing it and
>>> starting a seperate "module" within apr's repo would make the most sense
>>> and should remove the veto from 1.3 - making everyone happy.
>> I definately don't want to see this handled piecemeal.  Please leave it in
>> place, and Joe please reconsider your veto.
> 
> I have not vetoed anything.  In principle I have no objection to putting 
> the SSL code in apr-util.  As I said originally: I would prefer to see 
> the code develop in a branch until it reaches a suitable state for 
> inclusion in the trunk/a release.

If 1.3 looked immanent, I'd agree.  Since we have a bit of time, it seems
we could leave development on trunk for a time till we are satisfied that
progress is being made, or shift it to a branch if there's a strong desire
to ship out a 1.3.0.  Sensible?

Re: future of ssl code

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Mar 28, 2007 at 10:09:43AM -0500, William Rowe wrote:
> > Given Joe's stance (which I'm taking as a veto) I think removing it and
> > starting a seperate "module" within apr's repo would make the most sense
> > and should remove the veto from 1.3 - making everyone happy.
> 
> I definately don't want to see this handled piecemeal.  Please leave it in
> place, and Joe please reconsider your veto.

I have not vetoed anything.  In principle I have no objection to putting 
the SSL code in apr-util.  As I said originally: I would prefer to see 
the code develop in a branch until it reaches a suitable state for 
inclusion in the trunk/a release.

joe

Re: future of ssl code

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
David Reid wrote:
> William A. Rowe, Jr. wrote:
>>
>> Does SSL block any other change in 1.3?
> 
> Not as far as I'm aware.

One I can think of - the threadpool API, but I'm willing to wait for
a richer feature set before calling 1.3 baked.

I'm pretty sure httpd would like to pick up 1.3.x by the time they are
ready to ship httpd-2.4, but that's a little ways off yet and still.

>> It's a bigger problem than the openssl bindings.
> 
> I realise that, but several people have now mentioned other aspects of
> openssl that they'd like to see supported. 

I really don't want the entire kitchen sink.  If we can keep this to the
essential nuts and bolts of a TLS conversation, and extracting information
about the client and server credentials, then we won't end up with an
extension of openssl that CAN'T be supported with cryptocme, gnutls or
other equally valid ssl/tls implementations.

>> A big comment.  APR and APR-util current include non-FIPS implementations
>> of FIPS-140 specified algorithms.  OpenSSL built with FIPS enabled has
>> proper implementations.  So please don't begin yanking openssl.
>>
>> In fact I need to toggle off those apr-implementations, and stub in
>> openssl's implementations in the presence of some --enable-fips flag.
> 
> I don't see how this has direct implications on what happens with the
> ssl code, but I'm willing for you to correct my misunderstaning :-)
>
>> Apache+APR+OpenSSL won't 'become FIPS certified' (in fact, only a small
>> core of OpenSSL is fips validated).  But as things stand, it violates
>> the FIPS standard and security policy document.  I hope to change that.

We both need to consume openssl, for different elements.  In fact, I'm
thinking of refactoring all the pieces that are subject to FIPS-140
explicit implementations (which will just be stubbed out to openssl),
and these include every algorithm we have that is part of the openssl
project's fips_canister, such as sha1 hashes.

>> So we can all agree that apr-util is bloated, but please leave things
>> as-is for 1.3?
> 
> Given Joe's stance (which I'm taking as a veto) I think removing it and
> starting a seperate "module" within apr's repo would make the most sense
> and should remove the veto from 1.3 - making everyone happy.

I definately don't want to see this handled piecemeal.  Please leave it in
place, and Joe please reconsider your veto.

The right way to handle it is to agree to agree on exactly how we will
change the ldap/dbm/dbd/xml/xlate/ssl library bindings to be dynamic
or modular.

Until we agree on that, I DON'T want to special-case ssl :(  It's wasted
effort.  Let's focus on addressing the real issues folks have identified
in the apr_ssl_* implementation?

Bill


Re: future of ssl code

Posted by David Reid <da...@jetnet.co.uk>.
William A. Rowe, Jr. wrote:
> David Reid wrote:
>> A while back there was a comment on this list that the ssl code should
>> be "ripped out" before the next release. There was no additional
>> information as to why, but that's OK.
> 
> Actually, I don't know that it is OK.
> 
> 1.2.9 on the current-stable branch should be released shortly.  Independent
> of the 1.3 changes for SSL.
> 
> Does SSL block any other change in 1.3?

Not as far as I'm aware.

> 
>> Maybe it should be removed into a seperate module along the same lines
>> as apr-iconv? Additionally we should look at what really needs to go
>> into it. There are a few bits of code that I'd like to think about
>> moving into a library with apr's platform independance, but the bloat
>> that is apr-util no longer seems like the ideal landing site.
> 
> In fairness, the bloat which is apr-util is agreeably just-bloat.  Don't
> worry about it, there is a much greater urgency to remove 4 of 6 db libs
> which an application won't consume, or ldap for a non-ldap application.
> And ssl, if ssl is not used by an application.
> 
> It's a bigger problem than the openssl bindings.

I realise that, but several people have now mentioned other aspects of
openssl that they'd like to see supported. Other libs have been
mentioned as well. I think all the things that people have mentioned are
valid and would be useful additions, but I'm not sure I really want to
simply lump them all into apr-util. Having this as a standalone seperate
module probably does make more sense in the long term as I can envision
having both an apr-ssl and apr-crypto library. Moving these to a
seperate repo within apr would seem like the best plan as that way they
can be developed on their own timeline, with features coming and going
as required for the development process.

> 
>> So, thoughts? Comments? Suggestions?
>>
>> FWIW, this will have ramifications for some other code that I plan on
>> working on soon(ish).
> 
> A big comment.  APR and APR-util current include non-FIPS implementations
> of FIPS-140 specified algorithms.  OpenSSL built with FIPS enabled has
> proper implementations.  So please don't begin yanking openssl.
> 
> In fact I need to toggle off those apr-implementations, and stub in
> openssl's implementations in the presence of some --enable-fips flag.

I don't see how this has direct implications on what happens with the
ssl code, but I'm willing for you to correct my misunderstaning :-)

> 
> Apache+APR+OpenSSL won't 'become FIPS certified' (in fact, only a small
> core of OpenSSL is fips validated).  But as things stand, it violates
> the FIPS standard and security policy document.  I hope to change that.
> 
> So we can all agree that apr-util is bloated, but please leave things
> as-is for 1.3?

Given Joe's stance (which I'm taking as a veto) I think removing it and
starting a seperate "module" within apr's repo would make the most sense
and should remove the veto from 1.3 - making everyone happy.

AFAIU we don't need anything special for the move, but the PMC will
likely need to have a vote or sacrifice a goat I guess?

Re: future of ssl code

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
David Reid wrote:
> A while back there was a comment on this list that the ssl code should
> be "ripped out" before the next release. There was no additional
> information as to why, but that's OK.

Actually, I don't know that it is OK.

1.2.9 on the current-stable branch should be released shortly.  Independent
of the 1.3 changes for SSL.

Does SSL block any other change in 1.3?

> Maybe it should be removed into a seperate module along the same lines
> as apr-iconv? Additionally we should look at what really needs to go
> into it. There are a few bits of code that I'd like to think about
> moving into a library with apr's platform independance, but the bloat
> that is apr-util no longer seems like the ideal landing site.

In fairness, the bloat which is apr-util is agreeably just-bloat.  Don't
worry about it, there is a much greater urgency to remove 4 of 6 db libs
which an application won't consume, or ldap for a non-ldap application.
And ssl, if ssl is not used by an application.

It's a bigger problem than the openssl bindings.

> So, thoughts? Comments? Suggestions?
> 
> FWIW, this will have ramifications for some other code that I plan on
> working on soon(ish).

A big comment.  APR and APR-util current include non-FIPS implementations
of FIPS-140 specified algorithms.  OpenSSL built with FIPS enabled has
proper implementations.  So please don't begin yanking openssl.

In fact I need to toggle off those apr-implementations, and stub in
openssl's implementations in the presence of some --enable-fips flag.

Apache+APR+OpenSSL won't 'become FIPS certified' (in fact, only a small
core of OpenSSL is fips validated).  But as things stand, it violates
the FIPS standard and security policy document.  I hope to change that.

So we can all agree that apr-util is bloated, but please leave things
as-is for 1.3?