Subject: Re: [secdir] SecDir review of draft-ietf-hip-rfc6253-bis

From: Samu Varjonen <[email-masked]>
Date: Fri, 12 Feb 2016 13:47:11 +0200

Hi,

answers inline (and a question).

-Samu Varjonen

On 22.12.2015 19:22, Sean Turner wrote:
> Fear not as this is just the secdir review!
> 
> I have reviewed this document as part of the security directorate’s
> ongoing effort to review all IETF documents being processed by the
> IESG.  These comments were written with the intent of improving
> security requirements and considerations in IETF drafts. Comments not
> addressed in last call may be included in AD reviews during the IESG
> review.  Document editors and WG chairs should treat these comments
> just like any other last call comments.
> 
> draft summary: This is a bis document that specifies the certificate
> parameter and the error signaling in case of a failed verification for
> the Host Identity Protocol (HIP) v2; the previous version was
> experimental now it's moving to the standards track.  The big change
> is that the SPKI cert type got dropped.
> 
> secdir summary: pretty darn close to being ready, but I’ve got a
> couple of questions and some nits:
> 
> Questions:
> 
> Q1. What’s the rationale for the renumbering of the certificate types?
>  Were there no HIPv1 implementations that used the SPKI option so it’s
> safe to just renumber?  If you’re not 100% sure whether there were any
> you could mark values 2, 4, 6, and 8 as reserved/obsoleted and keep on
> using the existing values for 3, 5, 7.  nit-wise: In s2 values 5-8 are
> still referenced in the last three paragraphs.

The type number table in the document has been reverted and 2,4,6, and 8 
has been obsoleted.

> 
> Q2. I see that you pointed from s4 to s5 of 5280 for revocation.  The
> only error signaling in your s5 seems to be “invalid", but 5280
> defines a number of CRL reason codes, e.g., key compromise, but also
> “hold”.  If you’re not planning on supporting “hold” then it might be
> worth adding some more text to say that any HIP certificate serial
> number that appears on the CRL is treated as “invalid” regardless of
> the reason code.  BTW - I’m in no way suggesting that you support
> hold.
> 

We do not support hold and clarification is added

> Q3. On the errors, does the credential_required error cover the case
> where the sender's cert is sent but not the intermediary?  I’m
> assuming short paths, i.e., root, intermediary, and end-entity, so
> you’d just send the end-entity and the recipient says “hey send me
> you’re intermediary” or does it just fail with the
> credentials_required error?
> 

It will fail with credentials_required error. Clarification added.

> Nits:
> 
> nit 0: Often a bis draft will include a section that says what’s
> changed since the original draft, but since it’s basically just “we
> dropped the SPKI certificate type” maybe just put something like that
> at the end of the introduction?
> 
> nit 1: I read this a couple of times:
> 
>   CERT parameters with the same Cert group number
>   in the group field indicate a logical grouping.
> 
> Isn’t it that the same group # is in the Cert group field:?
> 
>   CERT parameters with the same number
>   in the Cert group field indicate a logical grouping.
> 

Is this about missing CERT word in the group field? Or did I 
misunderstand this question?

> nit 2: There’s some error signaling, but what happens if I set Cert ID
> to 0, the CERT parameters are not sent in ascending order, etc. do you
> throw an error?
> 

The numbers in the Cert ID field MUST start from 1 up to Cert count. 
Cert ID = 0 would result in INVALID_CERTIFICATE.
About the order the document says "The CERT parameters MUST be placed in 
ascending order, within a HIP control packet, according to their Cert 
group field." Different order would result in INVALID_CERTIFICATE. 
Clarification added to the text.

> nit 3: Do you need to specify how you do the padding, e.g., all 1s, all 
> 0s?
> 

All 0s clarified in the text.

> nit 4: About the example cert in Appendix A:
> 
> 0. It’s not really a certificate in Appendix A it’s a pretty print of
> some certificate fields.  Maybe you could include the HEX
> representation of DER cert in section A.1 and the pretty print in
> section A.2?
> 

Updated version is added as above.

> 1. The example shows sha1WithRSAEncryption and that’s not one of the
> algs in RFC 7401; switch it to ecdsa-with-SHA384 or whatever is being
> used most now.
> 
> 2. The serial # can’t be 0 it’s got to be a positive # (see s4.1.2.2 of 
> 5280)
> 

Fixed

> 3. Might be good to update the validity period :)
> 

Fixed

> 4. Are 1024-bit keys normal maybe it should be changed to 2048-bit?
> 

Fixed

> 5. I get that there will be profiles for the HIP certificates, but
> there’s a couple of extensions not shown that are in pretty much every
> certificate compliant with RFC 5280; AKI and KU come to mind.
> 

This example's idea is just to show how to include HITs into the 
certificate so I do not see the reason to add all the possible 
extensions to it. Unless they could contain HITs.

> A question that I’m curious about but that has absolutely nothing to
> do with whether this draft progressing or not (i.e., IESG these are
> not the droids you’re looking for): Is anybody implementing the DSA
> suite in RFC 7401?
> 
> spt