Subject: Re: [MSEC] WGLC on draft-ietf-msec-gdoi-update-08

From: Sean Turner <[email-masked]>
Date: Sat, 02 Apr 2011 19:09:58 +0200

#1) Add "updates: 3547 (once approved)" to the header.  This will make 
it match the intro/abstract.  I can almost guarantee a discuss from 
somebody on this.

#2) ISAKMP Phase 1 is described in Section 2.1 as "one such mechanism". 
  This isn't a MUST support.  As far as I can tell there isn't a MUST 
implement Phase 1 protocol (the protocols in Appendix A are also just 
examples).  But, to implement this document you've got to do things as 
described in RFC 2407-2409.  It seems like this doc and IKEv1 are pretty 
well intertwined.  And, the g-ikev2 draft doesn't use these formats it 
uses something else...Let's be clear about what it's being used with. 
It seems like this section ought to be changed as follows:

OLD:

   The following sections describe one such "phase 1" protocol.  Other
   protocols which may be potential "phase 1" protocols are described in
   Appendix A.  However, the use of the protocols listed there are not
   considered part of this document.

  2.1.  ISAKMP Phase 1 protocol

   This document defines how the ISAKMP phase 1 exchanges as defined in
   [RFC2409] can be used a "phase 1" protocol for GDOI.  The following
   sections define characteristics of the ISAKMP phase 1 protocols that
   are unique for these exchanges when used for GDOI.

   Section 7.1 describes how the ISAKMP Phase 1 protocols meet the
   requirements of a GDOI "phase 1" protocol.

NEW:

   This document defines how the ISAKMP phase 1 exchanges as defined in
   [RFC2409] can be used a "phase 1" protocol for GDOI.  The following
   sections define characteristics of the ISAKMP phase 1 protocols that
   are unique for these exchanges when used for GDOI.

   Section 7.1 describes how the ISAKMP Phase 1 protocols meet the
   requirements of a GDOI "phase 1" protocol.

Delete the 2.1 header and renumber 2.1.1 and 2.1.2 to 2.1 and 2.2, 
respectively.

I would also delete Appendix A.  The currently proposed IKEv2 mechanism 
isn't reusing these fields and the KINK option never got defined.

#3) Sec 3.2: r/Hashes are computed in the manner described within RFC 
2409./Hashes are computed in the manner described within [RFC2409].

#4) Sec 3.2: expand prf on first use.

#5) Sec 3.3: 2 lower case "must" should they be "MUST"?
              1 lower case "should" should it be "SHOULD"?

#6) Sec 4.2: Something is wrong with this:

   Flags MUST have the Encryption bit set according to [RFC2008, Section
   3.1].

Maybe it should be:

   Flags MUST have the Encryption bit set according to Section 3.1 of
   [RFC2408].

#7) Sec 5: r/in accordance with RFC 2408./in accordance with [RFC2408].

#8) Sec 5.1, 5.2: r/defined in RFC 2408./defined in [RFC2408].

#9) Sec 5.2: This is difficult to parse:

    The
    next payload MUST NOT be a SAK Payload or SAT Payload type, but the
    next non-Security Association type payload.

Maybe it's supposed to be:

    The
    next payload MUST NOT be a SAK Payload or SAT Payload type; it MUST
    be the
    next non-Security Association type payload.

#9) Sec 5.2: r/Must/MUST   X4

#10) Sec 5.2.1: r/SA Attributes payloads must be/SA Attributes payloads 
MUST be

#11) Sec 5.3: r/Must/MUST X2
               r/should/SHOULD
               r/must/MUST

#12) Sec 5.3: What is the conformance requirement for the following:

   Any or all attributes may be optional, depending on the group policy.

#13) Sec 5.3.1:

r/ attributes may be present / attributes MAY be present  or change to 
say they are OPTIONAL.

r/attributes must/attributes MUST

#14) Sec 5.3.1:

The KEK_MANAGEMENT_ALGORITHM attribute may only be included in a
    GROUPKEY-PULL message.

