Subject: [dane] Pseudocode review

From: Paul Hoffman <[email-masked]>
Date: Wed, 4 Jan 2012 06:46:25 -0800

Greetings again. As Jakob indicated, we would *really* like to have a careful review of the pseudocode now instead of waiting for WG Last Call. TLSA has rules spread in many part of the body text, and the pseudocode is supposed to pull every rule into one place, correctly. Many developers like pseudocode, and will tend to focus on this non-normative appendix rather than the body text. Ilari already found one small error (thanks!), and we would like developer-like folks on the list to poke harder at the section so that we are somewhat confident that everyone understands what the protocol is doing.

--Paul Hoffman

---

From: Stephane Bortzmeyer <[email-masked]>
Date: Thu, 5 Jan 2012 09:55:49 +0100

On Wed, Jan 04, 2012 at 06:46:25AM -0800,
 Paul Hoffman <[email-masked]> wrote 
 a message of 8 lines which said:

> we would *really* like to have a careful review of the pseudocode
> now instead of waiting for WG Last Call. 

I've read it (carefully, I believe), I think I have understand it and
I did not notice mistakes.

After "This appendix is non-normative.  If the steps below disagree
with the text earlier in the document, the steps earlier in the
document should be considered correct and this text incorrect.", I
suggest to add:

For instance, pay attention to the fact that, like most pseudo-codes,
it is more strict than the normative text. For instance, it forces an
order on the evaluation of criteria (X.509 before TLSA when
certificateUsage == 0) which is not mandatory.

---

From: Paul Hoffman <[email-masked]>
Date: Thu, 5 Jan 2012 07:48:13 -0800

> I did not notice mistakes.

Whew! It would be nice to hear that from at least a few others (and it would be fine to hear about mistakes as well).

> certificateUsage == 0) which is not mandatory.

Good suggestion, yes.

--Paul Hoffman

---

From: James Cloos <[email-masked]>
Date: Thu, 05 Jan 2012 15:18:35 -0500

>>>>> "PH" == Paul Hoffman <[email-masked]> writes:

>> I did not notice mistakes.

PH> Whew! It would be nice to hear that from at least a few others (and
PH> it would be fine to hear about mistakes as well).

Other than the already-reported missing }, I agree that it looks correct
and should be easy for anyone coding a tlsa-aware client to understand.

-JimC
-- 
James Cloos <[email-masked]>         OpenPGP: 1024D/ED7DAEA6

---

From: Ondrej Mikle <[email-masked]>
Date: Thu, 05 Jan 2012 22:32:02 +0100

> Whew! It would be nice to hear that from at least a few others (and it would be fine to hear about mistakes as well).

I've re-read it a few times and I think it's very well written. I didn't find 
anything that wasn't mentioned already.

>> certificateUsage == 0) which is not mandatory.

Seconded.

(Some of the nested if's could be rewritten as "if (X and Y)" to avoid explicit 
PKIX/TLSA evaluation order, but it would hurt clarity of pseudocode.)

Ondrej Mikle

---

From: Ondrej Mikle <[email-masked]>
Date: Mon, 09 Jan 2012 22:49:08 +0100

While writing update to "Creating TLSA records" I noticed an additional detail 
that might be added to paragraph proposed by Stephane:

> certificateUsage == 0) which is not mandatory.

At the end of the paragraph, I suggest to add sentence:

"In practical deployment it might be actually beneficial to switch the 
pseudocode order and evaluate TLSA criteria before PKIX validation in order to 
minimize delay due to network operations like revocation checks."

Reason is simple: TLSA check is fast compared to OCSP/CRL check, AIA chasing and 
other chain-building methods proposed in RFC 4158.

BTW there is a rather interesting treatment of current PKIX validation libraries 
in "What exactly are the benefits of libpkix over the old certificate path" 
threads of http://news.gmane.org/gmane.comp.mozilla.crypto mailinglist archive 
(Jan 4-Jan 6 2012 messages).

Ondrej Mikle

---

From: Stephane Bortzmeyer <[email-masked]>
Date: Tue, 10 Jan 2012 09:48:20 +0100

On Thu, Jan 05, 2012 at 10:32:02PM +0100,
 Ondrej Mikle <[email-masked]> wrote 
 a message of 29 lines which said:

> (Some of the nested if's could be rewritten as "if (X and Y)" to
> avoid explicit PKIX/TLSA evaluation order, but it would hurt clarity
> of pseudocode.)

