Re: [TLS] AD review of draft-ietf-tls-oldversions-deprecate-06

[Benjamin Kaduk <[e-mail]>]

Hi Kathleen,

Also inline.

On Wed, Aug 12, 2020 at 04:29:56PM -0400, Kathleen Moriarty wrote:
> Hi Ben,
> 
> Thanks for your review.  Some initial responses are inline.
> 
> On Sun, Jul 26, 2020 at 5:22 PM Benjamin Kaduk <[e-mail]> wrote:
> 
> > Thanks for putting together the -06 based on my preliminary comments, and
> > my apologies for taking so long to get back to it.  It turns out that going
> > through the 80-odd documents we update takes a while!
> >
> >
> > I have a bunch of suggestions that are basically editorial, that I'll
> > make a pull request for (along with suggested changes for several of the
> > following comments).  It's up at:
> > https://github.com/tlswg/oldversions-deprecate/pull/3
> 
> 
> Thanks for that!
> 
> >
> >
> > We mention in the Abstract (but not Introduction!) that this document
> > moves 2246 and 4346 (but not 6347!) to the historic state.
> > Unfortunately, this is slightly problematic from a process perspective,
> > since the current way to make things Historic
> > (
> > https://www.ietf.org/about/groups/iesg/statements/designating-rfcs-historic-2014-07-20
> > )
> > requires a separate "status change document" that gets its own IETF LC,
> > to effectuate the status change.  Most references to the status change
> > document can be replaced by this RFC after it's published, but formally
> > the move to historic is *not* done by this document.  I've pulled into
> > my pull request the language used in RFC 8429 to describe moves to
> > Historic; please make sure to reproduce that language in the
> > Introduction as well as the Abstract.
> >
> 
> Do you need us to submit a draft to request that status change?

No -- it's done purely in the datatracker without an I-D, so I will plan to
do that.  (Unfortunately, it has to be one status-change document per
status-change, so it'll be a lot of clicking for me, but that's life.)

