Subject: Re: [OPSAWG] Roman Danyliw's Discuss on draft-ietf-opsawg-tacacs-yang-10: (with DISCUSS and COMMENT)

From: "Wubo (lana)" <[email-masked]>
Date: Tue, 27 Apr 2021 07:14:19 +0000

Hi Roman, all,

Thank you all for the review and discussion. The new revision is just posted based on the comments, please see the diff:
https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg-tacacs-yang-11

Thanks,
Bo

-----邮件原件-----
发件人: OPSAWG [mailto:[email-masked]] 代表 Rob Wilton (rwilton)
发送时间: 2021年4月20日 17:19
收件人: Roman Danyliw <[email-masked]>; The IESG <[email-masked]>
抄送: [email-masked]; [email-masked]; [email-masked]
主题: Re: [OPSAWG] Roman Danyliw's Discuss on draft-ietf-opsawg-tacacs-yang-10: (with DISCUSS and COMMENT)

Hi Roman,

Thanks for the review.   I have chipped in a couple of comments on your discuss concerns below.

> -----Original Message-----
> From: iesg <[email-masked]> On Behalf Of Roman Danyliw via 
> Datatracker
> Sent: 19 April 2021 23:08
> To: The IESG <[email-masked]>
> Cc: [email-masked]; Joe Clarke (jclarke) <[email-masked]>; opsawg- 
> [email-masked]; [email-masked]
> Subject: Roman Danyliw's Discuss on draft-ietf-opsawg-tacacs-yang-10:
> (with DISCUSS and COMMENT)
> 
> Roman Danyliw has entered the following ballot position for
> draft-ietf-opsawg-tacacs-yang-10: Discuss
> 
> When responding, please keep the subject line intact and reply to all 
> email addresses included in the To and CC lines. (Feel free to cut 
> this introductory paragraph, however.)
> 
> 
> Please refer to 
> https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-opsawg-tacacs-yang/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> ** RFC8907 was published with informational status and it contained 
> substantial caution in its security considerations that the protocol 
> was fundamentally insecure and would not “meet modern-day 
> requirements.”  This measured approach was taken to provide a stable 
> description of a widely deployed protocol and to serve as the basis 
> for future improvements.
> 
> The context for this follow-on, seemingly related work does not track 
> the situation around RFC8907 (as I remember it).  Specifically:
> 
> -- this functionality is new, and is not documenting the “as is” 
> deployed state

My understanding is that RFC8907 defines the current TACACS+ protocol as it stands, and that this draft is intended to document the YANG configuration data model that goes alongside RFC8907, and that these two documents act as a starting point for any future TACACS+ work in IETF.

> 
> -- this functionality is advocating for supporting an insecure 
> approach with proposed standard (rather than informational) status
[RW] 

The proposed standard status really comes from the fact that it is documenting a formal API (i.e., the YANG data model), and that this document should reflect IETF consensus on the data model for how TACACS+ is managed in YANG.  I also believe that the intention is that we can build on RFC8907 and this draft to improve the security aspects of the TACACS+ protocol over time and having the base YANG model draft being standards track is probably better to build on.

As a side note, a question from the routing ADs came up regarding what the status of documents containing YANG Modules should be (in that case for YANG models documenting experimental RFCs).  Perhaps it would help for me to write up some guidelines and to discuss those with the IESG?