-1 Clarity depends on your taste. Personally, I find (X and Y) clearer
than (if X (if Y... 

When people ask for pseudo-code, they typically mean "procedural
pseudo-code" while "functional pseudo-code" would be
better. Functional pseudo-code can be both a good specification *and*
an executable implementation, unlike procedural pseudo-code.

Let's replace Fortran pseudo-code by Haskell pseudo-code in RFCs :-)

---

From: Miek Gieben <[email-masked]>
Date: Mon, 16 Jan 2012 11:12:40 +0100

[ Quoting <[email-masked]> at 06:46 on Jan  4 in "[dane] Pseudocode re..." ]
> and will tend to focus on this non-normative appendix rather than the body
> text. Ilari already found one small error (thanks!), and we would like
> developer-like folks on the list to poke harder at the section so that we are
> somewhat confident that everyone understands what the protocol is doing.

To pseudo code looks good to me.

But I do have a question, the line:

    if (Match(matchingType, Select(selectorType, C), R)) {
            Finish(2)
    }

Isn't this better?

    if (Match(R.matchingType, Select(R.selectorType, C), R)) {
            Finish(2)
    }

It wasn't immediately clear to me where matchingType and selectType came
from (when just reading the pseudo code).

Kind regards,
Miek Gieben

---

From: Paul Hoffman <[email-masked]>
Date: Mon, 16 Jan 2012 08:14:55 -0800

> from (when just reading the pseudo code).

Yes. Based on earlier suggestions, the next draft will have:
      if ((C passes PKIX validation in H) and
              Match(R.matchingType, Select(R.selectorType, C), R.cert)) {
          Finish(2)

--Paul Hoffman

---

From: Scott Schmit <[email-masked]>
Date: Sat, 21 Jan 2012 20:20:34 -0500

On Wed, Jan 04, 2012 at 06:46:25AM -0800, Paul Hoffman wrote:
> Greetings again. As Jakob indicated, we would *really* like to have a
> careful review of the pseudocode now instead of waiting for WG Last
> Call. TLSA has rules spread in many part of the body text, and the
> pseudocode is supposed to pull every rule into one place, correctly.
> Many developers like pseudocode, and will tend to focus on this
> non-normative appendix rather than the body text. Ilari already found
> one small error (thanks!), and we would like developer-like folks on
> the list to poke harder at the section so that we are somewhat
> confident that everyone understands what the protocol is doing.

It looks correct, but some thoughts on how it could be better:

I hate magic numbers. Since you're comfortable using enumerations
elsewhere in the pseudocode, I'd recommend replacing 0/1/2 as arguments
for Finish with ABORT_TLS/NO_TLSA/ACCEPT (or similar). Ideally, the
usage/select/match numbers would be assigned to enumerations as well,
but they're defined in the spec, so they aren't as big a deal.

It's alluded to in Section 4.4, but it would probably be good to add "in
conflict with local policy" and/or "with insufficient security per local
policy" to the list of reasons that a record might be unusable. This
helps resolve issue #35.

In various places, you use R.foo to represent fields in the TLSA record,
but then use R when you should use R.cert (partly fixable by having the
helper functions use X.cert or Y.cert, but also shows up in the trust
anchor handling section). It's obvious what you mean, but programming
trains us to be literal.

Grammar/spelling: final comment should read "if here, then none of the
TLSA records were sufficient for TLS"

Nits you can ignore if you want given your disclaimer on the pseudocode:

It feels weird to me to do all the work to validate the EE cert against
the chain before even checking if one of the certs in it match the one
specified by the TLSA record. That said, it is pseudocode; deployed
code will almost certainly look very different anyway.

This pseudocode implies that the TLSA checker will/should search
possible PKIX chains more thoroughly than the client normally would to
try to find a match. This is a little out of step with the warnings in
A.1.1. (And the main text is silent on the question.)

Your Select & Match functions can return undef, but you don't handle
them. Of course, your filtering should have removed the records that
would cause the functions to return undef before these functions get
them, but it would probably be better to mark those lines as unreachable
(with this pseudocode). That makes it clear that receiving those values
is a coding error.

Hope this helps,

-- 
Scott Schmit

---

From: Paul Hoffman <[email-masked]>
Date: Sun, 22 Jan 2012 09:35:42 -0800

On Jan 21, 2012, at 5:20 PM, Scott Schmit wrote:

> for Finish with ABORT_TLS/NO_TLSA/ACCEPT (or similar).

Good call. Done.

> Ideally, the

It would be better to keep them aligned with the spec instead of adding a level of indirection with some equivalences.

> policy" to the list of reasons that a record might be unusable. This

Yes.

> anchor handling section). It's obvious what you mean, but programming

Already noted before, already fixed.

> TLSA records were sufficient for TLS"

Sure.

> code will almost certainly look very different anyway.

Correct. I *think* the pseudocode matches the wording in the document for linguistic order, even if it is not optimized for speed. If others really want this change, we can make it.

> A.1.1. (And the main text is silent on the question.)

I'm not sure what you mean here; please say more.

> (with this pseudocode). That makes it clear that receiving those values

Checking for coding errors is not the correct way to do this. I have replaced "return undef" with "// unreachable", just like we have for the Finish function.

> Hope this helps,

Definitely, but I still want to know more about the one noted above.

--Paul Hoffman

---

From: Scott Schmit <[email-masked]>
Date: Sun, 22 Jan 2012 13:33:25 -0500

> I'm not sure what you mean here; please say more.

Sure. In the pseudocode:
   // A TLS client might have multiple trust anchors that it might use
   //    when validating the TLS server's end entity certificate. Also,
   //    there can be multiple PKIX validation chains for the
   //    certificates given by the server in TLS. Thus, there are
   //    possibly many chains that might need to be tested during
   //    PKIX validation.

Normally, if there are multiple possible chains between the certificate
and a trust anchor, the client will stop after finding the first chain
that validates successfully. Here, it checks every possibility until it
either finds a TLSA match or finds none at all.

In A.1.1, the text implies that the client will stop after finding the
first chain that works, so the operator should chose their anchor with
care, because the association could fail for some clients or under some
conditions (e.g., the client has or hasn't visited a site with an
alternate certificate recently).

-- 
Scott Schmit

---

From: Scott Schmit <[email-masked]>
Date: Sun, 22 Jan 2012 13:37:19 -0500

> confident that everyone understands what the protocol is doing.

Sorry I missed this the first time, but I notice that with certificate
usage type 2, the pseudocode doesn't pay attention to the selector type
or match type fields. At the same time, there's no enforcement of the
implicit rule that the selector type will be 0 and the match type 0.
Also, nothing in the text says anything approaching "if the usage is 2,
the selector type and match type MUST be 0".

I don't think such a rule is necessary, provided the trust anchor used
is a commonly-known one. For private trust anchors, they'll need to use
matching type 0 and selector type 0 if the connection is going to
succeed. Selector type 1 implies that the client has a full certificate
to extract the SPKI from, which in this case, it wouldn't. Perhaps that
should be in the text somewhere.

-- 
Scott Schmit

---

From: James Cloos <[email-masked]>
Date: Sun, 22 Jan 2012 14:51:02 -0500

>>>>> "SS" == Scott Schmit <[email-masked]> writes:

SS> At the same time, there's no enforcement of the implicit rule that
SS> the selector type will be 0 and the match type 0.  Also, nothing in
SS> the text says anything approaching "if the usage is 2, the selector
SS> type and match type MUST be 0".

What implicit rule?

Why shouldn't 2 0 1 or 2 0 2 be OK?  Or 2 1 1 or 2 1 2 for that matter?

The tls server will send a cert chain, so matching a digest, spki or
digest of an spki should be cryptographically secure.

-JimC

---

From: Scott Schmit <[email-masked]>
Date: Sun, 22 Jan 2012 15:45:05 -0500

On Sun, Jan 22, 2012 at 02:51:02PM -0500, James Cloos wrote:
> >>>>> "SS" == Scott Schmit writes:

What I meant by implicit rule is that the pseudocode assumes that if
the usage is 2, selector and match are 0, otherwise the pseudocode won't
work. At the same time, it doesn't filter out 2 x y records where x != 0
and y != 0.

> Why shouldn't 2 0 1 or 2 0 2 be OK?  Or 2 1 1 or 2 1 2 for that matter?

Sorry, I misremembered the rule concerning inclusion of the TA in the
certificate chain sent by the server.  Rereading the TLS RFC, I see that
the server isn't required to send the TA's certificate, but it isn't
prohibited from doing so either. I had assumed that the TA is always
excluded. What I wrote is still true if the server chooses not to
send it.

As I explained in the following paragraph that you didn't quote,
non-zero selector and match types will work fine as long as the client
already knows the trust anchor. If the trust anchor is being introduced
by the TLSA record, it's legal but won't work if trust anchors aren't
included in the cert chain sent by the server.

Apologies for the confusion.

-- 
Scott Schmit

---

From: James Cloos <[email-masked]>
Date: Sun, 22 Jan 2012 17:18:04 -0500

>>>>> "SS" == Scott Schmit <[email-masked]> writes:

SS> What I meant by implicit rule is that the pseudocode assumes that if
SS> the usage is 2, selector and match are 0, otherwise the pseudocode won't
SS> work. At the same time, it doesn't filter out 2 x y records where x != 0
SS> and y != 0.

OK. I thought you meant an implicit rule in the text, not in the pseudocode.

Clearly my eyes must have glazed a bit when I first reviewed the peudocode;
I think the R.certUsage == 2 case should have a (Match(matchingType,
Select(selectorType, C), R) clause, too, like usage 1 does.

SS> Rereading the TLS RFC, I see that the server isn't required to send
SS> the TA's certificate, but it isn't prohibited from doing so
SS> either. I had assumed that the TA is always excluded. What I wrote
SS> is still true if the server chooses not to send it.

My impression was that usage 2 was added primarily for the case where
the chains have depth == 1.

SS> Apologies for the confusion.

Likewise!

-JimC

---

From: Paul Hoffman <[email-masked]>
Date: Mon, 23 Jan 2012 09:54:43 -0800

Sorry, I'm still missing it, and I think that the problem is on my end.

> either finds a TLSA match or finds none at all.

I don't see how the wording above indicates that all chains must be tested. It says "might need to be tested".

> alternate certificate recently).

Yes, that is correct.

If you propose alternate wording for the pseudocode comment above, I suspect I'll say "that's what we meant" and use it directly.

--Paul Hoffman

---

From: Paul Hoffman <[email-masked]>
Date: Mon, 23 Jan 2012 10:02:42 -0800

On Jan 22, 2012, at 10:37 AM, Scott Schmit wrote:

> or match type fields.

I had a very good reason for not adding that when Jakob and I were first working on the pseudocode, and for the life of me I can't remember it. I'll add it in the -15.

> At the same time, there's no enforcement of the

I fully disagree with that last paragraph. Usage type 2 is useful for the case of "I generated this cert myself and I'm going to use it in the TLS handshake so that the chain to the (new TLSA) root is length 0".

--Paul Hoffman

---

From: Ondrej Mikle <[email-masked]>
Date: Mon, 23 Jan 2012 19:35:37 +0100

> If you propose alternate wording for the pseudocode comment above, I suspect I'll say "that's what we meant" and use it directly.

I removed the "first match" behavior from the update to A.1 I posted a
while ago. Though in real deployment it will take time until
implementations support building and checking all paths (e.g. RFC 4158).

What about adding the following sentence at the end of the "// A TLS
client might have multiple trust anchors that it might..." pseudocode
commend:

"Note that there will be transitional period where clients may not check
all chains and instead pick one arbitrary chain."

Ondrej Mikle

---

From: Paul Hoffman <[email-masked]>
Date: Mon, 23 Jan 2012 15:04:19 -0800

> all chains and instead pick one arbitrary chain."

I'm still confused. Why should an implementation check all chains? The first good chain is good enough, yes?

--Paul Hoffman

---

From: Ondrej Mikle <[email-masked]>
Date: Tue, 24 Jan 2012 01:53:05 +0100

> I'm still confused. Why should an implementation check all chains? The first good chain is good enough, yes?

I'm not sure where the confusion happens, so I'll write it out explicitly in a
"python-ic" pseudocode.

Wanted: first chain where the expression "TLSA association succeeds && PKIX
validation succeeds" evaluates to true (according to DANE rules).

Current implementations of PKIX validation (except CryptoAPI) have only a method
like:

findFirstValidChain(chainFromHandshake, trustAnchors, cachedCerts) returning
CertChain

In a naive implementation, a developer might be tempted to do:

tlsaRecord = fetchTlsa(fqdn, port, protocol)
certChain = findFirstValidChain(chainFromHandshake, trustAnchors, cachedCerts)
valid = checkTlsa(certChain, tlsaRecord)

Such "first match" implementation will ocassionally fail as false-negative. The
"checkTlsa" method does first TLSA check, then PKIX validation on a chain
(including CLR/OCSP) according to DANE rules.

An implementation that should be correct according to DANE rules might look like
(taking into account blocking operations like OCSP/CRL/AIA fetching):

tlsaRecord = fetchTlsa(fqdn, port, protocol)
# build initial certchains queue to check from "offline" data
queue = [chainFromHandshake] + buildValidChains(chainFromHandshake,
trustAnchors, cachedCerts)

while (not queue.empty):
  chain = queue.popFront()
  if checkTlsa(chain, tlsaRecord): return True

  #add chains we didn't see before via RFC 4158 methods like AIA fetch
  queue += buildRfc4158Chains(chain, trustAnchors, cachedCerts)

return False #no certchain matched

Methods "buildValidChains" and "findFirstValidChain" differ only in the fact
that the first one returns all chains, not just the first one (cycle detection
is skipped for clarity).

Ondrej Mikle

---

From: "Jim Schaad" <[email-masked]>
Date: Mon, 23 Jan 2012 22:36:46 -0800

I would hope that the code in these libraries would start accepting the DANE
information as a chain building parameter and thus the first valid chain
changes based on the DANE records.

Jim

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf Of
> Ondrej Mikle
> Sent: Monday, January 23, 2012 4:53 PM
> To: Paul Hoffman
> Cc: [email-masked]
> Subject: Re: [dane] Pseudocode review
> 
> >>>> In A.1.1, the text implies that the client will stop after finding
> >>>> the first chain that works, so the operator should chose their
> >>>> anchor with care, because the association could fail for some
> >>>> clients or under some conditions (e.g., the client has or hasn't
> >>>> visited a site with an alternate certificate recently).
> >>>
> >>>
> >>> Yes, that is correct.
> >>>
> >>> If you propose alternate wording for the pseudocode comment above, I
> suspect I'll say "that's what we meant" and use it directly.
> >>
> >>
> >> I removed the "first match" behavior from the update to A.1 I posted
> >> a while ago. Though in real deployment it will take time until
> >> "Note that there will be transitional period where clients may not
> >> check all chains and instead pick one arbitrary chain."
> >
> >
> > I'm still confused. Why should an implementation check all chains? The
first
> good chain is good enough, yes?
> 
> Wanted: first chain where the expression "TLSA association succeeds &&
> PKIX validation succeeds" evaluates to true (according to DANE rules).
> 
> findFirstValidChain(chainFromHandshake, trustAnchors, cachedCerts)
> returning CertChain
> 
> tlsaRecord = fetchTlsa(fqdn, port, protocol) certChain =
> findFirstValidChain(chainFromHandshake, trustAnchors, cachedCerts) valid =
> checkTlsa(certChain, tlsaRecord)
> 
> 
> An implementation that should be correct according to DANE rules might
look
> like (taking into account blocking operations like OCSP/CRL/AIA fetching):
> 
> 
> tlsaRecord = fetchTlsa(fqdn, port, protocol) # build initial certchains
queue to
> check from "offline" data queue = [chainFromHandshake] +
> buildValidChains(chainFromHandshake,
> that the first one returns all chains, not just the first one (cycle
detection is
> skipped for clarity).
> 
> Ondrej Mikle
> _______________________________________________
> dane mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/dane

---

From: Martin Rex <[email-masked]>
Date: Wed, 25 Jan 2012 00:56:14 +0100 (MET)

Ondrej Mikle wrote:
> Current implementations of PKIX validation (except CryptoAPI)
> have only a method like:
> 

I'm actually heavily opposed to suggesting such convolutions
in dane protocol.

It perfectly sensible for a TLS client, and conforming to PKIX,
to simply ASN.1 decode the well-formed certification path from
the Server's certificate message, scan the list of preconfigured
trust anchors whether it matches a cert from the server's cert path
and then call a certificate path validation algorithm with the
server-supplied cert path and the matching trust anchor.

I know that there are some TLS clients (mostly browser) doing
all kinds of fancy, weird and stupid things before validating
the cert chain provided by the server, but that is wasting energy
and causing TLS server configuration errors to result in
inconsistent, sometimes seemingly non-deterministic failures
regularly precluding timely recognition and fix of the server
configuration error by the responsible admin.

-Martin

---

From: Ondrej Mikle <[email-masked]>
Date: Wed, 25 Jan 2012 14:19:58 +0100

> configuration error by the responsible admin.

+1. I don't favor the way browsers employ "all kinds of fancy, weird and
stupid things before validating" either. Attempt to hide server errors
was a stupid marketing push ("because browser XY doesn't show error and
we don't want users to go away"). But reverting to previous behavior may
be a hard task.

Ondrej Mikle

---

From: Scott Schmit <[email-masked]>
Date: Wed, 25 Jan 2012 09:01:26 -0500

> we don't want users to go away"). But reverting to previous behavior may

+1 I'd almost be in favor of tilting the spec in favor of "if you don't
reconfigure your server to include just the certs you want checked, it's
not allowed to work. It's not that hard to do it right, it won't break
anything for non-DANE clients, so turn away from the dark side!"

-- 
Scott Schmit