[regext] Benjamin Kaduk's Discuss on draft-ietf-regext-login-security-07: (with DISCUSS and COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-regext-login-security-07: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-regext-login-security/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

This document+extension claims to provide "Login Security" but has no
substantive discussion of why the previous mechanism was insecure and
how this extension improves the security.  I find it hard to believe
that any such discussion could fail to acknowledge that sending the
plaintext password (after only processing for whitespace) to the server
(akin to SASL PLAIN) is severely lacking on several security-related
fronts.  While it may be possible that there is adequate justification
for only pursuing the smallest incremental improvement in the current
document, it would require some additional discussion to convince me
that the "small incremental improvement" of removing a protocol-level
maximum password length is the best choice at this time, as opposed to a
broader approach that attempts to tackle more axes upon which "security"
can be measured.  Does this discussion already exist somewhere?
(This document also includes functionality for relatively rich event
notifications, that are likely worth doing in their own right, but do
not seem to be "security improvements" per se, to me.)

I also think that there many places in the description of the XML
elements/attributes that are underspecified, given that XML is
traditionally thought of as a machine-readable format.  Several
instances have already been noted by my fellow IESG members (e.g.,
custom events, statistical warnings, "value" attribute), but they seem
prevalent enough that I would like to see the authors make a pass
through all the protocol elements and assess which ones need to be
machine-readable vs. only for human consumption, and accurately document
that.  I list some examples in the Comment section, and specifically
call out the <loginSec:pw> and <loginSec:newPW> encodings, which leaves
many ambiguities with respect to what non-ASCII behavior is allowed
other than OpaqueString, what constitutes "whitespace" in the two listed
situations, and whether the password encoding is related to the XML
document encoding of the request.  As I note in the comment, my
understanding was that the PRECIS profiles were intended to be used at a
protocol-level (vs. a deployment level) and thus the nature of the
profile usage would be fairly tightly specified by this document;
perhaps the responsible AD (or someone else) will correct my
understanding.

I'd also like to have a bit of discussion regarding the prohibition of
using the literal string "[LOGIN-SECURITY]" as an actual password.
Section 3.2 notes '''[t]he server MUST NOT allow the client to set the
password to the value "[LOGIN-SECURITY]"''', and though I did not do a
full case-by-case analysis, this feels like something that a server
implementing this extension wants to do always, regardless of whether a
given client indicates support for this extension.  That seems like it
could meet the criteria to mark this document as Updating RFC 5730, to
reserve this sentinel value.  Is there more reasoning for or against
having this document Update the password-handling behavior RFC 5730 to
reserve this sentinel value?


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

(per the first Discuss point) I understand that per RFC 5734 we're
always using TLS client certificates to identify the client machine (but
can have multiple user credentials using the same client certificate for
their TLS connections), but that doesn't completely alleviate the
problems inherent to passwords (see Discuss).  For example, with
passwords there's the risk of password theft, whether from the on-disk
server password database or by leveraging a fraudulent TLS server
certificate to capture live incoming traffic, but that's not a complete
listing.

Section 1

(Side note: I note that the phrase "security event" is used as a term of
art by the SECEVENT WG, for the "security event token" work of RFC 8417
and its consumers.  They seem to have a somewhat different set of
expected "security events", not that this document necessarily needs to
change in response.)