What happens if it's included in the PUSH message?

#15) Just noted to instances of GROUPKEY_.  Aren't they supposed to be 
GROUPKEY-*?

#16) Sec 5.3.3.3: r/as recommended in/as defined in

#17) Sec 5.3.6: Any reason not to point to FIPS 180-3?

#18) Sec 5.3.6: r/and SIG_HASH_ALGORITHM is not required to be present 
in a SAK Payload/and SIG_HASH_ALGORITHM is OPTIONAL in a SAK Payload

#19) Sec 5.3.7: Does anybody really want RSA-PSS?

#20) Sec 5.4: r/Must/MUST
               r/Section 3.3 of RFC 2408./Section 3.3 of [RFC2408].

#21) Sec 5.4.1: r/ Section 4.2.1 of RFC 5374/Section 4.2.1 of [RFC5374]
                 r/should/SHOULD X2 ??

#22) Sec 5.5: r/Must/MUST

#23) Sec 5.5.1: Protocol, SEC ID Port, DST ID Prot, DST ID Port bullets 
r/should/SHOULD ?

#24) Sec 5.5.1: r/from RFC 2407/from [RFC2407]
                 r/[RFC2407],/[RFC2407]),
                 r/ESP must also/ESP MUST also

#25) Sec 1.3: Add GSPD

#26) Sec 5.5.1.1.1: r/ RFC 5374/[RFC5374]

#27) Sec 5.5.1.1.1: r/must/MUST ?

#28) Sec 5.5.1.1.1: In the following, is 2119 language needed:

    SA TEK payload
    may be required in one or both directions.

#29) Sec 5.5.2: r/All Security Protocols must provide/All Security 
Protocols MUST provide

#30) Sec 5.6.1: r/The attributes must follow/The attributes MUST follow
                 r/At least one TEK must be/At least one TEK MUST be

#31) Sec 5.6.1.2: r/to the member./to the GM.

#32) Sec 5.6.2 and 5.6.3: r/The attributes must follow/The attributes 
MUST follow

#33) Sec 5.6.3 and 5.6.3.1: r/there must be only/there MUST be only

#34) Sec 5.6.3.1 and 5.6.3.2: r/Must/MUST

#35) Sec 1.3: add SSIV

#36) Sec 5.7: r/Must/MUST
               r/must/MUST X3

#37) Sec 5.9: r/must be deleted, they must be/are to be deleted, they 
MUST be

#38) Sec 10: References for 2403, 2404, 2407-2409, 3447, 4106, 4543, 
4754, 4868, 5903 are normative according to the following: 
http://www.ietf.org/iesg/statement/normative-informative.html

Assuming you delete App A, remove references to 4330 and 5996.

Please ensure the classification of the other references are in 
accordance with the IESG statement.  If you've got any questions, let me 
know.  I don't want to find out later that some informative reference to 
an informative RFC is actually a normative reference and we need to do a 
2nd IETF LC to note the DOWNREF.

I still need to finish Section 7 and 8.

spt

On 3/31/11 12:14 PM, Vincent Roca wrote:
> Hello everybody,
>
> As you probably noticed the GDOI draft has been significantly
> updated WRT the previous 07 version. For details see:
>
> http://tools.ietf.org/rfcdiff?url2=draft-ietf-msec-gdoi-update-08.txt
>
> This situation motivates another WGLC. So please review
>
> draft-ietf-msec-gdoi-update-08
> http://datatracker.ietf.org/doc/draft-ietf-msec-gdoi-update/
>
> and send comments to the WG mailing list by April 18th.
> (NB: if ever this deadline is too close, then tell me...)
>
> Regards,
>
> Vincent
> _______________________________________________
> MSEC mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/msec
>

---

From: Yoav Nir <[email-masked]>
Date: Wed, 13 Apr 2011 23:15:08 +0200

It looks ready to me.

---

From: Brian Weis <[email-masked]>
Date: Mon, 18 Apr 2011 10:14:02 -0700

