You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Simo Sorce <si...@redhat.com> on 2014/08/06 00:24:29 UTC

[PATCH] Support RFC5929 - Channel Bindings for TLS

Hello dev,

I have been working for a little while on making it possible to use
channel bindings within an Apache server.
In order to do that some support to extract information form the TLS
layer is necessary in the server.

The attached patch adds a new function call that modules can call in
order to obtain the channel binding data defined in RFC 5929.

I've tested that the patch works (at least for tls-server-end-point)
with the following:
- Apache 2.4.10 + patch
- mod_auth_gssapi [1] with support for the new function
- gss-ntlmssp [2]
- Internet Explorer on a modern Windows machine, performing
SPNEGO/GSSAPI/NTLMSSP auth + channel bindings

Any feedback is welcome,
Simo.


[1] https://github.com/modauthgssapi/mod_auth_gssapi
[2] https://fedorahosted.org/gss-ntlmssp/

-- 
Simo Sorce * Red Hat, Inc * New York

Re: [PATCH] Support RFC5929 - Channel Bindings for TLS

Posted by Simo Sorce <si...@redhat.com>.
On Wed, 2014-08-27 at 17:15 +0100, Joe Orton wrote:
> On Wed, Aug 27, 2014 at 10:05:40AM -0400, Simo Sorce wrote:
> > Yes the spec is "strange" wrt digest, which is why the code looks
> > strange too, here is the quote from RFC 5929 (4.1):
> 
> Great, thanks.  It took me two commits due to PEBKAC issues...
> 
> http://svn.apache.org/viewvc?view=revision&revision=r1620926
> http://svn.apache.org/viewvc?view=revision&revision=r1620927
> 
> I made a couple of style & docs tweaks and adjusted the types, partly 
> because of compiler warnings:
> 
> - use apr_status_t not int for APR_* error codes in return value
> - switched to "unsigned char *" for the buffer type throughout
> - used unsigned int for the size parameter to X509_digest()
> 
> Hope these are OK, let me know if not!

Looks good to me,
Thanks a lot!

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


Re: [PATCH] Support RFC5929 - Channel Bindings for TLS

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Aug 27, 2014 at 10:05:40AM -0400, Simo Sorce wrote:
> Yes the spec is "strange" wrt digest, which is why the code looks
> strange too, here is the quote from RFC 5929 (4.1):

Great, thanks.  It took me two commits due to PEBKAC issues...

http://svn.apache.org/viewvc?view=revision&revision=r1620926
http://svn.apache.org/viewvc?view=revision&revision=r1620927

I made a couple of style & docs tweaks and adjusted the types, partly 
because of compiler warnings:

- use apr_status_t not int for APR_* error codes in return value
- switched to "unsigned char *" for the buffer type throughout
- used unsigned int for the size parameter to X509_digest()

Hope these are OK, let me know if not!

Regards, Joe

Re: [PATCH] Support RFC5929 - Channel Bindings for TLS

