Re: [OPSAWG] Secdir early review of draft-ietf-opsawg-tacacs-tls13-07

[Russ Housley]

Doug:

> Many thanks for the review, Russ!
>  
> Please see below the initial changes based upon your comments, hopefully they 
> have met the intent. Please advise if the updates are not addressing what you 
> had in mind, or for any concerns.
>  
> Best Regards,
>  
> The Authors.
>  
>  
> From: Russ Housley via Datatracker  >
> Date: Thursday, 25 April 2024 at 20:32
> To: sec...@ietf.org   >
> Cc: draft-ietf-opsawg-tacacs-tls13@ietf.org 
>  
>  >, opsawg@ietf.org 
>  mailto:opsawg@ietf.org>>
> Subject: Secdir early review of draft-ietf-opsawg-tacacs-tls13-07
> 
> Reviewer: Russ Housley
> Review result: Not Ready
> 
> I 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 primarily for the benefit of the Security Area
> Directors.  Document authors, document editors, and WG chairs should
> treat these comments just like any other IETF Last Call comments.
> 
> Document: draft-ietf-opsawg-tacacs-tls13-07
> Reviewer: Russ Housley
> Review Date: 2024-04-25
> IETF LC End Date: Unknown
> IESG Telechat date: Unknown
> 
> Summary: Has Issues
> 
> 
> Major Concerns:
> 
> Section 3.2.2 says:
> 
>Implementations MUST support certificate-based TLS authentication and
>certificate revocation bi-directionally for authentication, identity
>verification and policy purposes.  Certificate path verification as
>described in Section 3.2.2.1 MUST be supported.
> 
> I do not understand this paragraph.  I suggest that you handle four
> topics in separate sentences:
> 
>(1) certificate for the server (for server authentication),
>(2) revocation checking of the server certificate by the client,
>(3) certificate for the client (for client authentication),
>(4) revocation checking of the client certificate by the server.
> 
>  The “Policy” purposes is probably over-prescriptive, as 
> implementations are clearly free to apply policy to any attributes such as 
> identity and other cert attributes that that bubble up from the transport as 
> they see fit and so I’ll remove that.
> 
> Otherwise, the intent is that:
> 
> “Certificate based mutual TLS authentication MUST be supported by 
> implementations along with the provisions of revocation and path verification 
> from [RFC5280] as described in 3.2.2.1.”
> 
> Does that make sufficient sense?  We do have a canonical version:
> 
> “   Certificate based mutual authentication MUST be supported.
> 
>Clients MUST support certificate-based TLS authentication of the Peer 
> Server.  Clients MUST support certificate revocation.
> 
>Servers MUST support certificate-based TLS authentication of the Peer 
> Client.  Servers MUST support certificate revocation.
> 
>Certificate path verification as described in Section 3.2.2.1 MUST be 
> supported.”
> 
> If this structure is still preferred, or more info is really required, please 
> advise.
> 

To be a little less verbose, I suggest:

~~~
Certificate based mutual authentication MUST be supported.  Each peer MUST 
validate the certificate path of the remote peer, including revocation 
checking, as described in Section  3.2.2.1.
~~~

> Section 3.2.2 says: "Pre-Shared Keys (PSKs) [RFC4279] ...".  RFC 4279 is
> not appropriate for use with TLS 1.3, so it should not be cited here.
> 
> Agreed: Removed that citation.
> 

Thanks.
> Section 5.1.2 says: "... servers MAY abruptly disconnect clients that
> do."  I think this ought to make a stronger recommendation about 0-RTT.
> Other profiles for the use of TLS with specific protocols (like
> draft-ietf-netconf-over-tls13) completely forbid 0-RTT.
> 
>  
> 
> Agreed: changed to MUST.
> 

