Re: [Ietf-krb-wg] draft-ietf-krb-wg-otp-preauth-13 PROTO write-up comments

Larry,

Thanks for the review, my responses are below.

--Gareth

> 1) Nits: Section 1.2 page 4, s/the standard Reply Key/the 
> classic Reply Key (as defined in RFC4120)/. The word "standard" 
> is overloaded in this context though the same phrase would sound 
> perfectly fine anywhere else. This is just a suggestion and the 
> same applies to the rest of my comments with regarding to the 
> word "classic" vs "standard.

I am happy to change standard to classic.

> 2) Nits: section 3.2 page 9, ditto s/standard/classic/.
OK, I changed "standard shared-key" to "classic shared-key"

> 3) Nits: section3.2, page 9, can you please upper case KDC 
> in "kdc-authentication", as "KDC-authentication".
Done

> 4) Grammar: section 3.2, page 9, s/PA-OTP-CHALLENGE that do
> not have the set the/ PA-OTP-CHALLENGE that do not have the/, 
> i.e. extra "the set" seems to be just a copy&paste error.

Done

> 5) Nits: section 3.3 page 11, at the very end, 
> s/if the algorithm identifiers do not conform/if 
> none of the algorithm identifiers conforms/.

OK, done

> 6) Section 3.4, page 13. If kdc-authentication is required 
> then a PA-OTP-REQUEST containing an otp-value must be 
> rejected. This discusses the KDC side of verifying the 
> preauth-data, how does the KDC know if KDC-authentication 
> is required. Isn't that KDC-authentication is a client 
> side thing?

Yes, I think this is a mistake.  The following sentence from section 3.2 covers the fact that the client SHOULD NOT return send the OTP value to the client if local policy does not permit it.

   If the "must-encrypt-nonce" flag is set in the otp-keyInfo then the
   OTP value MUST be used to generate the Client Key and Reply Key as
   described in Section 3.6 and MUST NOT be included in the otp-value
   field of the PA-OTP-REQUEST.  If the flag is not set then the OTP
   value MUST be included in the otp-value field of the PA-OTP-REQUEST
   and MUST NOT be used in the key derivation.  In this case, the Client
   Key and Reply Key SHALL be the same as the Armor Key as described in
   Section 3.6 and so if the returning of OTP values in this way does
   not conform to local policy on the client then it SHOULD NOT use the
   token for authentication.

I would think that this policy would also cover KDC-authentication and so maybe the last sentence should be modified as follows:

".. And so if the returning of OTP values in this way does not conform to local policy on the client (for example, if KDC-Authentication is required and is not being provided by other means) then it SHOULD NOT use the token for authentication."

> 7) Nits: section 3.4, page 14, s/standard encrypted 
> timestamp/classic encrypted timestamp/.

Done

> 8) Section 3.4, page 14, the expiration time of the OTP 
> account. This text introduces a new concept "OTP account". 
> Does this imply a separate life-time management is required 
> or recommended for OTP usage? Can you please clarify?

These were added to cover the possibility that the account related to the user's OTP token and the their OTP PIN could have a different expiration times from their Kerberos account.
For example, if the OTP token is managed by a separate server then that server could have its own account management distinct from that of the KDC.

> 9) Section 3.4, page 14, the current text says "it SHOULD 
> return the same response as for a non-OTP expired password", 
> first, what does it mean by "non-OTP expired password", does 
> it imply there may exist an OTP expired password, secondly, 
> and is it intentional that the KDC is not returning 
> KDC_ERR_PIN_EXPIRED in this case?

I believe that this was an oversight.  I updated section 2.3 to include the new KDC_ERR_PIN_EXPIRED but omitted to update section 3.4.  I suggest that the last sentence of this paragraph be reworded as follows.

   Finally, if the KDC determines that the
   user's PIN has expired then it SHOULD return a KRB-ERROR of type 
   KDC_ERR_PIN_EXPIRED as described in section 2.3

> 10) Section 4.1,page 30,  for consistency please add a ',' at 
> the end of "-pin-not-required(4)'.

Done

> 11) Section 4.1, page 18, s/both flags MUST NOT be set and 
> the client MUST regard/ both flags MUST NOT be set, or the 
> client MUST regard/.

Done