Posted by Simo Sorce <si...@redhat.com>.
On Wed, 2014-08-27 at 14:57 +0100, Joe Orton wrote:
> Hi Simo - thanks for sending this in!  I'm fine with adding this.  One 
> question - this part looks a bit magic:
> 
> On Tue, Aug 05, 2014 at 06:24:29PM -0400, Simo Sorce wrote:
> > +    } else if (x != NULL) {
> > +        const EVP_MD *md;
> > +
> > +        md = EVP_get_digestbynid(OBJ_obj2nid(x->sig_alg->algorithm));
> > +        if (md == NULL ||
> > +            md == EVP_md5() ||
> > +            md == EVP_sha1()) {
> > +            md = EVP_sha256();
> > +        }
> > +        if (!X509_digest(x, md, cb, &l)) {
> > +            return APR_EGENERAL;
> > +        }
> 
> I get "pick a better digest" - is this specified in the RFC?

Yes the spec is "strange" wrt digest, which is why the code looks
strange too, here is the quote from RFC 5929 (4.1):

   o  if the certificate's signatureAlgorithm uses a single hash
      function, and that hash function is either MD5 [RFC1321] or SHA-1
      [RFC3174], then use SHA-256 [FIPS-180-3];

   o  if the certificate's signatureAlgorithm uses a single hash
      function and that hash function neither MD5 nor SHA-1, then use
      the hash function associated with the certificate's
      signatureAlgorithm;

The code implements this recommendation literally.

HTH,
Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


Re: [PATCH] Support RFC5929 - Channel Bindings for TLS

Posted by Joe Orton <jo...@redhat.com>.
Hi Simo - thanks for sending this in!  I'm fine with adding this.  One 
question - this part looks a bit magic:

On Tue, Aug 05, 2014 at 06:24:29PM -0400, Simo Sorce wrote:
> +    } else if (x != NULL) {
> +        const EVP_MD *md;
> +
> +        md = EVP_get_digestbynid(OBJ_obj2nid(x->sig_alg->algorithm));
> +        if (md == NULL ||
> +            md == EVP_md5() ||
> +            md == EVP_sha1()) {
> +            md = EVP_sha256();
> +        }
> +        if (!X509_digest(x, md, cb, &l)) {
> +            return APR_EGENERAL;
> +        }

I get "pick a better digest" - is this specified in the RFC?

Regaards, Joe

Re: [PATCH] Support RFC5929 - Channel Bindings for TLS

Posted by Daniel Kahn Gillmor <dk...@fifthhorseman.net>.
On 08/05/2014 09:06 PM, Simo Sorce wrote:
> Yeah I know it is broken, does it mean you want to have it disabled and
> return an error if requested until a fixed openssl library/call is
> available ?

Not only did i not have a concrete proposal, I don't have any particular
say in the matter -- i'm not on the apache team :)  I just wanted to
make sure that adopting/incorporating something like this is done with
knowledge of the state of the art.

That said, i do like the idea that such a mechanism would be able to
indicate to the user whether a given connection is using a tls-unique
based on the revised session hash, or if it is using the old (broken)
mechanism.  Whether that's best handled as an error response or some
other approach, i'm not sure.

Regards,

	--dkg




Re: [PATCH] Support RFC5929 - Channel Bindings for TLS

Posted by Simo Sorce <si...@redhat.com>.
On Tue, 2014-08-05 at 19:13 -0400, Daniel Kahn Gillmor wrote:
> On 08/05/2014 06:24 PM, Simo Sorce wrote:
> 
> > I have been working for a little while on making it possible to use
> > channel bindings within an Apache server.
> > In order to do that some support to extract information form the TLS
> > layer is necessary in the server.
> 
> This is great idea, but be aware that tls_unique is fundamentally broken
> in its current form:
> 
>   http://secure-resumption.com/
> 
> This will be fixed with an update to TLS, which was recently approved
> for adoption by the TLS WG:
> 
>  https://tools.ietf.org/html/draft-bhargavan-tls-session-hash-01
> 
> but i don't think it's implemented in any of the major toolkits yet.

Yeah I know it is broken, does it mean you want to have it disabled and
return an error if requested until a fixed openssl library/call is
available ?

I care more for tls-server-end-point for now anyway as that's what
Microsoft browsers use.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


Re: [PATCH] Support RFC5929 - Channel Bindings for TLS

Posted by Daniel Kahn Gillmor <dk...@fifthhorseman.net>.
On 08/05/2014 06:24 PM, Simo Sorce wrote:

> I have been working for a little while on making it possible to use
> channel bindings within an Apache server.
> In order to do that some support to extract information form the TLS
> layer is necessary in the server.

This is great idea, but be aware that tls_unique is fundamentally broken
in its current form:

  http://secure-resumption.com/

This will be fixed with an update to TLS, which was recently approved
for adoption by the TLS WG:

 https://tools.ietf.org/html/draft-bhargavan-tls-session-hash-01

but i don't think it's implemented in any of the major toolkits yet.

	--dkg