Thanks.
> Section 5.1.4 talks about loading certificate chains.  It might also
> talk about loading the information needed to perform revocation checks
> or the use of OCSP stapling.
> 
> Agreed. We have removed the part about loading the certs from 5.1.4 
> into 3.2.2.1, which is now augmented with extra requirements for revocation, 
> hopefully this should cover:
>  
> “Other approaches may be used for loading the intermediate certificates onto 
> the client, but MUST include support for revocation.
>  
> For example,  details the AIA (Authority Information 
> Access) extension to access information about the issuer of the certificate 
> in which the extension appears. It can be used to provide the address of the 
> Online Certificate Status Protocol (OCSP) responder from where revocation 
> status of the certificate can be checked.”
> 
> 
s/revocation/revocation checking/

Otherwise this look fine to me.
> Section 5.1.4 says: "Certificate fingerprints are another option."
> More explanation or a reference is needed here,
> 
> 
> This is removed along with the pr

Re: [OPSAWG] Secdir early review of draft-ietf-opsawg-tacacs-tls13-07

[Douglas Gash (dcmgash)]

Many thanks for the review, Russ!

Please see below the initial changes based upon your comments, hopefully they 
have met the intent. Please advise if the updates are not addressing what you 
had in mind, or for any concerns.

Best Regards,

The Authors.


From: Russ Housley via Datatracker 
Date: Thursday, 25 April 2024 at 20:32
To: sec...@ietf.org 
Cc: draft-ietf-opsawg-tacacs-tls13@ietf.org 
, opsawg@ietf.org 
Subject: Secdir early review of draft-ietf-opsawg-tacacs-tls13-07
Reviewer: Russ Housley
Review result: Not Ready

I 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 primarily for the benefit of the Security Area
Directors.  Document authors, document editors, and WG chairs should
treat these comments just like any other IETF Last Call comments.

Document: draft-ietf-opsawg-tacacs-tls13-07
Reviewer: Russ Housley
Review Date: 2024-04-25
IETF LC End Date: Unknown
IESG Telechat date: Unknown

Summary: Has Issues


Major Concerns:

Section 3.2.2 says:

   Implementations MUST support certificate-based TLS authentication and
   certificate revocation bi-directionally for authentication, identity
   verification and policy purposes.  Certificate path verification as
   described in Section 3.2.2.1 MUST be supported.

I do not understand this paragraph.  I suggest that you handle four
topics in separate sentences:

   (1) certificate for the server (for server authentication),
   (2) revocation checking of the server certificate by the client,
   (3) certificate for the client (for client authentication),
   (4) revocation checking of the client certificate by the server.
 The “Policy” purposes is probably over-prescriptive, as implementations 
are clearly free to apply policy to any attributes such as identity and other 
cert attributes that that bubble up from the transport as they see fit and so 
I’ll remove that.
Otherwise, the intent is that:
“Certificate based mutual TLS authentication MUST be supported by 
implementations along with the provisions of revocation and path verification 
from [RFC5280] as described in 3.2.2.1.”
Does that make sufficient sense?  We do have a canonical version:
“   Certificate based mutual authentication MUST be supported.
   Clients MUST support certificate-based TLS authentication of the Peer 
Server.  Clients MUST support certificate revocation.
   Servers MUST support certificate-based TLS authentication of the Peer 
Client.  Servers MUST support certificate revocation.
   Certificate path verification as described in Section 3.2.2.1 MUST be 
supported.”
If this structure is still preferred, or more info is really required, please 
advise.



Section 3.2.2 says: "Pre-Shared Keys (PSKs) [RFC4279] ...".  RFC 4279 is
not appropriate for use with TLS 1.3, so it should not be cited here.
Agreed: Removed that citation.


Section 5.1.2 says: "... servers MAY abruptly disconnect clients that
do."  I think this ought to make a stronger recommendation about 0-RTT.
Other profiles for the use of TLS with specific protocols (like
draft-ietf-netconf-over-tls13) completely forbid 0-RTT.

Agreed: changed to MUST.


Section 5.1.4 talks about loading certificate chains.  It might also
talk about loading the information needed to perform revocation checks
or the use of OCSP stapling.
Agreed. We have removed the part about loading the certs from 5.1.4 into 
3.2.2.1, which is now augmented with extra requirements for revocation, 
hopefully this should cover:

