On Fri, Jan 22, 2021 at 7:30 AM Daniel Migault <mglt.i...@gmail.com> wrote:
>
> Hi,
>
> I apology for responding so late - I missed the thread. I want this document 
> to be moved forward but so far I do not have the impression my concerns have 
> been addressed. I suppose that results from my lake of responsiveness and I 
> apology. Please find my response inline and let me know what can be achieved 
> reasonably.
>
> Yours,
> Daniel
>
>
> On Tue, Oct 27, 2020 at 11:34 PM Sean Turner <s...@sn3rd.com> 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 
>> > <nore...@ietf.org> 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.
>
> <mglt>
> maybe we could add these are still considered as secured at the time of 
> writing.  It is also tempting to add that given we deprecate sha1 and md5 in 
> the signature, it is tempting to suggest to use unless necessary hmac-sha256. 
>  I have commented the PR12
> </mglt>
>>
>> <
>> > [...]
>> >
>> > 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?
>>
> <mglt>
> The reason I would maybe have preferred to have indications for the client 
> and the server is that it is always easier for a server to say that it is 
> following the specs than to indicate that the client is not.
> This is correct the result is similar, but client usually complement servers 
> and we usually specify both. I believe that would be better to be updated.
> </mglt>
>>
>> > 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.
>>
> <mglt>
> Let's say that there are some softwares using SHA-1 / MD5 that will be 
> upgraded. I would have probably incline to put MUST NOT... but SHOULD NOT 
> carries the message, and I do not believe that is worth further discussion.
> </mglt>
>>
>> > 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.
>
> <mglt>
> sure. My point here was that Certificate MUST be signed by the signature, 
> hash provided by the extension. This mandates the CAs deprecates SHA1 as 
> well, and I am unclear if that is a correct assumption. I think this could be 
> addressed in a section or note related to Certificate.
> </mglt>
>>

Daniel, do you mean this ?

https://cabforum.org/2014/10/16/ballot-118-sha-1-sunset/


>>
>> > 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.
>>
> <mglt>
> I see your point and agree.
> </mglt>
>
>
>>
>> > 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?
>
>
> <mglt>
> yes. I have the impression that since SHA1/MD5 MUST NOT be mentioned in the 
> "signature_algorithms", this assumes that CAs do not sign using these 
> algorithms. I tend to believe that worth being mentioned. As mentioned 
> before, I think that could be mentioned in the draft.
> </mglt>
>>
>>
>> > 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.
>>
> <mglt>
> Updating the update works. It believe that saying a section should be remove 
> is not too hard, and it seems to me that mentioning the default behaviour is 
> important. I would say that could get implementers confused to implement part 
> of the specifications that do not hold any more. I would prefer to have this 
> being addressed.
>
> I am reading RFC7525 as recommending a non default parameter while this 
> document removed the support of default parameters. So to me the updating the 
> status of the default parameters seems more updating the 5246 then 7525.
> </mglt>
>
>> > 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.
>>
>
> <mglt>
> I agree we may be able to find all the dots. I think this provides more 
> insight to clarify the reading of 5246. I agree the intend is clearly stated 
> in the title and section 2 addresses most of it and I believe that some 
> flexibility is permitted, but that is unclear to me where to put the bar. I 
> still believe that could be easily be addressed.
> </mglt>
>
>>
>> > 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.
>
>
> <mglt>
> yes, but the current update only RECOMMENDs RFC7525.
> </mglt>
>>
>>
>> > 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.
>>
> <mglt>
> I suppose I proposed to move RECOMMENDED to MUST to accomplish the transition 
> as I do not see RECOMMENDED sufficient to guarantee interoperability. At that 
> point, I am inclined to say the MUST status is achieve as there are quite few 
> hash functions deployed and available and that the life time of TLS 1.2 is 
> expected to be limited. This could be made RECOMMENDED acceptable, but MUST 
> would be preferred if possible. Is there anything I am missing or any reason 
> to favour RECOMMENDED over MUST ?
> </mglt>
>
>> > 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).
>>
> <mglt>
> yes. My understanding so far is that the document deprecate SHA-1 and MD5 for 
> TLS 1.3 not for TLS 1.2 for the IANA section.
> </mglt>
>
>> > 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?
>>
> <mglt>
> yes, my point was to indicate that currently "N" deprecates the TLS 1.2 
> legacy protocol for TLS 1.3 as opposed to the the protocols for TLS 1.2. 
> Unless I misinterpreted the IANA registries I did not have the impression 
> that the signature scheme replaced the registries of TLS 1.2. It is possible 
> I am missing something with the IANA registries, but otherwise, I think the 
> draft should be updated.
> </mglt>
>>
>> spt
>> _______________________________________________
>> TLS mailing list
>> TLS@ietf.org
>> https://www.ietf.org/mailman/listinfo/tls
>
>
>
> --
> Daniel Migault
> Ericsson

_______________________________________________
TLS mailing list
TLS@ietf.org
https://www.ietf.org/mailman/listinfo/tls

Reply via email to