[Anima] Russ/Ben: Re: Russ Housley's review of draft-ietf-autonomic-control-plane-24

[Toerless Eckert <[e-mail]>]

Hi Russ/Ben, *

I had a writeup of this reply several weeks back, and then it got lost in some
stupid crash, so i had to recreate it alltogether, and that conflicted
with other priorities. And then i made the wrong choice trying to finish the
IPsec and Joel halperns review before pushing out -25, so further delay to
get this reply out. Oh well..

Diff here

http://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht?url1=https://raw.githubusercontent.com/anima-wg/autonomic-control-plane/6f3151058bcbea7369cfe402203c22a920b54db4/draft-ietf-anima-autonomic-control-plane/draft-ietf-anima-autonomic-control-plane.txt&url2=https://raw.githubusercontent.com/anima-wg/autonomic-control-plane/0caa400fd1c554ece49fddc7dabe8140195aa5bf/draft-ietf-anima-autonomic-control-plane/draft-ietf-anima-autonomic-control-plane.txt

You are also free to compare the final -25 against -24, but that has
additional changes in response to bove mentioned input to -24, Joel and
IPsec stuff.

For the numbered list of arguments for use of rfc822name, please use latest -25
version, it adds argument 3.5 about CA issues signing certs with non-standrd
attributes.

I will discuss the additional arguments/discussions re. rfc822name on the
according threads on the mailing list. 

I find the discussion about rfc822name to be somewhat of an outliar. All the
rest of the SEC IESG review was really good and to the actual point of security.
Sure, it caused me a lot of pain/work getting up to speed on a lot of security 
protocol details, but IMHO hopefully good improvement for the doc. This rfc822name,
 if you excuse me saying so is an outliar:

- The decision for rfc822name was a long-time compromise by practical experience
  and repeated discussions in the WG.

- No contest beyond yours was maintained after reviewing the points explaining
  the choice in 6.1.2. This list was improved several times to address questions
  and resolved them. I have not seen any comment from you how the arguments
  made in that list create a good framework to justify the choice.

-  We also had what i felt to be a good discussion with Ben in person at IETF106
   @ANIMA-WG, that i thought would have explained/persuaded him of our choice, but
  the only followup i saw afterwards was only repeating the "this looks like not
  the right thing" statement.

- I don't think such an assessment is a good technical argument during WG
  time or IETF review compared to the arguments in the document in the list
  in 6.1.2, and even less so during IESG SEC review:

- I have not heard any actual security issue raised against this choice. I
  was under the impression that IESG SEC reviews primary role is to exactly
  support that.

- We have for all the years we revisited the decision a security advisor in
  ANIMA and she too never raised a security issue about this.

Specific replies to all the points inline below. And thanks a lot for the
excellent_review - rfc822name ;-)

Cheers
    Toerless

On Mon, Mar 23, 2020 at 08:56:14PM -0700, Benjamin Kaduk wrote:
> Hi all,
> 
> I had asked Russ to take a look at the rfc822Name usage.  I guess I did not
> do a good enough job of setting the background information for how the
> certificate containing it would be used, so he had to read some other bits
> of the document as well.
> 
> FWIW I agree with his sentiment that "assigning another name type seems
> preferable, even if it has the same syntax" (and that something more
> efficient could be used if we had desire to do so).
> 
> -Ben


> On Fri, Mar 20, 2020 at 05:12:52PM -0400, Russ Housley wrote:
> 
> The document defines Enrollment:
> 
>    Enrollment:  The process where a node presents identification (for
>       example through keying material such as the private key of an
>       IDevID) to a network and acquires a network specific identity such
>       as an LDevID and trust anchors such as Certificate Authority (CA)
>       certificates.
> 
> First, an annoyance.  CA = Certification Authority (not Certificate Authority).  This applies to an earlier definition and many other places as well.

[fun excourse]

In my defense, i grew up in a town with a dialect:

google: site:cisco.com "certificate   authority" -> 59,700
google: site:cisco.com "certification authority" ->  8,580

Also: 

https://en.wikipedia.org/wiki/Certificate_authority