Hi Sean,

Thanks for the comprehensive review ... good points, all. I've comments on a few of them inline below.

On Apr 2, 2011, at 10:09 AM, Sean Turner wrote:

> #1) Add "updates: 3547 (once approved)" to the header.  This will make it match the intro/abstract.  I can almost guarantee a discuss from somebody on this.
> 
> #2) ISAKMP Phase 1 is described in Section 2.1 as "one such mechanism".  This isn't a MUST support.  As far as I can tell there isn't a MUST implement Phase 1 protocol (the protocols in Appendix A are also just examples).  But, to implement this document you've got to do things as described in RFC 2407-2409.  It seems like this doc and IKEv1 are pretty well intertwined.  And, the g-ikev2 draft doesn't use these formats it uses something else...Let's be clear about what it's being used with. It seems like this section ought to be changed as follows:
> 
> OLD:
> 
> Delete the 2.1 header and renumber 2.1.1 and 2.1.2 to 2.1 and 2.2, respectively.
> 
> I would also delete Appendix A.  The currently proposed IKEv2 mechanism isn't reusing these fields and the KINK option never got defined.

Removing this text in Section 2 and Appendix A make a lot of sense. The concerns that caused them to be added to RFC 3547 have probably dissolved.

> 
> #3) Sec 3.2: r/Hashes are computed in the manner described within RFC 2409./Hashes are computed in the manner described within [RFC2409].
> 

These need to be reworded for clarity.

first must: "Before a GM contacts the GCKS, it must determine the group identifier and acceptable Phase 1 policy via an out-of-band method." This is less of a requirement than stating a pre-requisite. We propose replacing "must" with "needs to".

second must/should: "If the SEQ payload is present, the sequence number in the SEQ payload must be checked against any previously received sequence number for this group. If it is less than the previously received number, it should be considered stale and ignored." The requirement is actually this: "If the SEQ payload is present, the sequence number included in the SEQ payload MUST be greater than any previously received sequence number. If it is less than the previously received number, it MUST be considered stale and ignored."

> #10) Sec 5.2.1: r/SA Attributes payloads must be/SA Attributes payloads MUST be
> 

The "should" ought to be a "MUST" to ensure interoperability. "Value specifying a port associated with the source Id. A value of zero means that the SRC ID Port field MUST be ignored."

> 
> #13) Sec 5.3.1:
> 
> r/ attributes may be present / attributes MAY be present  or change to say they are OPTIONAL.
> 

OK, some cleanup is in order. We propose removing heading 5.3.1 to make the attribute definition part of 5.3, which is more natural. Then there needs to be a description of what attributes are required, with the rest being OPTIONAL (i.e., included if the group policy has included it).

How about:

  o KEK Attributes -- Contains KEK policy attributes associated with
   the group.  The following attributes may be present in a SAK Payload.
   The attributes must follow the format defined in ISAKMP (Section 3.3
   of [RFC2408]).  In the table, attributes that are defined as TV are
   marked as Basic (B); attributes that are defined as TLV are marked as
   Variable (V).

                ID Class                   Value    Type
                --------                   -----    ----
                RESERVED                     0
                KEK_MANAGEMENT_ALGORITHM     1        B
                KEK_ALGORITHM                2        B
                KEK_KEY_LENGTH               3        B
                KEK_KEY_LIFETIME             4        V
                SIG_HASH_ALGORITHM           5        B
                SIG_ALGORITHM                6        B
                SIG_KEY_LENGTH               7        B
                RESERVED                     8        B
                Standards Action            9-127
                Private Use               128-255
                Unassigned                256-32767

   The KEK_ALGORITHM, SIG_ALGORITHM attributes MUST be included; others
   are OPTIONAL, and included depending on group policy.  The
   KEK_MANAGEMENT_ALGORITHM attribute MUST NOT be included in a
   GROUPKEY-PULL message, and MUST be ignored if present.

> What happens if it's included in the PUSH message?

See above.

