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 <sean@xxxxxxxxx> 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 <noreply@xxxxxxxx> 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 -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
