Re: [regext] review of draft-ietf-regext-login-security-03

[Martin Casanova <[e-mail]>]

Jim

Below my comments. Thanks.

MC

On 04.04.19 00:20, Gould, James wrote:
>
> Martin,
>
>  
>
> Thank you for doing a detailed review of
> draft-ietf-regext-login-security.  I provide my responses to your
> feedback embedded below.
>
>   
>
> —
>
>  
>
> JG
>
>
> cid:[e-mail]933A30
>
> *James Gould
> *Distinguished Engineer
> [e-mail]
>
> 703-948-3271
> 12061 Bluemont Way
> Reston, VA 20190
>
> Verisign.com <http://verisigninc.com/>
>
>  
>
> *From: *regext <[e-mail]> on behalf of Martin Casanova
> <[e-mail]>
> *Date: *Tuesday, March 26, 2019 at 1:56 PM
> *To: *"[e-mail]" <[e-mail]>
> *Subject: *[EXTERNAL] [regext] review of
> draft-ietf-regext-login-security-03
>
>  
>
> Jim
>
> Since we support that draft I started implementing and read it again
> more carefully. I have the following comments / feedback
>
> 1. (comment) Event type "cipher" or "tlsProtocol" :
>
> In case the client uses deprecated protocol or cipher we currently
> hang up the TLS connection immediately so there is no possibility to
> send a type "error" security event.
> At chapter 3.1 near "exDate" is written that "At expiry there MAY be
> an error to connect or MAY be an error to login." So in case of an
> error to connect you run in the same situation and will not be able to
> send back a "error" level event but thats OK. However it is useful to
> warn the client during a transition period when we know that we are
> going to disable a certain cipher or TLS protocol on a specific day in
> the future.
>
> JG – Yes, connectivity events like “cipher”, “tlsProtocol”, and
> “certificate” are really only useful when they are provided as a
> warning with an optional exDate to indicate when the event will result
> in a connection error.  The policies for the possible set of events
> can be defined using the Login Security Policy Extension
> (https://tools.ietf.org/html/draft-gould-regext-login-security-policy),
> which is an extension of the Registry Mapping
> (https://tools.ietf.org/html/draft-gould-carney-regext-registry).  The
> clients need to look for the security events in the login response to
> act on them.       
>
MC -Agreed.


> 2 . (comment) Event type “newPw”:
> Here we currently use  2306 “Parameter value policy error” and write
> in the <reason> of <extValue> element that the new password was too
> weak. 
> I guess we would use the loginSec Event in the future in case the
> extension was specified at login whether the pw was changed via
> login-security extension or not.
>
> I know that you have foreseen draft-gould-regext-login-security-policy
> to query about password complexity but I think for convenience of the
> registrar it would still be nice to somehow include the password
> requirement in the response even if it is redundant. Otherwise one has
> to implement draft-gould-regext-login-security-policy  at the same
> time or communicate the requirement out of band.
>
> Maybe like that
>
>    S:        <loginSec:event
>    S:          type=”newPW”
>    S:          value=”expression: (?=.*\d)(?=.*[a-zA-Z])(?=.*[\x21-\x2F\x3A-\x40\x5B-\x60\x7B-\x7E]) (?!^\s+) (?!.*\s+$)(?!.*\s{2,})^[\x20-\x7e]{16,128}$”
>    S:          level=”error”>
>    S:          New password does not meet complexity requirements
>    S:        </loginSec:event>
>    S:    <result code=”2200”>
>    S:      <msg>Authentication error</msg>
>
>
> Are you sure you want to return a 2200 and not a 2306 Parameter value
> policy error in this case (page 10). I don’t see a reason why this
> should be another return code with or without extension.
>
> JG – On the first part of your feedback, I’m not sure that we want to
> inject policy elements into the error response.  Of course the policy
> information could be embedded within the human readable message, such
> as below.  I don’t recommend the client parsing the human readable
> message for policy information, but this would be useful for logging
> and troubleshooting on the client-side.  One issue is that the client
> can only receive the password complexity expression upon an error. 
> It’s best to handle the password complexity expression out-of-band or
> via the Login Security Policy Extension
> (draft-gould-regext-login-security-policy) to enable the client to
> know about the policy prior to an error.      
>
>    S:        <loginSec:event
>    S:          type=”newPW”
>    S:          level=”error”>
>    S:          New password does not meet complexity requirement: (?=.*\d)(?=.*[a-zA-Z])(?=.*[\x21-\x2F\x3A-\x40\x5B-\x60\x7B-\x7E]) (?!^\s+) (?!.*\s+$)(?!.*\s{2,})^[\x20-\x7e]{16,128}$
>    S:        </loginSec:event>

MC - agreed:  - useful for logging and troubleshooting on the
client-side, shouldn't be parsed.. Generally I think error feedback
should be as indicative of the problem as possible so the registrar
would not open a ticket at the registry to ask why the error occurred or
how it could be prevented. The out of band documentation is a must
anyway so these tickets will be answered by RTFM! as in many cases :)
I guess this is a "nice to have" but I also don't see a problem with
including this piece of policy information. It's not a policy
information that is enforced by XML Schema or so..


> JG – On the second part of your feedback, I believe that the login
> should fail if an invalid new password is passed with a 2200
> “Authentication error”.  The use of 2200 is not normative, so the
> server could choose a different result code, such as 2306.  The Login
> Security Extension will provide more specific information associated
> with the error.  Are there thoughts from others related to which error
> result code (2200, 2306, or other) should be used for an invalid new
> password and whether it should be normative?       
>
MC - RFC 5730

      2200    "Authentication error"

              This response code MUST be returned when a server notes an
              error when validating client credentials.

So I think 2200 is OK if you see <newpw> already as client credentials.
However a client could get confused with 2200 thinking the error code
relates to the <pw> filed which is correct. (if it wasn't for the
<loginSec:event>) What is your current return code when pw complexity is
not OK without using loginSec extension ? Ours is 2306..

>
> 3. (question) Event type "stat" :
>
> How often would you send back this event
>
>    <loginSec:event
>      type="stat"
>      name="failedLogins"
>      level="warning"
>      value="100"
>      duration="P1D">
>      Excessive invalid daily logins
>    </loginSec:event>
>
> Only once with first successful login after the series of failed ones
> or for a whole day ? I suggest one time with first successful login.
>
> JG – My thought is that returning the statistical information should
> not be stateful, so that means that the interval would be up to the
> server (e.g., for a one day duration start calculating the statistic
> based on a start / end of midnight UTC), but all applicable events
> would be returned in all login responses.  I do not recommend the
> server keeping track of which events have been returned and which have
> not been returned.  What are your thoughts on this? 
>
MC -  Agreed that this should not be stateful and I wouldn't want to
persist anything for this. Currently we have a mechanism in place that
hangs up the existing session and blocks new TLS sessions for x seconds
when there were too many failed request in the past y seconds. We do
have a "sliding window" algorithm in place to determine whether a client
is currently blocked or nor not. We could use the same to inform at
every successful  login that a blocking event occurred in the last 24
hours (since now)


> 4. (question) In chapter 4.1 EPP <login> Command is written
>
> ..
>
> internal contiguous
>        whitespace that includes #x9 (tab), #xA (linefeed), #xD (carriage
>        return), and #x20 (space) is replaced with a single #x20 (space).
> ...
>
> Since this is "normal" XML parsing behavior should there not be
> reference to where this is described for general XML processing. (I
> don't know where that would be though)
>
> JG – The choice was to directly include the definition within the
> draft in place of referencing out.  I’m not aware of an RFC that can
> be referenced for the XML processing of an XML schema “token” type. 
> My recommendation is to leave the definition directly in the draft
> without an external reference.
>
MC - OK
>
> 5.  (suggestion) The element  <loginSec:userAgent> could be more
> structured to make it easier for the registry to parse the different
> fields and to give a hint to the registrar what information should be
> provided.
>
> Therefore I suggest  child elements for example
>
> <os>           Operating System
> <client>      Client technology (eg. java)
> <version> Client software version (eg. 1.8.0) etc.
>
> JG – If we were going to make the <loginSec:userAgent> structured, my
> recommendation is to handle it with the following definition:
>
>    <complexType name="userAgentType">
>
>     <sequence>
>
>       <element name="client" type="token"
>
>        minOccurs="0"/>
>
>       <element name="lang" type="token"
>
>        minOccurs="0"/>
>
>       <element name="os" type="token"
>
>        minOccurs="0"/>
>
>     </sequence>
>
>    </complexType>
>
> JG – Below would be an example structured <loginSec:userAgent> element:
>
>    <loginSec:userAgent>
>
>      <loginSec:client>EPP SDK 1.0.0</loginSec:client>
>
>      <loginSec:lang>Java 11.0.2</loginSec:lang>
>
>      <loginSec:os>x86_64 Mac OS X 10.21</loginSec:os>
>
>    </loginSec:userAgent>
>
> JG – What are your thoughts on the structured version of the
> <loginSec:userAgent> element.  If this works, I will revise the draft
> to support it. 
>
MC -  Maybe the "lang" field is a bit misleading since "lang" also
exists in RFC 5730 where it has another meaning.

<element name="lang" type="language"
          maxOccurs="unbounded"/>

Hopefully nobody would fill it out with "EN".. So maybe we could find
another terminology. How about "tec" for technology ?

The information  that would be interesting to us is

- Client application and version (eg. EPP SDK 1.0.0)
- Client technology  and version (eg. openjdk version "1.8.0_191" or
Perl or ?)
- Client OS and version (eg.|Distributor ID:    Ubuntu  Description:  
 Ubuntu 16.04.5 LTS)|)

So the version is important too to decide if a client is affected by
some security issues but maybe this would be an overkill to enforce /
"ask for" or not ?


Other than that I have currently no feedback - so looking forward to the
next next revision..


>
>  
>
> Thanks
>
>  
>
> Martin Casanova
>
>
>
> -- 
> --- 
> SWITCH 
> Martin Casanova, Domain Applications
> Werdstrasse 2, P.O. Box, 8021 Zurich, Switzerland 
> phone [phone], direct [phone]
> [e-mail] <mailto:[e-mail]>, www.switch.ch <http://www.switch.ch> 
>  
> Working for a better digital world

-- 
--- 
SWITCH 
Martin Casanova, Domain Applications
Werdstrasse 2, P.O. Box, 8021 Zurich, Switzerland 
phone [phone], direct [phone]
[e-mail], www.switch.ch 
 
Working for a better digital world