> 12) Section 4.3, page 23, for consistency, please remove the 
> comma at the end of "-mandatory(2)," so that it looks like 
> "--mandatory(2)" .

Done

> 13) Section 6.1, this section explains MITM can choose the 
> session key when anonymous PKINIT is used. That is no longer 
> true with anonymity draft 12 (the latest). Can this be 
> updated? I believe Sam raised this issue during the last WGLC 
> on -12. Can you please summarize the discussion and the conclusion?

Sam raised the following question regarding this section during the review of draft 12

	> I'm a bit confused about the concern where a host key is compromised.
	> It's basically always possible for a client and an attacker to work
	> together to spoof the KDC if the attacker knows the host key.  This
	> tends to be one of the least serious concerns that happens 
	> when the host
	> key is compromised.	
	>

This text  was added following an issue raised by Greg Hudson on 28 July 2009

	On Tue, 2009-07-28 at 13:27 -0400, Sam Hartman wrote:
	> This makes sense to me.
	> So, S/KEY with host armor would be OK to allow?

	I think it's worth documenting that it would allow someone with
      the host key to fool the client, whereas a straight RFC 4120 AS-REQ does
      not allow this.

	It's not a terribly interesting vulnerability because a
	compromised host key normally means a compromised host, which generally
	gives you more latitude to mess with the user's activities than
	impersonating the KDC does.

> 14) Grammar: appendix b4, s/The Client generates Client Key 
> Reply Key as described in/ The Client generates the Client 
> Key and the Reply Key as described in/

Done

> 15) Appendix b4, page 39, s/proceed as with the standard 
> sequence/proceed as with the classic sequence as defined in RFC4120/.

Done

> 16) Section 2.3, on top of page 7,  "as described above". I 
> believe what was described above was removed. You can say 
> "similarly as illustrated in RFC3244" and add a non-normative 
> reference to RFC3244.

The "as described above" is actually new text in draft 13.  I believe that it was meant to describe the handling of expired PINs in the previous paragraph and can simply be removed.

> 17) I have the opinion that the reference to PKINIT and 
> anonymity draft should be normative. Do you have good reasons 
> otherwise?

I don't have any particular reasons to keep them as informational, I just didn't think that they were used in a normative manner.  I've moved them to the normative section.

> 18) Section 3.6, page 16, the current text says that "the 
> salt SHALL be the default salt for the principal requested in 
> the AS-REQ". I do not think "default salt for the principal" 
> is defined in existing standards or in this document. I can 
> see a few possibilities to address this issue. A) we might 
> need to have a protocol change, for example, we can add the 
> optional "salt" field of KerberosString type in the OTP 
> challenge message and allow the client to retry if it got it 
> wrong when the two-pass variant; B) you can simply say that 
> the salt is an empty string if the OTP value is already 
> salted when computed from a secret; C) go back to draft 12 
> and I do agree that approach in -12 can create an unnecessary 
> pain point. 

What was meant by this text was that the salt used would be the default salt defined for the enctype and principal.  So for example, for AES it would be a value derived from the principals name.

> 19) draft-ietf-keyprov-pskc has been published as RFC 6030.

OK, updated.

> 20) I was able to compile the ASN module in appendix A except 
> the latest 3 lines before the "end", ignoring the blank line 
> right before that. Can you please confirm this module passes 
> a modern ASN1 compiler?
> 

The ASN.1 module was checked using OSS ASN.1 Syntax Checker Version 8.1.1.

Are you having trouble with the following:

    AnyURI ::= UTF8String
       (CONSTRAINED BY { 
       /* MUST be a valid URI in accordance with IETF RFC 2396 */ 
       }) 

> Draft 13 contains significant changes from draft 12 mostly 
> due to WGLC comments. The quality of the document has been 
> significantly improved. Out of these, issue 18 must be 
> addressed before the document can move forward but overall 
> the document is very close. In order to allow the document 
> progress quickly, I just started another WGLC on 13 together 
> with these comments. Taking into account of the holidays at 
> this time of the year, I have extended the last call to cover 
> 3 weeks instead of the normal 2 week length.
> 
_______________________________________________
ietf-krb-wg mailing list
ietf-krb-wg@lists.anl.gov
https://lists.anl.gov/mailman/listinfo/ietf-krb-wg