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

["Gould, James" <[e-mail]>]

Benjamin,



Thank you for your thorough review and thoughtful feedback.  My responses are embedded below.



--



JG







James Gould

Distinguished Engineer

[e-mail] <applewebdata://13890C55-AAE8-4BF3-A6CE-B4BA42740803/[e-mail]>



703-948-3271

12061 Bluemont Way

Reston, VA 20190



Verisign.com <http://verisigninc.com/>



On 1/22/20, 9:31 PM, "regext on behalf of Benjamin Kaduk via Datatracker" <[e-mail] on behalf of [e-mail]> wrote:



    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.)



JG - The goal of draft-ietf-regext-login-security is to enhance the security of RFC5730 using an EPP extension, which means that it is evolutionary and not revolutionary.  The introduction describes the key security issue with RFC5730 with the hard limit of a 16-character maximum password length, which draft-ietf-regext-login-security addresses in the least impactful method possible.  Adding features that make security events more transparent in the protocol by adding the ability to pass the client user agent, to help the server identify potential security events, and by adding the ability for the server to proactively return security events to the client, are security enhancements not associated with RFC5730 being insecure.  There was discussion on the mailing list related to the use of alternative authentication approaches for EPP, but those proposals need to mature into something concrete for discussion.  Please forward any ideas that you have to the regext mailing list.



    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.



JG - The use of free-form strings (e.g., string, token, normalizedString) is nothing new with XML and all of the elements and attributes of draft-ietf-regext-login-security have been fully considered for the level of specification.  There are many examples in the EPP RFCs, including the definition of the <pw> and <newPw> elements in RFC5730, where the format is left up to server policy.  Inclusion of the 16-character maximum length in RFC5730 is an example of a policy that was injected into the RFC5730 that needs to be fixed based on the latest security practices.  The intention of draft-ietf-regext-login-security is to extend RFC5730 without adding new policy constraints that would hinder its adoption in the industry and would support future security policies.  The sentence "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]" was added to draft-ietf-regext-login-security to address the PRECIS profile concern with the AD.



    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?



JG - One of the goals of this extension is to enhance RFC 5730 by overriding the <pw> and <newPw> elements with the use of this sentinel value and support for passing the new <loginSec:pw> and <loginSec:newPw> elements without the 16-character maximum length restriction.  There is no need to mark draft-ietf-regext-login-security as updating RFC 5730, since a client will know that the server is implementing draft-ietf-regext-login-security based on the inclusion of the urn:ietf:params:xml:ns:epp:loginSec-1.0 extension namespace returned in the EPP greeting and the server will look for the sentinel value and the extension in the login command.  This is a natural form of an EPP extension that RFC 5730 fully supports.



    ----------------------------------------------------------------------

    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.



JG - Are you proposing a change here?  See my response to your first Discuss point.



    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.)



JG - Thanks for the references, I'll take a look at them.



    (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.)



JG - Thanks for the feedback.  I believe that the definition of the Event as the first section (section 3.1) of the Object Attributes section (section 3) is the appropriate location.



    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.)



JG - There was much discussion on the mailing list about this section and the appropriate normative language.  Paragraphs 2 and 3 do propose a method with meeting the requirement in paragraph 1.  I do believe that SHOULD is appropriate in this case.



    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"'''...



JG - There is no mention of version number in RFC 5730, but it is general understood in regext that the version numbers always increase.  Do you believe something needs to be added explicitly in section 2, such as adding the sentence "A newer version of the extension is expected to use an XML namespace with a higher version number than the prior versions." at the end of the first paragraph?



    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"?)



JG - I view the <loginSec:event> element as an noun (or object) in its own right that is contained in a list.  Is the issue associated with the sentence "There MAY be multiple events returned that provide information for the client to address." being in the middle of a paragraph describing the event object?  How about changing that sentence to read "The <loginSec:event> element is contained in a list of one or more elements in the <loginSec:loginSecData> element, so there MAY be multiple events returned that provide information for the client to address."?



       "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.)



JG - The "password" and "certificate" types are based on known cases of implementing a password-expiry policy and proactively informing clients of expiring client certificates.  There has been cases where clients were unaware of an expiring client certificate that caused a client-side connectivity outage, so now the server can inform them of an expiring client certificate.  Others cases would fall into the "custom" security event.



       "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.



JG.- The "exDate" attribute can be used with multiple event types.  For example, the server could decide to deprecate TLS ciphers ("cipher") or protocols ("tlsProtocol") and specify an expiry of when they will be removed, which would result in a connection failure.  The " exDate " attribute could also be used with the “custom” event, where it may not result in connection or login failure.  The purpose of the MAY is to make it clear that the expiry may result in an failure (connect or login), but there are cases where it will not result in a failure.  The <loginSecPolicy:errorAction> element in draft-gould-regext-login-security-policy provides an example of defining the policy of what occurs at expiry for an event type, which includes the possible values of "connect" for a connection failure, "login" for a login failure, or "none" for no failure.



JG - How about changing the "error to" sentence to read "At expiry there MAY be a connection failure or MAY be an login failure."?  A similar change can be made to the following example sentence with "An example is an expired certification that will result in a connection failure or an expired password that may result in a login failure.".  Do you agree with these changes?





       "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).