(side note: IMHO, the long list of security-event attributes (and
possibly the security events themselves) is not needed in a high-level
Introduction like this.  But that's not an actionable comment.)

Section 2

   Servers which implement this extension SHOULD provide a way for
   clients to progressively update their implementations when a new
   version of the extension is deployed.

   Servers SHOULD (for a temporary migration period up to server policy)
   provide support for older versions of the extension in parallel to
   the newest version, and allow clients to select their preferred
   version via the <svcExtension> element of the <login> command.

I see the opsdir review (and response), but I can't help wondering if
this second paragraph is a proposed way to meet the
requirement^Wrecommendation of the first paragraph.  If so, it would be
tempting to make the first one into a "MUST", since we really think it's
important that servers should do this in some way, though we don't have
as strong an opinion on how.  (I do not expect my
reasoning to convince you more if the opsdir review did not, though, so
feel free to treat this as a side note.)

I also think that the response to the genart review was incomplete --
the genart reviewer's point about knowing, given two versions of this
extension, which one is said to be "newer" is perhaps still open.
(Suppose version 1.1 is published in 2021, 2.0 in 2022, and 1.2 in 2.23.
Is version 2.0 "newer than" 1.2?)  I didn't find much in 5730 that we
could lean on, though it's kind of drowned out by the sea of '''<?xml
version="1.0"'''...

Section 3.1

   The <loginSec:event> MAY include a free-form description.  All of the

nit: I'm not sure whether we want to consider "<loginSec:event>" as an
adjective ("<loginSec:event> element") or a noun in its own right, but I
am having a hard time with the definite article in the current text.
Since the previous sentence talks about multiple events being allowed,
maybe "Each" is best?  (Also, is this description "as the body of the
element"?)

   "type":  A REQUIRED attribute that defines the type of security
       event.  The enumerated list of "type" values includes:
       [...]

(side note: the specific "type" values chosen by the authors is rather
different from what I would have written.  It feels a bit presumptuous
to assume that expiration is the only password- or certificate-related
event that could occur and privilege those events to have the bare
"password" and "certificate" type values, for example.  I don't know
what the server would do to convey knowledge of a breach of the server's
password database file, for example  But what's there is clearly
documented so I have no real grounds to object to it per se.)

   "exDate":  Contains the date and time that a "warning" level has or
       will become an "error" level.  At expiry there MAY be an error to
       connect or MAY be an error to login.  An example is an expired

The "MAY"s feel a little out of place here, and I would suggest "At this
expiration time, the error might be realized as either a connection
failure or a login failure" to also avoid the "error to" construction
that flows strangely to my eye.

   "value":  Identifies the value that resulted in the login security
       event.  An example is the negotiated insecure cipher suite or the
       negotiated insecure TLS protocol.

Right now the encoding used for this value feels highly under-specified.
Consider the case of an insecure TLS cipher suite.  IANA manages a table
of TLS ciphersuites
(https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-4)
so one might see a pair of hex values, a single four-hex-digit-string, a
human-readable "Description", the decimal value corresponding to the
four-hex-digit-string, etc.  I mostly expect that this value will only
ever be displayed to a human and is not expected to be
machine-parseable, but we really need to say that if so (and perhaps
also that the human-readable string form is preferred when there are
multiple choices).

   Example login security event for password expiration, where the
   current date is 2018-03-25:

I appreciate the use of a "current date" for purposes of the example;
that's a nice touch!

Section 3.2

   The <loginSec:pw> element MUST override the [RFC5730] <pw> element
   only if the <pw> contains the predefined value of "[LOGIN-SECURITY]",
   which is a constant value for the server to use the <loginSec:pw>
   element for the password.  Similarly, the <loginSec:newPW> element

nits(?): the "MUST override [...] only if" construction seems prone to
misreading (both here and for <newPW>").  Is it a prohibition against
overriding in other scenarios?  A mandate to override?  Both?  Perhaps a
declarative "When the <pw> element contains the predefined value
"[LOGIN-SECURITY]", the <loginSec:pw> element overrides the <pw>
element" is more clear.  Also, there seems to be a word or two missing
for the current formulation to parse properly, in "is a constant value
for the server to use the <loginSec:pw> element for the password" --
perhaps saying that this "indicates to" the server.  (I myself would use
the phrase "sentinel value" to describe this password string, but maybe
that's overused.)

Section 3.3

   Coordinated Time (UTC) using the Gregorian calendar.  The extended
   date-time form using upper case "T" and "Z" characters defined in
   [W3C.REC-xmlschema-2-20041028] MUST be used to represent date-time
   values, as XML Schema does not support truncated date-time forms or
   lower case "T" and "Z" characters.

nit(?): "the EPP XML Schema" or "this protocol's XML Schema", I think.

Section 4.1

Sending detailed userAgent information seems to merit some discussion of
privacy considerations w.r.t user profiling and/or tracking.

   <loginSec:userAgent>:  OPTIONAL client user agent that identifies the
       client application software, technology, and operating system

nit: is this better as "client user agent information"?

   <loginSec:pw>:  OPTIONAL plain text password that is case sensitive,
       has a minimum length of 6 characters, and has a maximum length

Why is it optional?
Also, 6 characters feels a bit short for a password, these days.  (Yes,
I did pull up SP 800-63B.)

       that is up to server policy.  All leading and trailing whitespace
       is removed, and all internal contiguous whitespace that includes

We list explicitly the values considered "internal whitespace" but not
"leading and trailing whitespace".  I note that using Unicode attributes
to determine whitespace nature (which would seem reasonable to many
people) will recognize quite a few more codepoints as being whitespace,
and it is important to have a clear and unambiguous procedure.

       #x9 (tab), #xA (linefeed), #xD (carriage return), and #x20
       (space) is replaced with a single #x20 (space).  This element
       MUST only be used if the [RFC5730] <pw> element is set to the
       "[LOGIN-SECURITY]" value.

What does "only be used" mean?  Is this restricting transmission from
the client or processing by the server, or both?  What should the error
handling behavior be when it is present and it's not supposed to be?

[ditto for <loginSec:newPW>]

   (space) - #x7E (~), with high entropy, such as 128 bits.  If non-
   ASCII characters are supported with the plain text password, then use
   a standard for passwords with international characters, such as the
   OpaqueString PRECIS profile in [RFC8265].

My understanding is that the PRECIS profiles are intended to be used at
a per-protocol level, not a per-deployment level.  So this needs to be
stronger than just "such as".

Also, if (as per the example) this is carried in a client's XML request
that contains an "encoding" attribute, what's the interaction between
that encoding and any per-password encoding for non-ASCII data?

[I think I asked about repeating the "ABC-12345" clTRID for multiple
examples in the same document for a previous EPP document, but forget
why it was deemed to be okay.  Assuming that I do remember the tenor of
the prevous discussion correctly, no response is needed here.]

   C:          <loginSec:os>x86_64 Mac OS X 10.11.6</loginSec:os>

(macOS 10.11 feels a bit dated for 2010)

   C:        <loginSec:pw>this is a long password</loginSec:pw>

(side note: Long, but fairly low-entropy.  "Correct horse battery
staple" might get a smile from some readers...  Similarly for the other
examples)

   Upon a completed login command (success or failed), the extension
   MUST be included in the response based on both of the following
   conditions:

I suggest "MUST be included in the response when both of the following
conditions hold".

   Example EPP response to a failed login command where the password has
   expired and the new password does not meet the server complexity
   requirements:

nit: the exDate given here is 2018-03-26, but the most recent "time of
request" we listed was 2018-03-25, which is before this nominal
expiration date.  It would be nice to either mention a new "time of
request" or mak the examples more consistent across each other.

I also wonder about the usefulness of sending detailed errors back to an
unauthenticated user.  It would, for example, probably not be a good
idea to report various statistical events to an unauthenticated user
that's potentially an attacker seeking information for further attacks.

Section 8

I think it would be appropriate to have a brief note that while this
extension increases the maximum length of password that may be used for
EPP authentication, it still involves sending unmodified passwords to
the server for verification, and as such retains the security
considerations of doing so from RFC 5730.

The maximum <loginSec:pw> length that Section 4.1 has as "up to server
policy" is in some sense an anti-DoS measure, to keep the server from
having to dedicate potentially "unlimited" resources to handling a
client request that is authenticated only by the TLS connection.

As mentioned above, we should have a bit of discussion of the
security/privacy considerations of the information sent with the
security events, particularly when sent to an unauthenticated user.

   The extension leaves the password (<pw> element) and new password
   (<newPW> element) minimum length beyond 6 characters and the maximum
   length up to sever policy.  The server SHOULD enforce minimum and

nit: I don't understand what "minimum length beyond 6 characters means".

Section 10

Why is RFC 2119 Normative but RFC 8174 Informative?  Together they are
both BCP 14, and I don't see why they would be treated differently.
[ed. I see Éric already asked and got a response.]