Subject: [dane] DANE for OpenSSL

From: Jakob Schlyter <[email-masked]>
Date: Wed, 15 May 2013 15:43:01 +0200

Andy Polyakov has committed initial support for DANE in OpenSSL - please see http://rt.openssl.org/Ticket/Display.html?id=3003 for more information.

	jakob

---

From: Richard Lamb <[email-masked]>
Date: Wed, 15 May 2013 08:26:09 -0700

Woo hoo.. now if I can use MSFT Detours (or something free) to make it part of windows crypto api... ;-) 

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of Jakob Schlyter
Sent: Wednesday, May 15, 2013 6:43 AM
To: dane WG list
Subject: [dane] DANE for OpenSSL

	jakob

_______________________________________________
dane mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/dane

---

From: "Marco Davids (SIDN)" <[email-masked]>
Date: Wed, 15 May 2013 16:33:20 +0100

> Woo hoo.. now if I can use MSFT Detours (or something free) to make it part of windows crypto api... ;-) 

Has anyone had time to play with GnuTLS already?

http://gnutls.org/manual/html_node/DANE-API.html

Experiences, thoughts?

--
Marco

---

From: Viktor Dukhovni <[email-masked]>
Date: Wed, 15 May 2013 15:55:17 +0000

On Wed, May 15, 2013 at 03:43:01PM +0200, Jakob Schlyter wrote:

> Andy Polyakov has committed initial support for DANE in OpenSSL
> - please see http://rt.openssl.org/Ticket/Display.html?id=3003 for
> more information.

Note, this "initial support", does not yet perform any verification
based on TLSA records, it just adds a convenience TLSA RR lookup
function that is conditional on libunbound.  The application will
need to call SSL_get_tlsa_record_byname() and then provide the output
to the OpenSSL library via a control operation before the handshake. 

There are complications because EE certificate usage (1/3) TLSA
records are different from TA (0/2) TLSA records.  The former (or
least 3 in any case) require no name checks, and the latter do.

OpenSSL has no means to communicate the distinction, the result of
the verification engine is either "verified" (as in trust chain
verified) or not.  Since with DANE it is not enough to know whether
the chain is trusted, one needs to know whether name checks are
still required,  applications will have to also communicate the
the names to accept (plural per draft-ietf-dane-srv) to the OpenSSL
library.

Further complications can arise with session reuse, depending on
how client applications associate cached sessions with a particular
peer.

There is still a bunch of work before this is usable.

