Re: Ambiguities in section 3.1 of the keyboard-interactive draft

[nisse%lysator.liu.se@localhost (Niels Möller)]

pgut001%cs.auckland.ac.nz@localhost (Peter Gutmann) writes:

> This draft currently contains ambiguities that make it more or less impossible
> to create interoperable implementations:

As I started implementing this draft some week ago, I can add some
more comments

Section 2, Rationale

   Currently defined authentication methods for SSH are tightly
   coupled with the underlying authentication mechanism. [...]

   Challenge-response and One Time Password mechanisms are also easily
   supported with this authentication method.

I don't think the given examples support this authentication method.
One-time passwords are easily supported using plain "password"
authentication. Some forms of challenge response authentication can
also be done by combining using "password" authentication and
USERAUTH_BANNER. A more correct rational consists of two points:

1. It makes it easier to support challenge response systems with
   hardware tokens such as securid. (This was the motivation given
   when the method was introduced, iirc).

2. It matches the PAM way of doing user authentication. To some people
   this is a huge advantage. (Personally, I believe the PAM way is pretty
   broken, and not at all suitable for network authentication).

The default openssh configuration for some linux distributions have
disabled plain "password" authentication in favour of
keyboard-interact, my guess is that they do this in order to work
better with PAM. I think this is bad for interoperability, and think
it would make sense to make a strong recommendation that if a
particular system supports old fashioned authentication with a
username and password, then it SHOULD also support the
USERAUTH_REQUEST "password" method when speaking the ssh protocol.

Section 3, Protocol Exchanges

I think a client should be allowed to abort a sequence of
SSH_MSG_USERAUTH_INFO_REQUEST, SSH_MSG_USERAUTH_INFO_RESPONSE by
sending a new SSH_MSG_USERAUTH_REQUEST.

This can happen for example if the client thinks that it can
authenticate using keyboard-interactive, but later has to change its
mind (the used forgot his securid at home, presses the cancel button
on the keyboard-interactive dialog, and next tries to remember his
long passphrase for publickey authentication).

Section 3.1, Initial Exchange

      byte      SSH_MSG_USERAUTH_REQUEST
      string    user name (ISO-10646 UTF-8, as defined in [RFC-3629])
      string    service name (US-ASCII)
      string    "keyboard-interactive" (US-ASCII)
      string    language tag (as defined in [RFC-3066])
      string    submethods (ISO-10646 UTF-8)

The "submethods" field is problematic, as already pointed out by
Peter. The entries are supposedly to be used as identifiers. I'd
suggest that this field be defined as a "name-list" (defined in the
architecture draft), where the entries are ASCII names using the same
conventions as for other negotiated methods (i.e. "blahonga-token" is
a standardized method, "securid%securid.com@localhost" is an extension).

Section 3.2, Information Requests
      
      byte      SSH_MSG_USERAUTH_INFO_REQUEST
      string    name (ISO-10646 UTF-8)
      string    instruction (ISO-10646 UTF-8)
      string    language tag (as defined in [RFC-3066])
      int       num-prompts
      string    prompt[1] (ISO-10646 UTF-8)
      boolean   echo[1]
      ...
      string    prompt[num-prompts] (ISO-10646 UTF-8)
      boolean   echo[num-prompts]

The "name" field doesn't make much sense for me. First I assumed it
was a user name, which makes no sense (what is a client supposed to do
if it differs from the user name given in the USERAUTH_REQUEST
message?). But in the example in the end of the draft, it seems to
rather be part of the instruction, perhaps intended for a window title
or some such. I think this needs some clarification.

It may also be desirable to have one field where the server can put
the name of its selected submethod (i.e. an ssh name, not a free form
utf8 string).

The "language tag" is already deprecated, if we make changes, it
should be deleted. I don't remember the discussion leading to its
introduction or its deprecation.

The "num-prompts" is of type "int", which is not defined in the
architecture draft. I guess "uint32" is the intended type.

I also want to note that this and SSH_MSG_USERAUTH_INFO_RESPONSE are
the only request so far that uses a variable number of entries in this
way, which requires some additional complexity of the code for
formatting and parsing these messages (at least in my implementation,
which uses a general formatting function controlled by a format
string; I'd prefer not having to introduce loop constructs in the
format...). I think this is unfortunate, by I don't see any obviously
better way to do it.

As for processing of this field, Peter's suggestion makes sense to me:

>   If the server encounters an unrecognised submethod, it MUST ignore
>   that submethod. Otherwise, it MUST process the submethods in the
>   order specified by the client. In other words if the client
>   specifies submethods "method1,unknown,method2" the server should
>   first try method1, skip the unknown method, and then try method2.

I think we should also say that after the server has tried all methods
on the list that it recognizes and supports, it is free to try any
other submethods of its choice. This is what happens in the common
case that the client sends an empty submethods list.

Section 3.4, Information Responses

      byte      SSH_MSG_USERAUTH_INFO_RESPONSE
      int       num-responses
      string    response[1] (ISO-10646 UTF-8)
      ...
      string    response[num-responses] (ISO-10646 UTF-8)

Looks ok.

Regards,
/Niels