> 
> #15) Just noted to instances of GROUPKEY_.  Aren't they supposed to be GROUPKEY-*?
> 
> #18) Sec 5.3.6: r/and SIG_HASH_ALGORITHM is not required to be present in a SAK Payload/and SIG_HASH_ALGORITHM is OPTIONAL in a SAK Payload
> 

Not that we know of, but we thought this was esteemed to be better than PKCS 1.5 and so the protocol should support it. Is that not the current thinking?a

> #21) Sec 5.4.1: r/ Section 4.2.1 of RFC 5374/Section 4.2.1 of [RFC5374]

The second should is advice. How does this sound? "The values of ATD and DTD are independent. However, the most effective policy will have the DTD value to be the larger value as this allows new SAs to be activated before older SAs are deactivated."

> #23) Sec 5.5.1: Protocol, SEC ID Port, DST ID Prot, DST ID Port bullets r/should/SHOULD ?

These "should"s ought to be "MUST"s to ensure interoperability. E.g., "Value specifying a port associated with the source Id. A value of zero means that the SRC ID Port field MUST be ignored."
> #27) Sec 5.5.1.1.1: r/must/MUST ?

This is less of a requirement than an explanation. How about:

   Applications use the extensions in [RFC5374] to copy the IP addresses
   into the outer IP header when encapsulating an IP packet as an IPsec
   tunnel mode packet.  This allows an IP multicast packet to continue
   to be routed as a IP multicast packet.  This attribute also provides
   the necessary policy so that the GDOI group member can appropriately
   set up the GSPD.

> #29) Sec 5.5.2: r/All Security Protocols must provide/All Security Protocols MUST provide
> 
> #32) Sec 5.6.2 and 5.6.3: r/The attributes must follow/The attributes MUST follow
> 
> #37) Sec 5.9: r/must be deleted, they must be/are to be deleted, they MUST be
> 
> #38) Sec 10: References for 2403, 2404, 2407-2409, 3447, 4106, 4543, 4754, 4868, 5903 are normative according to the following: http://www.ietf.org/iesg/statement/normative-informative.html

Thanks for the pointer to the definitive statement ... I'll re-evaluate all of the Informative references against it.

> Please ensure the classification of the other references are in accordance with the IESG statement.  If you've got any questions, let me know.  I don't want to find out later that some informative reference to an informative RFC is actually a normative reference and we need to do a 2nd IETF LC to note the DOWNREF.
> 

Looking forward to this review, thanks.

Brian

-- 
Brian Weis
Security Standards and Technology, SRTG, Cisco Systems
Telephone: +1 408 526 4796
Email: [email-masked]

---

From: Sean Turner <[email-masked]>
Date: Mon, 18 Apr 2011 16:22:43 -0400

Brian,

The changes you suggest look okay to me.

As for RSA-PSS, yes the current advice is that it's "better" BUT it 
requires some interesting things like:
  - Parameters: With RSA v1.5 the parameters are NULL. With RSA-PSS 
they're are a bunch things you need to specify: hash algorithm, mask 
generation algorithm, salt length, and trailer field.
  - Certificate: Your certificate is going to be different because you 
need to carry these parameters.  Not sure any CA is actually issuing 
RSA-PSS certificates.

I think people are just going to skip it and go straight to ECDSA.  I 
guess it's okay to leave it in there for completeness because there's no 
requirement to support it.

I'll get on to the last sections today.

spt

---

From: Sean Turner <[email-masked]>
Date: Mon, 18 Apr 2011 16:54:55 -0400

Here's the rest of my comments on Sections 7 and 8 (all in 8):

Section 5.3.7 shows TBD-6 to 9 but the second paragraph in 8.1 only 
discussed TBD-6.  The others should be added in 8.1.

Section 5.6 shows TBD-7 and so does Section 8.1, but there's already a 
TBD-7 in Section 5.3.7.  Maybe renumber the one in Section 5.6 TBD-10?