This will by the way fail to compile if one defines OPENSSL_NO_LIBUNBOUND

    $ unifdef -DOPENSSL_NO_LIBUNBOUND ssl/dnssec.c | head -20
    #include <openssl/opensslconf.h>

    #include <string.h>
    #include <netdb.h>
    #include <openssl/bio.h>
    #include <openssl/dso.h>

    /*
     * Output is array packed as [len][data][len][data][0]
     */
    unsigned char *SSL_get_tlsa_record_byname (const char *name,int port,int
    type)
    {
	    unsigned char *ret=NULL;
	    char *query=NULL;
	    size_t qlen;

	    if (ctx == NULL) return NULL;

	    qlen = 7+5+strlen(name)+1;

because "ctx" is not declared in that case, the declartion requires
unbound.h:

    #include <unbound.h>

    static struct ub_ctx *ctx = NULL;

-- 
	Viktor.

---

From: Viktor Dukhovni <[email-masked]>
Date: Wed, 15 May 2013 18:20:57 +0000

> to the OpenSSL library via a control operation before the handshake. 

A few more comments:

    0.	The TLSA lookup function does not check the "bogus" field, which is
	documented as possibly set together with "secure", indicating a bogus
	DNS reply (unbound still returns the data it seems) and lets the caller
	decide.  So the new TLSA lookup function is not safe.

    1.  The introduction of a dependency on libunbound is I think a mistake,
	applications should obtain TLSA RRs via whatever library they see fit.
	It is sufficient for OpenSSL to specify a suitable encoding of the
	TLSA RRs to be passed to the verification routine.

    2.  The packed encoding chosen is rather unnatural.  A data structure
	would have been better than a packed array of lenghts and data buffers.

	    struct SSL_TLSA_DATA {
		size_t rrcount;
		struct {
		    size_t len;
		    unsigned char *data;
		} rrdata[1];
	    }

I don't think Andy Polykov reads this list.  I'll forward him my
comments under separate cover.

-- 
	Viktor.

---

From: Andy Polyakov <[email-masked]>
Date: Wed, 15 May 2013 21:07:42 +0200

>> to the OpenSSL library via a control operation before the handshake.

I've replied to first message and it went to moderator for approval.

> 	decide.  So the new TLSA lookup function is not safe.

OK.

> 	TLSA RRs to be passed to the verification routine.

Goal is to have libssl check config file for [libunbound] section and 
link to libunbound only if there is such section. In other words 
run-time link to libunbound will be optional.

> 	would have been better than a packed array of lenghts and data buffers.

Rationale behind the unnatural choice is to have possibility to generate 
record from scripting languages without having to create glue code.

> comments under separate cover.

I've now subscribed to the list.

---

From: Andy Polyakov <[email-masked]>
Date: Wed, 15 May 2013 21:19:39 +0200

>>     decide.  So the new TLSA lookup function is not safe.

Or? Manual page says if both are zero, then no security for domain. It 
says nothing about both being set to 1. And example at unbound.net 
suggests that they can't be set together:

	if(result->secure)
		printf("Result is secure\n");
	else if(result->bogus)

---

From: Viktor Dukhovni <[email-masked]>
Date: Wed, 15 May 2013 21:30:32 +0000

On Wed, May 15, 2013 at 09:19:39PM +0200, Andy Polyakov wrote:

> Or? Manual page says if both are zero, then no security for domain.
> It says nothing about both being set to 1. And example at
> unbound.net suggests that they can't be set together:
> 
> 	if(result->secure)

Yes, it seems I did not find a sufficiently clear reference in my
quick search for ub_resolve() documentation.  The "secure" bit
precludes bogus.

When you're implementing the verification code for TLSA RRs, we
should talk.  I have a working implementation via the verify
callback, and have spent a bunch of time thinking through some of
the more subtle issues.

You can look at tls_verify_certificate_callback() in:

    https://github.com/vdukhovni/postfix/blob/20130405-nonprod/postfix/src/tls/tls_verify.c

-- 
	Viktor.

---

From: Andy Polyakov <[email-masked]>
Date: Wed, 15 May 2013 20:47:20 +0200

> function that is conditional on libunbound.

Oops! Git noob mistake. 
http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=e815d72b1f489c2c38adf3eee87c02e1c5dd8f3c

> There is still a bunch of work before this is usable.

Yes, please get involved.

>     $ unifdef -DOPENSSL_NO_LIBUNBOUND ssl/dnssec.c | head -20

http://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=ddf918673d2d163fc0a6a6c9774b05dd1efb9857

---

From: Viktor Dukhovni <[email-masked]>
Date: Thu, 16 May 2013 07:14:08 +0000

> >function that is conditional on libunbound.

Got the new code. Thanks.

> Yes, please get involved.

There are a number of immediate issues to address, some quite
significant.  I encourage you to take a look at the corresponding
Postfix verification callback.  I contributed it to Postfix under
the Postfix license, but you may borrow from it under any license
that meets your needs.

The list below is not necessarily comprehensive.

    - At depth 0, dane_verify_callback() unconditionally returns "0"
      for certificate usages 0, 1.  This aborts the handshake.  The
      intent is probably to return "ok", not "0".  But if you're
      raising an error for lack of a suitable TLSA "witness" certificate,
      you're obligated to call the application callback to vet the error.

        if (depth==0) {
                switch (s->tlsa_witness&0xff) {         /* witnessed usage */
                case 0: /* CA constraint */
                        if (s->tlsa_witness<0 && ctx->error==X509_V_OK)
                                ctx->error = X509_V_ERR_INVALID_CA;
                        return 0;
                case 1: /* service certificate constraint */
                        if (tlsa_ret!=0 && ctx->error==X509_V_OK)
                                ctx->error = X509_V_ERR_CERT_UNTRUSTED;
                        return 0;
		...

    - At any depth, dane_verify_callback() must clear any witness for most 
      errors (parent-child signature errors, expiration, ...) that are found
      at an equal or lower depth.  The only errors that TLSA records can
      preempt are those relating lack of a trusted root.

      Otherwise, you may end up calling a chain valid, for containing a CA
      that did not actually sign the next certificate, or when that
      certificate expired.

    - There is no code to handle "IN TLSA 2 1 0", where a public key
      is given for a CA that may not be present in the peer's chain,
      but may the signer of the top certificate in the chain.

    - The application cannot distinguish between a usage 3 match,
      in which name checks don't apply, and other usages where name
      checks are still required.  Both cases given X509_V_OK, but this
      is not sufficient.

      The Postfix implementation never sets X509_V_OK with EE certs,
      instead applications can check that condition against the leaf
      certificate when the handshake completes.  This way (CA) verified
      certificates always need name checks, and if EE TLSA records
      are present the application calls suitable code to check these
      at the end.

    - The "IN TLSA x 0 0" comparisons are done via SHA-1 hashes, these
      are IMHO too weak in this context.  The comparison should be bit
      for bit, or use at least SHA-256.

    - When the TLSA RRset is invalid, instead of returning an error
      when the user is setting SSL->tlsa_record to -1, the handshake
      proceeds only to be aborted unconditionally.

        if (tlsa_record == (void*)-1) {
                ctx->error = X509_V_ERR_INVALID_CA;     /* temporary code?  */
                return 0;
        }

      This is needlessly late, and once again does not give the application
      callback a chance to continue.  Which violates the application callback
      contract (it must be able to choose to continue despite all errors).

      The SSL_CTRL_PULL_TLSA_RECORD control should return an error
      when TLSA record lookups fail, and applications should decide
      what to do at that point, rather than in mid handshake.

      Of course with SSL_CTRL_SET_TLSA_RECORD, the application already knows
      when the value is ((void *)-1) and should again decide at that time.

      So I would say that this value should never be set at handshake time,
      it should be valid or NULL.  Attempts to set it to "-1" should fail.

    - When allocating an SSL pointer via SSL_new() the ssl->tlsa_record element
      must be initialized to NULL.  The memset( , 0, ) is not sufficient, the
      binary representation of a NULL pointer is implementation dependent and
      may not be all zero.

    - witness_usage is an unused variable.

-- 
	Viktor.

---

From: Viktor Dukhovni <[email-masked]>
Date: Thu, 16 May 2013 15:17:26 +0000

>       that did not actually sign the next certificate, or when that

I think the larger issue is model mismatch between the new dane
callback which makes a global decision based observation of the
entire chain, and the legacy application callback it consults,
which previously only made decisions one error at a time.

Specifically, it is not reasonable to propagate errors to the
application callback that you'll later want to "retract" because
you finally found the trusted CA.  With applications that provide
TLSA records via either the PULL or the SET controls, it is likely
necessary to *document* and implement a new contract with the
application verification callback.

    - If none of the TLSA records are usable (unknown usage, selector
      or matching type or perhaps invalid length) the dane callback
      sets the verification status to a new value say:

	  ctx->error = X509_V_ERR_UNUSABLE_TLSA_RRS 

      and invokes the application callback (cert/depth TBD).  The
      application can decide to clear this error and revert to
      normal PKIX processing, or to leave it set and consider the
      chain unverified.

    - Otherwise, the dane callback will only call the application
      callback (at least once) when the dane callback is itself
      called at depth 0.

      On successful verification of the chain via trust anchor (usage 0
      or 2), which means that one the following is true:

	  1. A TA RR was matched at some higher depth.  The trust depth
	     (as with PKIX) is set one lower indicating the first
	     certificate in the chain that is issued by the TA,
	     provided no errors other than trust errors were found
	     at or below the trust depth.

	  2. An explicit public key whose certificate is not in the chain
	     signed the topmost certificate, which is not expired.  The
	     top depth becomes the trust depth, unless errors other than
	     trust errors are found at any depth in the chain.

	  3. The depth 0 cert is a self-signed TLSA TA with acceptable
	     usage bits to be both a CA and an SSL server and no
	     errors other trust errors have yet been found at depth 0.

      The certificate verification status will be set to X509_V_OK
      and the application callback will be called with ok=1, depth
      set to 0, and the current certificate set to the depth zero
      certificate.  The application callback is responsible for
      performing peer name checks on the leaf certificate setting the
      store error if name checks fail.

      If there is an EE TLSA record with certificate usage 3, and
      the only error is that the chain is not trusted, the store
      error must be set by the dane callback to X509_V_ERR_CERT_UNTRUSTED,
      and the application callback must be called with depth = 0
      and cert equal to the depth 0 certificate.  The application
      callback should be able to call a new documented function to
      perform EE verification of the leaf certificate.

      Otherwise, the application callback will be called with the
      "error certificate" (see the Postfix code) and corresponding
      error state and depth.  This allows the application can do
      proper error reporting and decide whether to complete the
      handshake.

Modifications to the above may be necessary, or some other approach
entirely may be better, but the current approach is I think
unreasonable.  The application callback is blind to the DANE checks
and is being called with misleading error indications that may be
retracted, and is not called with ok=1 at depth 0 when DANE decides
that the chain is trusted after all.  I don't think it is possible
to implement sensible application callbacks within the current
model.

-- 
	Viktor.

---

From: Warren Kumari <[email-masked]>
Date: Thu, 16 May 2013 11:25:21 -0400

> I've replied to first message and it went to moderator for approval.

Yes, yes you did -- We did get:
----
As list administrator, your authorization is requested for the
following mailing list posting:

   List:    [email-masked]
   From:    [email-masked]
   Subject: Re: [dane] DANE for OpenSSL
   Reason:  Post by non-member to a members-only list

At your convenience, visit:
   https://www.ietf.org/mailman/admindb/dane
to approve or deny the request.

-----

I went to go approve the message and got: 
-----
dane Administrative Database

There are no pending requests. Click here to reload this page.
----

I assumed that a: my co-chair had clicked approve or b: you had clicked the "Don't bother" link. 

Seeing as this didn't hit the list (and seems to have disappeared into the Ether / been eaten by the angry bit gods) can you please resubmit it…

Somehow we, or mailman, screwed up...
W

> Goal is to have libssl check config file for [libunbound] section and link to libunbound only if there is such section. In other words run-time link to libunbound will be optional.
> 
> Rationale behind the unnatural choice is to have possibility to generate record from scripting languages without having to create glue code.
> 

-- 
"Does Emacs have the Buddha nature? Why not? It has bloody well everything else..."