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

From: Larry Zhu <[email-masked]>
Date: Fri, 17 Dec 2010 04:16:34 +0000

While preparing the PROTO write-up for draft-ietf-krb-wg-otp-preauth-13, I have the following comments:

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.
2) Nits: section 3.2 page 9, ditto s/standard/classic/.
3) Nits: section3.2, page 9, can you please upper case KDC in "kdc-authentication", as "KDC-authentication".
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.
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/.
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?
7) Nits: section 3.4, page 14, s/standard encrypted timestamp/classic encrypted timestamp/.
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?
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?
10) Section 4.1,page 30,  for consistency please add a ',' at the end of "-pin-not-required(4)'.
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/.
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)" .
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?
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/
15) Appendix b4, page 39, s/proceed as with the standard sequence/proceed as with the classic sequence as defined in RFC4120/.
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.
17) I have the opinion that the reference to PKINIT and anonymity draft should be normative. Do you have good reasons otherwise?
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. 
19) draft-ietf-keyprov-pskc has been published as RFC 6030.
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?

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.

Thanks,
--Larry

_______________________________________________
ietf-krb-wg mailing list
[email-masked]
https://lists.anl.gov/mailman/listinfo/ietf-krb-wg

---

From: <[email-masked]>
Date: Fri, 17 Dec 2010 09:41:00 -0500

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."

> 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

> 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.
>

---

From: Larry Zhu <[email-masked]>
Date: Fri, 17 Dec 2010 22:36:12 +0000

> ".. 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 i…
I like this better too.

> 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?

> 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.
I can guess that is the case without your clarification. I raised the question thinking that the existing text sounds a bit abrupt, but if you think it is already clear, you can leave it as is. This can be your decision unless the group has a different opinion otherwise.

> 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

How about the following text, the specific issue is already fixed in anonymity-12, by adding the 
KDC contribution. Do you agree?

  However, the current anonymous PKINIT proposal is open to man-
  in-the-middle attacks since the attacker can choose a session key
   such that the session key between the MITM and the real KDC is the
   same as the session key between the client and the MITM.

> 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.

If so, we have a real issue here. The text before the following (copied right below here for convenience) describes how it should fail with an error when the PIN expires, but now you are saying it should succeed for the PIN-change service (which is basically what RFC3244 does as I alluded) but how that is described in this document? Are we on the same page?

   If the PIN change is to be handled by a PIN-change service then it is
   assumed that authentication to that service will succeed if the PIN
   has expired as described above

> The ASN.1 module was checked using OSS ASN.1 Syntax Checker Version 8.1.1.
That is all I wanted to know. Thanks.

  >     /* MUST be a valid URI in accordance with IETF RFC 2396 */ 

Exactly right. My aging compiler does not under this syntax.

> Done
Please also update the corresponding portion in appendix A.

To summarize, issue 13, 16 and 18 in the original list are not closed yet at this point. Thanks.

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] 
Sent: Friday, December 17, 2010 6:41 AM
To: Larry Zhu; [email-masked]
Subject: RE: draft-ietf-krb-wg-otp-preauth-13 PROTO write-up comments

Larry,

--Gareth

> 1) Nits: Section 1.2 page 4, s/the standard Reply Key/the classic 
> Reply Key (as defined in RFC4120)/. The word "standard"
> 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.

> 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?

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?

> 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?

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?

	 	

> 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

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.

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

> 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.

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?
> 

    AnyURI ::= UTF8String
       (CONSTRAINED BY { 

> 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

---

From: <[email-masked]>
Date: Wed, 22 Dec 2010 04:34:49 -0500

>
> Gareth wrote:
> > ".. 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."
> I like this better too.
>
I have updated sections 3.2 and 3.4.

> account. This
> > 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.
> I can guess that is the case without your clarification. I
> raised the question thinking that the existing text sounds a
> bit abrupt, but if you think it is already clear, you can
> leave it as is. This can be your decision unless the group
> has a different opinion otherwise.
>
If this text is unclear then I would rather clarrify it.
How about some text of the form:

   The KDC MAY also include the new PIN's expiration time
   and the expiration time of the OTP account within the last-req field
   of the PA-OTP-PIN-CHANGE.  (These fields can be used by the KDC to
   handle cases where then account related to the user's OTP token
   has a different expiration time to the user's Kerberos account.)

> session key
> How about the following text, the specific issue is already
> fixed in anonymity-12, by adding the KDC contribution. Do you agree?
>
This is OK with me.

> > 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.
>
> If so, we have a real issue here. The text before the
> following (copied right below here for convenience) describes
> how it should fail with an error when the PIN expires, but
> now you are saying it should succeed for the PIN-change
> service (which is basically what RFC3244 does as I alluded)
> but how that is described in this document? Are we on the same page?
>

The sentence above was meant to mean that if the user's PIN has expired that the KDC should fail the current AS-REQ and return a KRB-ERROR of type KDC_ERR_PIN_EXPIRED but that any subsequent request to the KDC for a ticket for a PIN-change service should succeed.  That is, the fact that the user's OTP PIN has expired should not prevent authentication to the KDC for a PIN-change service ticket from succeeding.

> Exactly right. My aging compiler does not under this syntax.

Love kindly pointed out that I somehow put C-style comments in the ASN.1 (which my compiler didn't object to)

I believe that the correct text should be

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

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

> not closed yet at this point. Thanks.
I will wait for the outcome on issue 18 on the list before doing any update.  Apart from a possible clarification for issue 16, I think all the others have been resolved.

> not have
> 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.
>
> 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
>
> account. This
> 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.
>
> return the
> 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.
>
> > 10) Section 4.1,page 30,  for consistency please add a ','
> at the end
> > of "-pin-not-required(4)'.
>
> Done
>
> the client
> session key
> Reply Key
> 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.
>
> 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.
>
> salt SHALL
> see a few
> 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.
>
>
> due to WGLC
> this time