“Other approaches may be used for loading the intermediate certificates onto 
the client, but MUST include support for revocation.

For example,  details the AIA (Authority Information 
Access) extension to access information about the issuer of the certificate in 
which the extension appears. It can be used to provide the address of the 
Online Certificate Status Protocol (OCSP) responder from where revocation 
status of the certificate can be checked.”



Section 5.1.4 says: "Certificate fingerprints are another option."
More explanation or a reference is needed here,

This is removed along with the previous comment.


Minor Concerns:

The draft uses RFC 2119 terms, but it lacks a reference to RFC 2119
and the usual RFC 2119 boilerplate.


There was a rather hidden reference using the “referencegroup” mechanism, 
so it did appear in the references, Is this sufficient?



   [BCP14]Best Current Practice 14,

  https://www.rfc-editor.org/bcp/bcp14.txt.

  At the time of writing, this BCP comprises the following:



  Bradner, S., "Key words for use in RFCs to Indicate

  Requirement Levels", BCP 14, RFC 2119,

  DOI 10.17487/RFC2119, March 1997,

  https://www.rfc-editor.org/info/rfc2119.



  Leiba, B., "Ambiguity of Uppercase vs Lowercase in RFC

  2119 Key Words", BCP 14, RFC 8174, DOI 10.17487/RFC8174,




Nits:

[OPSAWG] Secdir early review of draft-ietf-opsawg-tacacs-tls13-07

[Russ Housley via Datatracker]

Reviewer: Russ Housley
Review result: Not Ready

I 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 primarily for the benefit of the Security Area
Directors.  Document authors, document editors, and WG chairs should
treat these comments just like any other IETF Last Call comments.

Document: draft-ietf-opsawg-tacacs-tls13-07
Reviewer: Russ Housley
Review Date: 2024-04-25
IETF LC End Date: Unknown
IESG Telechat date: Unknown

Summary: Has Issues


Major Concerns:

Section 3.2.2 says:

   Implementations MUST support certificate-based TLS authentication and
   certificate revocation bi-directionally for authentication, identity
   verification and policy purposes.  Certificate path verification as
   described in Section 3.2.2.1 MUST be supported.

I do not understand this paragraph.  I suggest that you handle four
topics in separate sentences:

   (1) certificate for the server (for server authentication),
   (2) revocation checking of the server certificate by the client,
   (3) certificate for the client (for client authentication),
   (4) revocation checking of the client certificate by the server.
   
Section 3.2.2 says: "Pre-Shared Keys (PSKs) [RFC4279] ...".  RFC 4279 is
not appropriate for use with TLS 1.3, so it should not be cited here.

Section 5.1.2 says: "... servers MAY abruptly disconnect clients that
do."  I think this ought to make a stronger recommendation about 0-RTT.
Other profiles for the use of TLS with specific protocols (like
draft-ietf-netconf-over-tls13) completely forbid 0-RTT.

Section 5.1.4 talks about loading certificate chains.  It might also
talk about loading the information needed to perform revocation checks
or the use of OCSP stapling.

Section 5.1.4 says: "Certificate fingerprints are another option."
More explanation or a reference is needed here,


Minor Concerns:

The draft uses RFC 2119 terms, but it lacks a reference to RFC 2119
and the usual RFC 2119 boilerplate.


Nits:

Section 3: Please add a reference for FIPS 140.

Section 3.2.2.1: s/Certificate Authority (CA)/Certification Authority (CA)/

Section 3.3: s/Certificate Provisioning/Certificate provisioning/

Section 5.1.3: s/abandoned/abandoned./

Section 5.1.4: s/Certificate Authority (CA)/Certification Authority (CA)/

IDnits complaints about:

 == The 'Updates: ' line in the draft header should list only the numbers
of the RFCs which will be updated by this document (if approved); it
should not include the word 'RFC' in the list.



___
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg