On Wed, May 15, 2013 at 08:47:20PM +0200, Andy Polyakov wrote:
> >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.
>
> Oops! Git noob mistake.
Got the new code. Thanks.
> >There is still a bunch of work before this is usable.
>
> 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.
_______________________________________________
dane mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/dane