> >
> > I found three documents (3656, 4540, 7562) in the list of update targets
> > that are on the ISE (not IETF) stream.  I had talked to Adrian during my
> > preliminary review, and in principle it is permissible to make those
> > updates as part of this document, but we will need specific ISE approval
> > to do so.  I am supposed to sync up with him during IETF LC, but the
> > document needs to be updated to clarify that the updates of ISE
> > documents are/will be done with permission of the ISE.  (Please also try
> > to double-check that the list is complete; I didn't find an
> > authoritative list of "all documents on the ISE stream" to grep against,
> > and I didn't try to have something parse rfc-index.xml to output such a
> > list.
> >
> 
> OK, so you'd like a list added and that's not in your pull request, is that
> right?  We'll figure it out. Thanks in advance with the coordination with
> Adrian.

That's correct, this is not in my pull request (not least because that list
of three documents is incomplete -- I sent a more-likely-complete list of 6
documents in an off-list follow-up.
https://www.rfc-editor.org/search/rfc_search_detail.php?stream_name=Independent&page=All
will get a (presumably authoritative) list of ISE-stream documents, FWIW.

> 
> >
> > I note that in addition to our BCP 195 update (called out in Section 6),
> > we also update 3552, which is BCP 72.  This update is quite incidental
> > (compared to our BCP 195 update), so it seems clear that having this
> > document be part of BCP 195 is correct.
> >
> 
> Are you asking here to include an update to RFC3552?   Thanks.

No change needed here, I think -- just noting that we already list RFC 3552
as being updated, but that we don't need to be part of BCP 72, since the
update in question is largely incidental.

> >
> > Section 1.1
> >
> > I went through all 83 of the references in the big list, that are
> > supposed to be ones that "normatively reference TLS 1.0/1.1 or DTLS 1.0,
> > as well as the shorter list of already-obsoleted documents.
> >
> > I won't bore you with my full notes, but there are some particular
> > things that stood out from the review:
> >
> > - We have a separate list of updates for documents that are already
> >   obsolete (but don't say much about why we're going go the extra
> >   bother).  However, three of the documents in our main list of updates
> >   (4743, 4744, and 6460) are already Historic, and presumably should be
> >   treated more like the already-obsolete ones.
> >
> 
> Obsolete does not mean the same thing as deprecate though.  TLSv1.2 has
> been obsoleted by TLSv1.3, but not deprecated.  The deprecation goes the
> extra step to say not to use it and it triggers many to begin phase out
> plans.  Am I misunderstanding the question?

I think you're misunderstanding the question, yes, sorry.

I think we want the documents that are Historic to be listed separately
from the other ("regular") updates, in a manner akin to what is already
done for the documents that are currently obsolete.  Or, perhaps, to say
that there is no point in deprecating something that is already historic,
and not bother updating those three documents, but it seems okay to keep
the current status with a comprehensive list of updates.

> 
> >
> > - Several documents (e.g., 8261, 5023) specifically have MUST-level
> >   requirements for 1.0 (or 1.1) that disappear or become internally
> >   inconsistent with our current update, so we might want to fill that
> >   void.
> >
> 
> I'm not sure what you mean here.  This draft will update the ones listed.
> RFC8261 just has the following:
> "The DTLS implementation MUST support DTLS 1.0 [RFC4347
> <https://tools.ietf.org/html/rfc4347>] and SHOULD
> 
>    support the most recently published version of DTLS, which was DTLS
> 
>    1.2 [RFC6347 <https://tools.ietf.org/html/rfc6347>] when this RFC was
> published. "
> 
> This deprecates DTLS1.0.  Are you asking for us to add a MUST support
> DTLS1.2?

It's not clear that there's need to put in a MUST-level requirement. I was
envisioning something more like "some of the documents being updated in
this manner had specific requirements to support, implement, and/or use a
specific (D)TLS version; it is expected that implementations adopting the
updates made by this document will cause (D)TLS 1.2 to fulril that role
instead".  Or maybe that goes without saying, and there's no need to add
more text; I don't have a strong opinion.

> For RFC5023, there's the following text:
> "At a minimum, client and server
> 
>    implementations MUST be capable of being configured to use HTTP Basic
>    Authentication [RFC2617 <https://tools.ietf.org/html/rfc2617>] in
> conjunction with a connection made with
>    TLS 1.0 [RFC2246 <https://tools.ietf.org/html/rfc2246>] or a
> subsequent standards-track version of TLS
>    (such as [RFC4346 <https://tools.ietf.org/html/rfc4346>]),
> supporting the conventions for using HTTP over
> 
>    TLS described in [RFC2818 <https://tools.ietf.org/html/rfc2818>]."
> 
> This requires a lot of updates, but in terms of the text for TLS as it
> pertains to this draft, there's already text that says "or a subsequent
> standards-track version".  With the document being updated to deprecate the
> older versions, I think we're okay.  If you have something specific in mind
> or seeing something that I am not seeing, please advise.

I am definitely *not* proposing to add specific per-document updates to
each document in this category!  So we're looking at something generic like
my suggestion above, or nothing at all, I think.
> 
> > - A few documents (6749, 6750, 6739) make an interesting claim about TLS
> >   1.0 being the "most widely deployed version" and providing the
> >   "broadest interoperability".
> >
> 
> OK, so that was from a point in time and I think readers will understand
> that.  Are you asking for -bis updates?  I think deprecating is fine, we've
> worked with the community to see the numbers.  If it's a -bis that will
> help, let's separate that out.  If you'd like specific updates, please
> advise.  Thanks.

I don't have specific updates/text changes in mind here, it just seemed
notable as a particular mention of the now-obsolete TLS versions.

> 
> > - Many documents note specific MTI ciphers (largely just the MTI ciphers
> >   from the respective TLS version).  Since all ciphers defined for
> >   1.0/1.1 also work in 1.2 (with the exception of the IDEA and DES
> >   ciphers from RFC 5469, that it claims "have been removed from TLS
> >   version 1.2"), these requirements in theory are still in effect, even
> >   though those particular ciphers are not great and the badness of those
> >   ciphers is a lot of the justification for deprecating the old protocol
> >   versions.  I don't really think that a piecemeal effort to identify
> >   and update each specific MTI cipher mention is worth the effort,
> >   though it may be worth noting that TLS 1.2 has
> >   TLS_RSA_WITH_AES_128_CBC_SHA as MTI and that it is preferred over the
> >   older MTI ciphers.  (Actually, it looks like a few of the specific MTI
> >   mentions in the documents we're updating are already
> >   TLS_RSA_WITH_AES_128_CBC_SHA, hooray.)
> >
> 
> RFC7525 helps as well and the -bis of that to set TLSv1.2 best practices.
> Are you asking for any changes here? Thanks.

My take is no changes here, but leaving an opportunity for other opinions
to appear (if they exist).

> 
> > - As implied by the previous bullet point, the IDEA and DES ciphers
> >   specified by RFC 5469 are now entirely obsolete, which means that that
> >   entire document is also obsolete (or maybe historic?).  It seems like
> >   the right thing to do to effectuate that change via this document.
> >
> 
> Yes, good catch.  It does make sense to deprecate RFC5469 with this draft
> after looking at the introduction and purpose of the document.

In light of
https://www.ietf.org/about/groups/iesg/statements/designating-rfcs-historic-2014-07-20/
I'm inclined to say we should run the "move to historic" procedure for this
one.

> 
> >
> > - Similarly, the downgrade protection provided by the SCSV of RFC 7507
> >   seems to be entirely obsolete.  Any implementation that is compliant
> >   with this document will support only 1.2 or higher.  If it only
> >   supports one version, no downgrade is possible; if it also supports
> >   1.3 or newer, the new downgrade-detection mechanism defined by TLS 1.3
> >   applies, so the SCSV mechanism is entirely redundant (i.e., obsolete).
> >   We could effectuate that status change in this document as well.
> >
> 
> Has this been addressed in RFC8446?  If not, the specific downgrade
> examples are just listed as examples.  If a gap is left, then the full
> document should not be deprecated and made obsolete.  The text infers other
> versions in my read.  I have not looked to see if this was addressed in
> RFC8446 yet though.

I'd really like to get a few more people to weigh in on this one -- IIRC
David Benjamin and Martin Thomson had mentioned some thoughts in the chat
during the session at 108, and Ekr as author of 8446 would be expected to
have a good sense of what it does.  

The specific RFC 8446 mechanism here is described at
https://tools.ietf.org/html/rfc8446#section-4.1.3 : "TLS 1.3 has a
downgrade protection mechanism embedded in the server's random value.
[...]"

While the RFC 8446 mechanism has the client do the actual detection of
downgrade, there's a MUST-level requirement on clients to make the check,
so from a specification point of view the check can be treated as reliable.
The RFC 7507 mechanism has the server do the detection, but I think the end
result is still the same: in an "downgraded" exchange between two honest
participants, the handshake fails and the downgrade is detected.

Since the functionality is still useful, just superseded, this one seems
like a better fit for "obsoletes" (vs. "historic).

> Thanks.
> 
> >
> > - We list RFC 8465 as being updated, but it doesn't seem to say anything
> >   at all about TLS or reference the relevant RFCs.
> >
> 
> OK, I played with the numbers and RFC 8645 does mention TLS, but is
> specific to present versions.  I think it's safe to remove this from the
> list.  Thanks for catching that.

Okay, thanks for checking.

> 
> >
> >
> >    [RFC6614] has a requirement for TLSv1.1 although only makes an
> >    informative reference to [RFC4346].
> >
> From the text, I think it's still important to deprecate the use of TLSv1.1
> in this document.  If you take a look at the connection establishment
> section (2.3)
> 
>    2.  after completing the TCP handshake, immediately negotiate TLS
>        sessions according to [RFC5246
> <https://tools.ietf.org/html/rfc5246>] or its predecessor TLS 1.1.
> The
>        following restrictions apply:
> 
>        *  Support for TLS v1.1 [RFC4346
> <https://tools.ietf.org/html/rfc4346>] or later (e.g., TLS 1.2
>           [RFC5246 <https://tools.ietf.org/html/rfc5246>]) is
> REQUIRED.  To prevent known attacks on TLS
>           versions prior to 1.1, implementations MUST NOT negotiate TLS
> 
>                       versions prior to 1.1.
> 
> I think it's important to have the updates and not miss the opportunity.
> Do you want specific text for this text as well?

[Er, this part was just me quoting a snippet from the draft so that I could
comment on it, below]

> 
> > The text in question seems to be:
> >
> > %     *  Support for TLS v1.1 [RFC4346] or later (e.g., TLS 1.2
> > %        [RFC5246]) is REQUIRED.  [...]
> >
> > And RFC 5246 is a normative reference for it.  So we may want to say
> > "has a requirement for TLSv1.1 or later, although only makes an
> > informative reference to [RFC4346].  This requirement is updated to be
> > for TLSv1.2 or later."
> >
> 
> Would you like specific text to update section 2.3 as well?
> I think just dropping the first clause and removing the full sentence of
> the second reference in 2.3 would work.

That would be okay with me, though my proposal was more limited in scope --
just noting that since we already have a sentence specific to RFC 6614's
updates, we could add a few more words to explain the apparent oddity,
e.g.,

OLD:
   [RFC6614] has a requirement for TLSv1.1 although only makes an
   informative reference to [RFC4346].

NEW:
   [RFC6614] has a requirement for TLSv1.1 or later, although only makes an
   informative reference to [RFC4346].

or possibly:
   [RFC6614] has a requirement for TLSv1.1 or later, although only makes an
   informative reference to [RFC4346], since [RFC5246] was the current
   version of TLS when RFC 6614 was published.


> 
> >
> >    This document updates DTLS [RFC6347].
> >
> > I note that in the thread from my preliminary review there was some
> > support for the sentiment that these changes do not formally Update
> > 6347, in that what 6347 says about version negotiation is still true; we
> > just tell you not to do it.
> > (https://mailarchive.ietf.org/arch/msg/tls/YUySQbMVJPv31Fwv68rEpcmBMsM/)
> > But, if we do keep the Updates: relationship, I think we want to say
> > slightly more, e.g., noting that the interoperation with DTLS 1.0 that
> > it describes is now forbidden.
> >
> 
> That's fine with me.
> 
> 
> >
> > Section 2
> >
> >    Specific details on attacks against TLSv1.0 and TLSv1.1 as well as
> >    their mitigations are provided in NIST SP800-52r2 [NIST800-52r2], RFC
> >    7457 [RFC7457] and other referenced RFCs.  Although the attacks have
> >
> > Are the "other referenced RFCs" referenced from 7457 or from this
> > document.  This document references ... quite a few RFCs, so either way
> > we should probably clarify.
> >
> 
> That text was from before the search to find the many RFCs that reference
> the deprecated versions.  Let's just drop "and other referenced RFCs" as
> those are good references.
>  Thanks for catching that.

That works for me; thanks for the background.

> 
> >
> >    been mitigated, if support is dropped for future library releases for
> >    these versions, it is unlikely attacks found going forward will be
> >    mitigated in older library releases.
> >
> > [The wording here is kind of convoluted; I've suggested an alternative
> > in the aforementioned pull request.]
> >
> 
> OK, thanks.
> 
> 
> >
> >    NIST for example have provided the following rationale, copied with
> >    permission from NIST SP800-52r2 [NIST800-52r2], section 1.2 "History
> >    of TLS" (with references changed for RFC formatting).
> 
> 
> > (The version of SP800-52r2 that we reference has been withdrawn.  We
> > should probably check that the quotation in question has not changed if
> > we update to reference a current version of the NIST document.)
> >
> 
> OK, here is the current and published version link:
> https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-52r2.pdf
> 
> The text appears in section 1.1 of the same name and it looks like it's
> just altered slightly towards the end.  Here's the text in the published
> version:
> 
> "TLS 1.1, specified in RFC 4346 [24], was developed to address weaknesses
> discovered in TLS 1.0, primarily in the areas of initialization vector
> selection and padding error processing. Initialization vectors were made
> explicit^4 to prevent a certain class of attacks on the Cipher
> 
> 4 The initialization vector (IV) must be sent; it cannot be derived from a
> state known by both parties, such as the previous message.
> 
> Block Chaining (CBC) mode of operation used by TLS. The handling of padding
> errors was altered to treat a padding error as a bad message authentication
> code rather than a decryption failure. In addition, the TLS 1.1 RFC
> acknowledges attacks on CBC mode that rely on the time to compute the
> message authentication code (MAC). The TLS 1.1 specification states that to
> defend against such attacks, an implementation must process records in the
> same manner regardless of whether padding errors exist. Further
> implementation considerations for CBC modes (which were not included in RFC
> 4346 [24]) are discussed in Section 3.3.2.
> 
> TLS 1.2, specified in RFC 5246 [25], made several cryptographic
> enhancements, particularly in the area of hash functions, with the ability
> to use or specify the SHA-2 family of algorithms for hash, MAC, and
> Pseudorandom Function (PRF) computations. TLS 1.2 also adds authenticated
> encryption with associated data (AEAD) cipher suites.
> 
> TLS 1.3, specified in RFC 8446 [57], represents a significant change to TLS
> that aims to address threats that have arisen over the years. Among the
> changes are a new handshake protocol, a new key derivation process that
> uses the HMAC-based Extract-and-Expand Key Derivation Function (HKDF) [37],
> and the removal of cipher suites that use RSA key transport or static
> DiffieHellman (DH) key exchanges, the CBC mode of operation, or SHA-1. Many
> extensions defined for use with TLS 1.2 and previous versions cannot be
> used with TLS 1.3. "

Seems like it should be safe to update the reference and quoted text
(though maybe not leave the footnote inline).

> 
> > Section 3
> >
> >    The integrity of both TLSv1.0 and TLSv1.1 depends on a running SHA-1
> >    hash of the exchanged messages.  This makes it possible to perform a
> >    downgrade attack on the handshake by an attacker able to perform 2^77
> >    operations, well below the acceptable modern security margin.
> >
> > The 2^77 number seems to be the pre-shattered.io number.
> >
> >    Similarly, the authentication of the handshake depends on signatures
> >    made using SHA-1 hash or a not stronger concatenation of MD-5 and
> >    SHA-1 hashes, allowing the attacker to impersonate a server when it
> >    is able to break the severely weakened SHA-1 hash.
> >
> > Having reviewed the WGLC thread and the papers linked therefrom, I'm
> > suggesting that we change this to "not appreciably stronger" (along with
> > adding the missing article to "using SHA-1 hash").  In particular,
> > https://www.iacr.org/archive/asiacrypt2009/59120136/59120136.pdf
> > suggests that MD5 is particularly weak, and that attacking SHA-1 will
> > continue to be the bulk of the work even as attacks on SHA-1 become more
> > efficient than O(2^58), so that the difference is mostly just the factor
> > of 64 for the Joux multicollision.
> >
> 
> There's a separate draft on SHA-1 and MD5, how would you like to see this
> handled?

My proposals here are pretty minor -- change 2^77 to 2^65, and "not
stronger" to "not appreciably stronger".

> 
> >
> > Section 4
> >
> >    TLSv1.0 MUST NOT be used.  Negotiation of TLSv1.0 from any version of
> >    TLS MUST NOT be permitted.
> >
> > (editorial) I'm not sure how much precedent there is for a construction
> > that talks about negotiating one version of TLS "from" some other
> > version of TLS.  In my head the negotiation is something that's done by
> > or from an implementation, rather than a version.  I don't want to
> > insist on anything here, but I probably would have written something
> > more like "negotiation of TLSv1.0 by any implementation of TLS MUST NOT
> > be permitted".  (And similarly for the following section.)
> >
> 
> The current text was written that was as it's common to see downgrade
> negotiation details in specs.  There's a subtle difference in what's there
> versus what's been proposed as a change.  The current text is stating that
> downgrades to these deprecated versions is not permitted.  I read your
> proposed text as stating the same thing as what is said in the first
> sentence.

Okay, feel free to ignore this comment, then.

> 
> >
> >    Any other version of TLS is more secure than TLSv1.0.  TLSv1.0 can be
> >    configured to prevent interception, though using the highest version
> >    available is preferable.
> >
> > This second sentence seems almost like it's undercutting the premise of
> > MUST NOT use.  Would it be better to say something like "While TLSv1.0
> > can be configured to prevent some types of interception, using the
> > highest version available is preferred"?  (Similarly for the following
> > section.)
> >
> 
> I'm not seeing a big difference in this nit.  I think this may be a
> difference only in the author's preference and regional preferences.

I'm happy to leave this alone for now.

> 
> >
> >    Historically, TLS specifications were not clear on what the record
> >    layer version number (TLSPlaintext.version) could contain when
> >    sending ClientHello.  Appendix E of [RFC5246] notes that
> >    TLSPlaintext.version could be selected to maximize interoperability,
> >    though no definitive value is identified as ideal.  That guidance is
> >    still applicable; therefore, TLS servers MUST accept any value
> >    {03,XX} (including {03,00}) as the record layer version number for
> >    ClientHello, but they MUST NOT negotiate TLSv1.0.
> >
> > This "MUST accept any value" seems redundant with Appendix D.2 of RFC
> > 8446, which is also giving guidance in this space (but we don't
> > currently link to).  (Similarly for the following section.)
> >
> 
> Yes, this text is  purposefully similar.  I don't see why we'd link to
> RFC8446.  Am I missing something?

There's no particular need to reference it; I was just noting that we had
the option of replacing most of this with "appendix D.2 of RFC 8446
requires that a server ignore the TLSPlaintext.legacy_record_version, which
includes {03,00}" if we wanted to avoid redundant requirements text.

> 
> > Section 5
> >
> > Should we also wrap the DTLSv1.0 bits in here?  (Or have a dedicated
> > section?)  We don't currently have specific statements like "DTLSv1.0
> > MUST NOT be used", "MUST NOT send a ClientHello with
> > ClientHello.client_version set to {254,255}", etc.
> >
> 
> I thought there was a reason, but will defer to Stephen on this.  I  think
> I had added the prior section and perhaps it was just because DTLS had not
> been added to this document yet, but I think there may have been more to it?
> 
> Stephen, do you have anything to add/change?
> 
> Thanks for the careful review to improve the document, Ben.

And thank you for the updates!

-Ben