Re: [TLS] Iotdir last call review of draft-ietf-tls-md5-sha1-deprecate-04

[Sean Turner <[e-mail]>]

Daniel,

Alessandro created a PR to resolve your comments as suggested by me:
https://github.com/tlswg/draft-ietf-tls-md5-sha1-deprecate/pull/12
I was unable to propose text for all of your comments. Please review this email as well as the PR as well so we can move this I-D along.

Cheers,
spt

> On Oct 27, 2020, at 23:32, Sean Turner <[e-mail]> wrote:
> 
> 
> Please note the comment about Section 3 suggests changing server behavior from SHOULD NOT to a MUST NOT.
> 
>> On Oct 27, 2020, at 10:19, Daniel Migault via Datatracker <[e-mail]> wrote:
>> 
>> Reviewer: Daniel Migault
>> Review result: Ready with Nits
>> 
>> Hi,
>> 
>> 
>> I reviewed this document as part of the IoT 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.  
>> 
>> Review Results: Ready with Nits
>> 
>> Please find my comments below.
>> 
>> Yours,
>> Daniel
>> 
>> 
>>        Deprecating MD5 and SHA-1 signature hashes in TLS 1.2
>>                 draft-ietf-tls-md5-sha1-deprecate-04
>> [...]
>> 
>> 1.  Introduction
>> 
>>  The usage of MD5 and SHA-1 for signature hashing in TLS 1.2 is
>>  specified in [RFC5246].  MD5 and SHA-1 have been proven to be
>>  insecure, subject to collision attacks [Wang].  In 2011, [RFC6151]
>>  detailed the security considerations, including collision attacks for
>>  MD5.  NIST formally deprecated use of SHA-1 in 2011
>>  [NISTSP800-131A-R2] and disallowed its use for digital signatures at
>>  the end of 2013, based on both the Wang, et. al, attack and the
>>  potential for brute-force attack.  In 2016, researchers from INRIA
>>  identified a new class of transcript collision attacks on TLS (and
>>  other protocols) that rely on efficient collision-finding algorithms
>>  on the underlying hash constructions [Transcript-Collision].
>>  Further, in 2017, researchers from Google and CWI Amsterdam
>>  [SHA-1-Collision] proved SHA-1 collision attacks were practical.
>>  This document updates [RFC5246] and [RFC7525] in such a way that MD5
>>  and SHA-1 MUST NOT be used for digital signatures.  However, this
>>  document does not deprecate SHA-1 in HMAC for record protection.
>> 
>> <mglt>
>> RFC6194 may be mentioned as a reference for
>> not deprecating HMAC-SHA-1 as well as an
>> additional reference to [NISTSP800-131A-R2]. 
> 
> Are asking for something like this:
> 
> OLD:
> 
>  In 2011, [RFC6151] detailed the security considerations,
>  including collision attacks for MD5.
> 
> NEW:
> 
>  In 2011, [RFC6151] [RFC6194] detailed the security considerations,
>  including collision attacks for MD5 and SHA-1, respectively.
> 
>> Reading the text the situation of HMAC with
>> MD5 is unclear. Since we specify that SHA-1
>> is not deprecated for HMAC we may specify
>> the status for HMAC with MD5. Given RFC6151 I
>> hope the reason is that MD5 and HMAC-MD5 has
>> already been deprecated but I have not found
>> this. Maybe that would worth mentioning it
>> is deprecated already.
>> 
>> </mglt>
> 
> Are you asking for something like this:
> 
> OLD:
> 
>  However, this document does not deprecate SHA-1 in HMAC
>  for record protection.
> 
>  However, this  document does not deprecate MD-5 or SHA-1 HMAC
>  for record protection.
> 
>> [...]
>> 
>> 2.  Signature Algorithms
>> 
>>  Clients MUST NOT include MD5 and SHA-1 in the signature_algorithms
>>  extension.  If a client does not send a signature_algorithms
>>  extension, then the server MUST abort the handshake and send a
>>  handshake_failure alert, except when digital signatures are not used
>>  (for example, when using PSK ciphers).
>> 
>> <mglt>
>> It seems to me that the server behavior might
>> be defined as well. In our case this could be
>> something around the lines the server MUST
>> ignore MD5 and SHA1 values in the signature
>> algorithm extension. 
>> 
>> </mglt>
> 
> I guess that would be the way to absolutely stamp them out, but don’t we get the same result because the client is not sending the values in the signaure_algorithms extension?
> 
>> 3.  Certificate Request
>> 
>>  Servers SHOULD NOT include MD5 and SHA-1 in CertificateRequest
>>  messages.
>> 
>> <mglt>
>> It seems to me that the same level of
>> authentication should be provided for both
>> peers and that server MUST NOT  include MD5
>> or SHA-1.
>> 
>> A SHOULD NOT status might be welcome for a
>> smooth transition. At that time, collision
>> for MD5 and SHA1 are known for years. It is likely
>> that software that still need MD5 or SHA1 are
>> likely to never upgrade, so I doubt a smooth
>> path worth being taken. 
>> </mglt>
> 
> This has been a SHOULD NOT since it was a first published as an individual draft and through the WGLC. I would not feel comfortable changing it now without further discussion.
> 
> I tend to think it is okay to leave as SHOULD NOT because the server MUST use values from the now ever present signature_algorithms extension and MD5 and SHA1 are not going to be there. If the server is going to go nuts and include MD5 and SHA-1 in the CertifiateRequest even though it’s not been asked, well, you got bigger problems.
> 
>> 4.  Server Key Exchange
>> 
>>  Servers MUST NOT include MD5 and SHA-1 in ServerKeyExchange messages.
>>  If a client receives a MD5 or SHA-1 signature in a ServerKeyExchange
>>  message it MUST abort the connection with the illegal_parameter
>>  alert.
>> 
>> <mglt>
>> As per section 2, the client has clearly
>> indicated it does not support signature with
>> MD5/SHA1, so Server Key Exchange should not
>> end up with signature with SHA1/MD5. 
>> 
>> """
>> If the client has offered the "signature_algorithms" extension, the
>>  signature algorithm and hash algorithm MUST be a pair listed in that
>>  extension. 
>> """
>> 
>> It also seems to me that the constraint of
>> including a MD5 and SHA-1 signature is
>> related to the Certificate. I suspect that
>> some clarification are needed here.  
> 
> It’s about the digitally-signed struct for the dhe_dss and dhe_rsa cases in ServerKeyExchange.
> 
>> Since the case where the extension becomes
>> mandatory, the quoted text above of RFC 5246
>> might be updated as well, though this does
>> not appear that necessary.
> 
> So we might do it, but the question is whether implementers are going to be confused if we don’t update it.  I tend to think that the changes in s2 are clear that the extension will be present (except when sigs are not used) if the handshake is to complete.
> 
>> </mglt>
> 
> Not sure anything needs to be changed in this section based on the above.
> 
>> 5.  Certificate Verify
>> 
>>  Clients MUST NOT include MD5 and SHA-1 in CertificateVerify messages.
>>  If a server receives a CertificateVerify message with MD5 or SHA-1 it
>>  MUST abort the connection with handshake_failure or
>>  insufficient_security alert.
>> 
>> 
>> <mglt>
>> 
>> 6. Certificate 
>> 
>> Unless I am missing something, it seems to me
>> that signature may also be found in the
>> Certificate messages for the chain as well in
>> the restriction of the signature algorithm.
>> The end certificate is associated to the peer
>> while other certificate are related to a CA. 
>> 
>> It seems that client and server behavior may
>> be specified. The quoted text below may be
>> helpful to clarify. 
>> 
>> """
>> If the client provided a "signature_algorithms" extension, then all
>>  certificates provided by the server MUST be signed by a
>>  hash/signature algorithm pair that appears in that extension.
>> """
>> 
>> </mglt>
> 
> Are you suggesting that a new section be added to address the Certificate message?
> 
>> 6.  Updates to RFC5246
>> 
>>  [RFC5246], The Transport Layer Security (TLS) Protocol Version 1.2,
>>  suggests that implementations can assume support for MD5 and SHA-1 by
>>  their peer.  This update changes the suggestion to assume support for
>>  SHA-256 instead, due to MD5 and SHA-1 being deprecated.
>> 
>>  In Section 7.4.1.4.1: the text should be revised from:
>> 
>>  OLD:
>> 
>>  "Note: this is a change from TLS 1.1 where there are no explicit
>>  rules, but as a practical matter one can assume that the peer
>>  supports MD5 and SHA- 1."
>> 
>>  NEW:
>> 
>>  "Note: This is a change from TLS 1.1 where there are no explicit
>>  rules, but as a practical matter one can assume that the peer
>>  supports SHA-256."
>> 
>> 
>> <mglt>
>> I am reading the Note as an explanation on
>> why sha was taken as the default hash
>> function with the following rules. 
>> 
>> """
>> If the client does not send the signature_algorithms extension, the
>>  server MUST do the following:
>> 
>>  -  If the negotiated key exchange algorithm is one of (RSA, DHE_RSA,
>>     DH_RSA, RSA_PSK, ECDH_RSA, ECDHE_RSA), behave as if client had
>>     sent the value {sha1,rsa}.
>> 
>>  -  If the negotiated key exchange algorithm is one of (DHE_DSS,
>>     DH_DSS), behave as if the client had sent the value {sha1,dsa}.
>> 
>>  -  If the negotiated key exchange algorithm is one of (ECDH_ECDSA,
>>     ECDHE_ECDSA), behave as if the client had sent value {sha1,ecdsa}.
>> """
>> 
>> The current document does not update the
>> default hash function from sha to sha256 to
>> avoid interoperability issue where one peer
>> takes sha while the other one takes sha-256.
> 
> You are right that this section, which is updating TLS1.2 [RFC5246], is not changing the default to SHA-256, but the recommendations for configuring TLS 1.2, which are provided in RFC 7525/BCP 195, is being updated to recommend SHA-256 in the very next section.
> 
>> As a results, these rules and the "Note" may
>> eventually all together be replaced by your
>> text of section 2. 
>> 
>> The following text may also be removed:
>> 
>> """
>> If the client supports only the default hash and signature algorithms
>>  (listed in this section), it MAY omit the signature_algorithms
>>  extension.
>> """
> 
> So we might do it, but the question is whether implementers are going to be confused if we don’t do it. I think because s1 already says that client MUST send a signature_algorithms extension that we don’t have to indicate that that particular sentence be dropped/removed from the draft. I will admit this document mixes the two ways to do a bis document, i.e., old/new and describing blanket changes, but I think the intent is pretty clear based on the brevity of the draft.
> 
>> Regarding the Note, it seems to be that the
>> removal of support for MD5/SHA1 will result
>> in interoperability issues. At this point,
>> the issue is due to the obsolescence of the
>> implementation as deprecation of SHA1/Md5 has
>> started a long time ago. 
>> 
>> It is unclear to me how normative is
>> interpreted "can assume". Was the support of
>> MD5/SHA1 a SHOULD or a MUST? In both case, if
>> we were willing to maintain interoperability
>> between software that only implemented
>> MD5/SHA1, we should take a slower path and
>> introducing SHA-256 and having were MD5/SHA1
>> kept for interoperability purpose before
>> being deprecated. I do not think we should
>> take that path as implementations that
>> currently do not support SHA-256 are unlikely
>> to be updated and that deprecation of
>> SHA1/MD5 has started a long time ago. 
>> 
>> I would however mention the issue of
>> interoperability in the  section but not in
>> the text to update. In the text to update I
>> would maybe suggest that the support of
>> SHA-256 comes with a normative MUST
>> statement. 
>> 
>> 
>> </mglt>
> 
> I think we can accomplish migrating to SHA-256 by updating RFC 7525/BCP 195.
> 
>> Velvindron, et al.       Expires April 12, 2021                 [Page 3]
>> 
>> Internet-Draft      draft-ietf-tls-md5-sha1-deprecate       October 2020
>> 
>> 
>> 7.  Updates to RFC7525
>> 
>>  [RFC7525], Recommendations for Secure Use of Transport Layer Security
>>  (TLS) and Datagram Transport Layer Security (DTLS) recommends use of
>>  SHA-256 as a minimum requirement.  This update moves the minimum
>>  recommendation to use stronger language deprecating use of both SHA-1
>>  and MD5.  The prior text did not explicitly include MD5 or SHA-1; and
>>  this text adds guidance to ensure that these algorithms have been
>>  deprecated..
>> 
>>  Section 4.3:
>> 
>>  OLD:
>> 
>>  When using RSA, servers SHOULD authenticate using certificates with
>>  at least a 2048-bit modulus for the public key.  In addition, the use
>>  of the SHA-256 hash algorithm is RECOMMENDED (see [CAB-Baseline] for
>>  more details).  Clients SHOULD indicate to servers that they request
>>  SHA-256, by using the "Signature Algorithms" extension defined in TLS
>>  1.2.
>> 
>>  NEW:
>> 
>>  Servers SHOULD authenticate using certificates with at least a
>>  2048-bit modulus for the public key.
>> 
>>  In addition, the use of the SHA-256 hash algorithm is RECOMMENDED;
>>  and SHA-1 or MD5 MUST NOT be used (see [CAB-Baseline] for more
>>  details).  Clients MUST indicate to servers that they request SHA-
>>  256, by using the "Signature Algorithms" extension defined in TLS
>>  1.2.
>> 
>> <mglt>
>> I understand the reason we do specify that
>> hash algorithms that MUST NOT been used. This
>> is fine in the context of this document, but
>> it seems to me that if we were writing the
>> updated specification we may have rather
>> mentioned a minimum level of security hash
>> function needs to be met - in our case
>> SHA-256. I leave the co-authors make the
>> appropriated choice.   
>> 
>> </mglt>
> 
> Can you clarify what you would like changed? I am just confused because SHA-256 is RECOMMENDED in the proposed new text.
> 
>> 8.  IANA Considerations
>> 
>>  The document updates the "TLS SignatureScheme" registry to change the
>>  recommended status of SHA-1 based signature schemes to N (not
>>  recommended) as defined by [RFC8447].  The following entries are to
>>  be updated:
>> 
>>      +--------+----------------+-------------+-------------------+
>>      | Value  |  Description   | Recommended |     Reference     |
>>      +--------+----------------+-------------+-------------------+
>>      | 0x0201 | rsa_pkcs1_sha1 |      N      | [RFC8446][RFCTBD] |
>>      | 0x0203 |   ecdsa_sha1   |      N      | [RFC8446][RFCTBD] |
>>      +--------+----------------+-------------+-------------------+
>> 
>>  Other entries of the resgistry remain the same.
>> 
>> 
>> <mglt>
>> It seems to me that TLS 1.2 is using the TLS
>> hash and TLS signature registry TLS signature
>> registry and TLS 1.3 is using Signature
>> Scheme. 
>> 
>> I suspect that TLS hash values for sha1 and
>> md5 should be deprecated. And RFCTBD should
>> be added for sha1 and md5. Note that the 
>> SHOULD NOT status for CertificateRequest
>> may have prevented such deprecation. 
> 
> The TLS HashAlgorithm and TLS SignatureAlgorithm registries do not have a Recommended column. Likewise, there’s not a notes column. What I think we could do is replace the reference to [RFC5246] with [RFCTBD] (so it’s points to this document when it is published).
> 
>> A side effect is these code points for
>> signature scheme that were assigned for
>> compatibility with legacy (TLS 1.2)
>> signatures must not be used anymore -  if
>> there are no more valid with TLS 1.2. 
>> </mglt>
> 
> This is what changing the Recommended to “N” is above so I think we’re good here?
> 
> spt