grep -il "ertification authorit" rfc*.txt    | wc -> 272 (#RFCs)
grep -il "ertificate authorit" rfc*.txt      | wc -> 136 (#RFCs)

grep -il "ertification authorit" rfc8???.txt | wc -> 38 (#RFCs)
grep -il "ertificate authorit" rfc8???.txt   | wc -> 13 (#RFCs)

ratio is getting better for your side... a bit.

Does RFC editor have a negative list of wrong words ?
RFC editor abbreviation lists only the correct term.

[/fun excourse]

Fixed.

> IDevID and LDevID are certificates.  In this context, enrollment should be using an IDevID for authentication in order to get a LDevID.

That was the intention of the text.

Pls. check fixed text for CA, Enrollment, root CA, and TA, i hope they are better now.

> The document defines Trust Anchor in a way that is not fully compatible with RFC 5280.  It ought to use the definition from RFC 5280.

I apologize, but this is a bit of a riddle to me because i could not find text that looks like a definition of "trust anchor" in rfc5280 and your concern above also does not explain exactly how the ACP text does not comply with such an "definition".

Are you taking offense in conflating trust anchor and trust anchor information ?
That seems to be a common issue in other documents, eg. in 5280:

"When the trust anchor is provided in the form of a self-signed certificate"

"When trust anchor information" ?

In any case, in lack of a better pointe to an RFC, the ACP text now uses the X.509 text to define trust anchor and i tried to go through the text and distinguish between TA and TA information as good as i could.

If there are still concern about this point, i would appreciate if you could be more specific as to what exactly is still wrong and provide suggested text fixes.

> Section 6.1:
> 
> - The text uses the word "trust" in a vague way.  It should say what they are trusted to do, and what they are trusted to not do.
> 
> - The test says:
> 
>    An ACP node MUST
>    have a Local Device IDentity certificate (LDevID) and a Trust Anchor
>    (TA) consisting of a certificate (chain) used to sign the LDevID of
>    all ACP domain members.
> 
> This does not seem accurate to me in several ways.  Most importantly, a TA is not c certifications path or chain.

Fixed to: To trust another ACP member node with access to the ACP Domain, each ACP member requires
keying material: An ACP node MUST have a Local Device IDentity certificate (LDevID),
henceforth called the ACP Domain Certificate and information about one or more Trust Anchor (TA)
as required for the ACP domain membership check (<xref target="certcheck"/>)

{ Please suggest better text if this is still not good enough}.

> Section 6.1.1:
> 
> - s/X.509/X.509v3/

fixed.

> - What does "All elements are [RFC5280] compliant??? mean?

removed.

It was just meant to be unnecessary duplicaction of initial sentence from same section:
"ACP domain certificates MUST be [RFC5280] compliant X.509v3 certificates"


> - MUST do both RSA and ECC? I think this is too simplistic.  I do not expect devices to have both kinds of private key, but they need to handle both kinds of public key.  This seems to be where the transition paragraph is going, but it is not very clear.

Changed:

"ACP nodes MUST support RSA and Elliptic Curve (ECC) public keys in ACP certificates"

to

"ACP nodes MUST support handling ACP certificates with RSA public keys and ACP certificates with Elliptic Curve (ECC) public keys"

Hope i was guessing right what you are aluding to, if now, then explicit text suggestions are always welcome.

> - I do not understand the wording about ECC curves that MUST be supported.  Normally I see a single curve that MUST be supported for interoperability and and others that SHOULD be supported.  The "or better" wording is really unclear.  Also, each of the curves has a specific key length associated with it.

In the ACP we have any-to-any connectivity and hence any-any mutual cert authentication, so i felt it to be prudent to support some good range of cert key sizes to ensure interoperability across a wide range of equipment within a single ACP - but also exclude key sizes that we feel are unnecessarily weak, such as RSA with less than 2048 bits.

I changed the two paragraphs now to the following three:

---
<t>ACP nodes MUST support handling ACP certificates, TA certificates and certificate chain certificates (henceforth just called certificates in this section) with RSA public keys and certificates with Elliptic Curve (ECC) public keys. Certificates with ECC keys MUST indicate to be Elliptic Curve Diffie-Hellman capable (ECDH) if X.509 v3 keyUsage and extendedKeyUsage are included in the certificate.</t>

<t>ACP nodes MUST NOT support certificates with RSA public keys of less than 2048 bits. They MUST support certificates with RSA public keys with 2048 bits and SHOULD support longer RSA keys. ACP nodes MUST support certificates with ECC public keys using NIST P-256, P-384 and P-521 curves.</t>

<t>ACP nodes MUST support SHA-256, SHA-384, SHA-512 signatures for certificates with RSA key and the same RSA signatures plus ECDSA signatures for certificates with ECC key.</t>
---

I don't understand whether your note about the key length of the curves is an indication of missing text. When i first reviewed with Ben, i had to enter the curves because thats as specific as necessary AFAIK, but given how the key length is implied, i wouldn't understand why i would need to write those down. I don't remember that i have seen that being done either in other RFCs i read through.

In any case, specific text suggestions always welcome in case this text is not sufficient.

> - I would really like to see some clarity about digital signature vs. key establishment.  The two are jumbled together is a way that is difficult to figure out.  One way to read it is that digital signature is required, but key establishment is optional.  I am not sure that is the intent.

I may not understand what you mean with "key establishment".

The first two paragraphs talk about the requirements to handle public key in the certificate
including permitted minimum key-length for RSA.  Is that what you call "key establishment" ?

The third paragraph talks about requirements against the signatures in the certificates.

The idea is to have the necessary and sufficient text for all ACP nodes to be
able to authenticte each other wrt. to their certificate.

> - The TLS reference should point to TLS 1.3 (not TLS 1.2).

The MTI for the ACP is TLS 1.2, TLS 1.3 is SHOULD because of the need for
lowest common denominator in more embedded device space moving very slowly
from TLS 1.2 to TLS 1.3.  See e.g.: 6.8.2. Also note that TLS profile
for ACP is quite close to TLS 1.3 wrt. crypto requirements.

> Section 6.1.2:
> 
> - Using rfc822name seems wrong to me.  Assigning another name type seems preferable, even if it has the same syntax.  The semantics are different.  This is the approach used in RFC 4556 for the Kerberos Principal Name.

Can you point me to any specifc RFC text that actually prohibits what we want to do ?

I was reading through the RFCs (forgot number now) that i think defined the
encodings, and i could not find anything that would prohibit to use the existing
rfc822name for a name that is a valid rfc822name.

It would also be good if you review and address the bullet points 2.1 ... 2.6 and
3.1 ... 3.5 in section 6.1.2. These points explain and justify the choice
of rfc822name.

For example, points 2.1 , 2.4 and 2.5 would be closest in addressing your semantic concern.

> - The document claims that "subjectAlName / otherName" will not be able to be supported because it uses a field that is not mandatory.  I do not think that this is a very big deal.  The otherName definition is:
> 
>    OtherName ::= SEQUENCE {
>         type-id    OBJECT IDENTIFIER,
>         value      [0] EXPLICIT ANY DEFINED BY type-id }

> So, the "extra" processing is parse past the type and length for the SEQUENCE, perform an exact match to the constant value of the object identifier, parse past the type and length of the value.  For example, when using the python ASN.1 library for the certificate processing, the additional code is tiny.  After this, the parsing of the string is exactly the same.  In fact, it might be possible to define a format that is even easier to parse if it did not have to look like an email address.  Since this "extra" is so small, I think we should not use rfc822name in this context.

This is addressed in for example point 3.2 in 6.1.2. The ASN.1 libraries available in 
embedded systems do not necessarily provide the ability to be extended. They may
often come from embedded system OS vendors or third parties.

Also see points 2.2 and 2.3 of 6.1.2: There is a whole backend and diagnostic infrastructure that needs to be extended when introducing new types to decode the new field. This is highly undesirable for easy operationalization.

> Section 6.1.3:
> 
> - validity period checking (in step 1) is part of path validation (in step 3).

Sigh... hope i fixed all the references to the rule numbers in the document
after eliminating this step 1 and mentioning how it is included in what is
now step 2.

> - In step 4, revocation checking allowed to be initially skipped, but the wording is concerning since the check is in a MUST statement.  Then, the checking takes place when communications are established.  I assume some cache is used, otherwise the connection works for a short while, then shuts down, over and over and over.

Hopefully fixed.

Pls. check the reworded text. I conditioned the MUST
with a "when availalable" and changed the secondary MUST to lower case as they
just explain the mechanism.

Wrt to the up/down issue you mention: It seems cleat that one must periodically
re-try a connection to another peer because it could have received a new
certificate.

If the connection is torn down at any point in time because of newly learned
CRL/OCSP information, it also seems clear the connection needs to be torn down.

I do not understand all possible connection setup protocols, but from
what i understand, a negative CRL or OCSP information will not change back
to positive in the future, so a connection torn down once because of
a particular certificate would need to be retried later up to the
point that the signaling gets to the same certificate, at which point in
time it would fail. 

Aka: with caching of OCSP/CRL results, the connection would still be probed
periodically, but would not be brought up again until there is a new cert.

Is there anything of these explanations you would like to see written
down ? Or that are wrong ?

> Section 6.1.4:
> 
> - Please use the terms from RFC 5280.  That is, use "trust anchor" not "root-CA".

a) While trying to do this, it seemed to me as if the word "trust point" that
was also used in the text was also redundant. So i hopefully correctly
eliminated it and replaced it with TA.

b) Given how rfc7030 in 4.1.3 also refers to Root CA, and equaly
the BRSKI draft in several places, and also rfc8572, aka: all the
documents that this ACP relates to, i wonder what the rule of thumb is when
 it is appropriate to use root CA as a term and when not... I don't want
to be the first RFC of all the ones that the ACP builds on or relates
to that is NOT using the term... without understanding why ACP should be
the only one not using it...

There are similarily few occurrances of "root CA" as in those other documents...

Just wondering.

Thanks a lot for the review, and apoligies for my delay.

Cheers
    Toerless
> 
> Russ