JG - Thanks for the reference to the IANA TLS cipher suites.  I have personally not seen TLS implementations consistently use the IANA TLS cipher suites, so that is a good reference but unfortunately not universally used.  I agree that that value will not be automatically parsed and processed by the client, but will most likely will be logged and included in an alert for manual investigation.  The value returned based on the TLS implementation should provide some meaning to the client; although mapping to the client-side TLS cipher suite names is likely necessary and may require help from the server to interpret out of band.  What I have seen are string values similar to the Description field in https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-4, but again I have not seen consistency across the TLS implementations.  How about doing the following to address this:

  1.  Update the “cipher” type description to read:

a.       Identifies the use of an insecure or deprecated TLS cipher suite.  The “name” attribute MUST be set with the name of the cipher suite, which is free-form and is not expected to be parsed and automatically addressed by the client.  An example of cipher suite names can be found in the TLS Cipher Suites of the Transport Layer Security (TLS) Parameters IANA registry [link to https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-4].

  1.  Update the “tlsProtocol” type description to read:
     *   Identifies the use of an insecure or deprecated TLS protocol.  The “name” attribute MUST be set with the name of the TLS protocol, which is free-form and is not expected to be parsed and automatically addressed by the client.

Thoughts?





       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!



JG – The dates can be revised to 2020 instead of 2018.



    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.)



JG – On the use of sentinel or constant, I prefer the use of constant.  I can modify the <pw> reference and the <newPw> reference in 3.2 to read:

  1.  When the [RFC5730] <pw> element contains the predefined value of “[LOGIN-SECURITY]”, the <loginSec:pw> element overrides the <pw> element.
  2.  Similarly, when the [RFC5730] <newPw> element contains the predefined value of “[LOGIN-SECURITY]”, the <loginSec:newPw> element overrides the <pw> element.



    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.



JG – The limitation is with XML schema and is not specific to EPP.  This is boilerplate text in the EPP RFC’s (e.g., https://tools.ietf.org/html/rfc5731#section-2.4, https://tools.ietf.org/html/rfc5733#section-2.7, https://tools.ietf.org/html/rfc5732#section-2.4).



    Section 4.1



    Sending detailed userAgent information seems to merit some discussion of

    privacy considerations w.r.t user profiling and/or tracking.



JG – This is not tracking or profiling individual users, but is a system-to-system interface.  Do you have any recommendations for what would be discussed for a system-to-system interface?



       <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"?



JG – I will make that change.



       <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?



JG – It’s optional, since the user may still be using the <pw> element from RFC5730, which supports passwords up to 16-characters.  When the minimum password length policy is greater than 16-characters, than it would be required by server policy.



    Also, 6 characters feels a bit short for a password, these days.  (Yes,

    I did pull up SP 800-63B.)



JG – The working group discussed this in detail.  The agreement was to match the existing minimum in RFC5730.  It would be up to server policy to set a higher minimum.  The maximum length of 16 characters from RFC5730 was removed in draft-ietf-regext-login-security.



           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.



JG – This is explicitly stating what is done by the XML parsing when using the XML schema “token” type.  I agreed to add this based on working group feedback.



           #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?



JG – “only be used” means “only be set” or even “only be included”.  In other words, the client MUST only set the <loginSec:pw> element when the “[LOGIN-SECURITY]” value is set in the <pw> element.    The server would use the RFC5730 “<pw>” element, based on the language in section 3.2.  This is the corresponding language for the client to not provide two passwords in the login command.  Do you prefer “only bet set” over “only be used”?



    [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".



JG – There was discussion around PRECIS on the mailing list and this language was working out with the AD.



    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?



JG – The encoding in the XML header applies to the entire XML packet.  The encoding chosen would need to support non-ASCII data (e.g., “UTF-8”) and the XML parser would need to support that encoding to unmarshal the data correctly.  The encoding in the XML header and the characters allowed for the passwords is not new for EPP RFC 5730, where draft-ietf-regext-login-security is enabling the use of longer passwords.  As noted previously, the reference to the PRECIS profile in [RFC8265] was added to address the case where a server does support non-ASCII passwords based on working with the AD.



    [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.]



JG – The clTRID is not a security credential but is used as an Asynchronous Completion Token (ACT), so there is nothing wrong with using “ABC-12345”.



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



    (macOS 10.11 feels a bit dated for 2010)



JG – This can be updated to a newer Mac OS, such as 10.14 or 10.15.



       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)



JG – Agreed, but it provides better context for the element than a high-entropy value.



       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".



JG – This change can be made.



       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.



JG – Good catch!  I’ll change that the exDate to 2018-03-24.  Actually, it will be 2020-03-24 based on the earlier agreement to change 2018 to 2020 throughout the draft.



    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.



JG – Agreed, but inclusion of the events would be up to the security policy of the server.  The extension supports returning it if the server needs to.  Below, I proposed adding a sentence associated with this in section 8.



JG



    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.


JG – Agreed, based on the feedback from Roman, The leading sentence “The Security Considerations of [RFC5730]<file:///Users/jgould/projects/github/login-security-extension/draft-ietf-regext-login-security.html#RFC5730> apply in this document, and this document enhances these considerations.” will be added to section 8.





    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.



JG – Agreed, are you asking for any change for this?



    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.



JG – How about adding the following at the end of the last paragraph in section 8:

“The server returning of security events to unauthenticated users needs to take into account the security/privacy issues of potentially returning information to potential attackers.”



       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".



JG – “beyond” can be changed to “greater than”, so it would read “…minimum length greater than 6 characters…”



    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.]



JG – Yes, that will be addressed, thanks!





    _______________________________________________

    regext mailing list

    [e-mail]

    https://www.ietf.org/mailman/listinfo/regext