Section 8.2: In the new registries, you need to say what needs to be 
done to update the registries (e.g., specification required, standards 
action).

spt

On 4/2/11 1:09 PM, Sean Turner wrote:
> #1) Add "updates: 3547 (once approved)" to the header. This will make it
> match the intro/abstract. I can almost guarantee a discuss from somebody
> on this.
>
> uses something else...Let's be clear about what it's being used with. It
> seems like this section ought to be changed as follows:
>
> OLD:
>

---

From: Sean Turner <[email-masked]>
Date: Mon, 18 Apr 2011 17:24:46 -0400

On 4/18/11 5:09 PM, Brian Weis wrote:
> Hi Sean,
>
> On Apr 18, 2011, at 1:22 PM, Sean Turner wrote:
>
>> Brian,
>>
>> As for RSA-PSS, yes the current advice is that it's "better" BUT it requires some interesting things like:
>> - Parameters: With RSA v1.5 the parameters are NULL. With RSA-PSS they're are a bunch things you need to specify: hash algorithm, mask generation algorithm, salt length, and trailer field.
>
> Thanks for the clarification. But if there are "a bunch of things you need to specify", then to get an interoperable solution I guess we'd need to specify them. :-) If that's the case, I'd rather take it out (given the lack of current RSS-PSS usage).

To be fair there's a set of defaults defined in the standard so if you 
were going to add RSA-PSS then you'd need to point to those defaults. 
If nobody is asking for it though ....

> Thanks,
> Brian
>
>> - Certificate: Your certificate is going to be different because you need to carry these parameters.  Not sure any CA is actually issuing RSA-PSS certificates.
>>
>> I think people are just going to skip it and go straight to ECDSA.  I guess it's okay to leave it in there for completeness because there's no requirement to support it.
>>

---

From: Brian Weis <[email-masked]>
Date: Mon, 18 Apr 2011 14:09:20 -0700

Hi Sean,

Thanks,
Brian

---

From: Brian Weis <[email-masked]>
Date: Mon, 18 Apr 2011 14:49:03 -0700

On Apr 18, 2011, at 2:24 PM, Sean Turner wrote:

> To be fair there's a set of defaults defined in the standard so if you were going to add RSA-PSS then you'd need to point to those defaults. If nobody is asking for it though ....

OK ... sounds like more work than its worth, and as no one is asking for it I propose to remove it as you originally suggested.

Thanks,
Brian

---

From: Brian Weis <[email-masked]>
Date: Mon, 18 Apr 2011 15:12:20 -0700

Hi Sean,

On Apr 18, 2011, at 1:54 PM, Sean Turner wrote:

> Section 5.3.7 shows TBD-6 to 9 but the second paragraph in 8.1 only discussed TBD-6.  The others should be added in 8.1.
> 
> Section 5.6 shows TBD-7 and so does Section 8.1, but there's already a TBD-7 in Section 5.3.7.  Maybe renumber the one in Section 5.6 TBD-10?

I also noticed these issues when removing RSA-PSS. The TBD-7 in Section 5.3.7 became TBD-6, and the rest retain their original numbering. Section 8.1 was also brought up to date.

> Section 8.2: In the new registries, you need to say what needs to be done to update the registries (e.g., specification required, standards action).

Vincent also has pointed this out. But Section 8 that says:

   "When the new
   registries are added, the following terms are to be applied as
   described in the Guidelines for Writing an IANA Considerations
   Section in RFCs [RFC5226]: Standards Action, and Private Use."

From RFC 5226:

      Standards Action - Values are assigned only for Standards Track
            RFCs approved by the IESG.
      Private Use: Private use only (not assigned), as described in Section 4.1.

I'm not sure how to be state this more clearly, but it seems as if that's needed. Suggestions are welcome.

Thanks,
Brian

---

From: Sean Turner <[email-masked]>
Date: Tue, 19 Apr 2011 08:42:57 -0400

Brian,

The only thing I can think to do is to move the text from 8 to 8.2 and 
repeat it for each of the new registries.

spt