> 
> ** Is this document intentionally breaking backward compatibility on the
> “shared-secret” size specified in RFC8907?
> 
> (a) Section 4.
>           case shared-secret {
>             leaf shared-secret {
>               type string {
>                 length "16..max";
>               }
> 
> (b) Section 10.5.1 of RFC8907 says “TACACS+ server administrators SHOULD
> configure secret keys of a minimum of 16 characters in length.”
> 
> As (b) is not a MUST (a “secret key” shorter than 16 is possible), it
> would
> appear that (a) breaks compatibility.
[RW] 

Section 10.5.1 of RFC8907 also states:

   *  TACACS+ clients SHOULD NOT allow servers to be configured without
      a shared secret key or shared key that is less than 16 characters
      long.

Given that an implementation could use a YANG deviation to allow shorter shared-secrets, I think that having the standard IETF YANG model require shared keys to be at least 16 characters is the right choice.

If clients were currently using shared keys of less than 16 characters then they would need to change to longer secrets, but the YANG data model is defining a new API anyway, so a certain amount of migration would be expected anyway.

Does this help with your discuss concerns?

Regards,
Rob

> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thank you to Yaron Sheffer for the SECDIR review, and for the changes in
> response.
> 
> ** Section 1.  Per “The System Management Model is augmented with TACACS+
> YANG
> module … as an alternative to … local user configuration”, is “local user
> configuration” that same as  the “user authentication model” noted earlier
> in
> the text?  If so, it would be helpful to make that clearer.
> 
> ** Section 4.  choice encryption. Per the name of this YANG item and the
> description (“Encryption mechanism between TACACS+ client and server”),
> please
> follow the convention of Section 4.5 of RFC8907 of calling this
> “obfuscation”.
> 
> ** Section 5.
> /system/tacacsplus/server:  This list contains the data nodes used to
>       control the TACACS+ servers used by the device.  Unauthorized
>       access to this list could cause a complete control over the device
>       by pointing to a compromised TACACS+ server.
> 
> Additional, outcomes also seem to be that modification of the counters
> could be
> used to hide attacks against the client
> 
> ** Additional nits:
> -- Section 4. Nit. s/TACAS server is providing/TACAS+ server is providing/
> 
> -- Section 5.  Wrong Section.  s/see Section 9 of TACACS+ Protocol
> [RFC8907]/
> see Section 10 of TACACS+ Protocol [RFC8907]/
> 
> 

_______________________________________________
OPSAWG mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/opsawg

---

From: "Joe Clarke (jclarke)" <[email-masked]>
Date: Tue, 27 Apr 2021 14:48:53 +0000

Thanks, Bo.  On your description for the renamed "security" choice, I
think it is overly descriptive in this case.  When a new secure T+
protocol comes out, this future proofing language will be obsolete. 
What about:

choice security {
  ...
  description
    "Security mechanism between TACACS+ client and server.";
  ...
    leaf shared-secret {
      ...
      description
        "...Note this security mechanism is best described as
'obfuscation' and not 'encryption' as it does not provide any meaningful
integrity, privacy, or reply protection.";

Thoughts from others?

Joe

---

From: tom petch <[email-masked]>
Date: Tue, 27 Apr 2021 15:57:32 +0000

From: OPSAWG <[email-masked]> on behalf of Joe Clarke (jclarke) <jclarke=[email-masked]>
Sent: 27 April 2021 15:48

choice security {
  ...
  description

Thoughts from others?

<tp>

Under descriptive!  I think that you need to explain why this is a choice rather then expecting the reader to know the BC/NBC rules off pat at the time of writing.  Perhaps

'This is modelled as a YANG 'choice' so that it can be augmented by a future YANG module in a backwards compatible manner.'

Note I say 'can be' not 'will be'

Tom Petch
Joe

---

From: "Joe Clarke (jclarke)" <[email-masked]>
Date: Tue, 27 Apr 2021 16:30:08 +0000

> Under descriptive!  I think that you need to explain why this is a choice rather then expecting the reader to know the BC/NBC rules off pat at the time of writing.  Perhaps

I'm not convinced that they need to know that in the module itself (vs.
the document reference), but I accept more context is needed.

> Note I say 'can be' not 'will be'

Maybe drop the word "future".

Joe

---

From: tom petch <[email-masked]>
Date: Wed, 28 Apr 2021 15:24:35 +0000

From: Joe Clarke (jclarke) <[email-masked]>
Sent: 27 April 2021 17:30

<tp>
Yes dropping 'future' is fine by me but I do think that this is better in the module.  As ever, the module is extracted from the RFC and floats off by itself so the module has to make sense on its own.

Tom Petch

Joe