IETF-SSH archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Publickey subsystem draft posted



OK, here are some comments. I'm in favour of a protocol of this type
existing, but I think this draft is going to need some work before
it's useful.

 - You don't mention any kind of packet structure within the
   subsystem channel, like SFTP has. Is this deliberate? It looks at
   the moment as if the individual components of any given request
   (e.g. string "add", string comment, string algorithm, string
   pubkey blob) are sent straight to the subsystem without any
   overall length field.

   If that's correct and not simply a documentation error, it
   _completely_ destroys extensibility: if a subsystem receives any
   request with a name it doesn't recognise, then it cannot know how
   much of the following data belongs to that request, so after
   returning REQUEST_NOT_SUPPORTED it _can't_ resume parsing at the
   next request boundary. Without this basic extensibility property,
   what was the point of having request types described in an
   extensible string namespace at all?

It is a documentation error-- thank heavens.

The generalized message format is:

UINT32 length
string type
... request specific data follows

Sections 2.2 and 2.3 should be updated to reflect this.

 - Speaking of extensibility, I strongly support Nicolas Williams's
   comment that there already exist SSH server implementations
   supporting a greater range of public key options and restrictions
   than your draft specifies. Surely at the very least there ought
   to be some sort of extension mechanism within the "add" request
   type?

Yes, this makes sense.  The "command" option was supposed to deal
with the various restrictions supported by openssh and other implementations, but...

  a. Since we don't support it, we were kind of shooting in the
     dark as to what people needed, and clearly, we undershot.
  b. As you point out below, it needs to be integrated with the
     add command to prevent 'vulnerability windows'

 - The "command" request is seriously under-specified. It does not
   mention whether the key in question is expected to have been
   added using "add" previously, or is added by the "command"
   request itself.

    * If the former, this is a security hazard if naively
      implemented: the _only_ way to set up a restricted key is by
      issuing "add" and then "command", but between those two
      requests, there is a window of opportunity in which someone
      possessing the private key can gain unrestricted access to the
      target account. Since restricted keys are typically keys you
      don't fully trust not to fall into enemy hands, there should
      never be such a window of opportunity. Therefore, an
      implementation MUST delay actually making the changes until
      the subsystem channel is closed or a subsequent
      synchronisation message is sent; this should be clearly
      specified.

    * If the latter, this would make far more sense to me, except
      that the comment field present in the "add" request is missing
      in the "command" request.

   Since I've already proposed an extensibility field in the "add"
   request, I'm tempted to suggest that "command" is a complete
   misfeature and ought to be implemented in terms of that instead.

What if the "add" command where changed as follows?

	string "add"
	string public-key algorithm
	string public-key blob
	uint32 attribute-count
		string attrib-name
		string attrib-value
		bool   mandatory
	repeated attribute-count times

  The server MUST attempt to store the public key for the user in the
  appropriate location so the public key can be used for subsequent
  public-key authentications.

  attrib-names without a '@' are reserved to the IETF.  Extensions
  MAY be used by composing the '@' with a DNS name, for example,
  'spiffy-extension%vandyke.com@localhost'

  If the server does not implement a mandatory attribute, it MUST fail
  the add.  For the purposes of a mandatory attribute, storage of the
  attribute is not sufficient, but requires that the server understand
  and implement the intent of the attribute.

  The following attributes are defined by this draft.

  "comment"
	The comment field contains user-specified text
	about the public key.  The server SHOULD make every
	effort to preserve this value and return it with the
	key during a list operation.

	The comment field is useful so the user can identify the
	key without resorting to comparing it's fingerprint.

  "command"
	Command bypasses the session channel "exec" and "shell"
        requests by always execution the specified command.

	This attribute SHOULD be mandatory.

  "restrict"
	The value of this attribute contains server functions that
	may not be preformed when this key is used.  It is a comma
	seperated list.  Elements containing an '@' are reserved for
	implementation / site specific extension; all other elements
	are reserved for use by the IETF.

	Currently defined restrictions are:

		"x11"
		"shell"
		"exec"
		"port-forward"
		"reverse-forward"

	This attribute SHOULD be mandatory.

  For example, we might have:

  "add"
  "ssh-rsa"
  public-key-blob
  4
  "comment"
  "2048 bit RSA key for my daughter"
  "special%openssh.org@localhost"
  <special data, could be binary>
  "restrict"
  "port-forward,reverse-forward,x11,exec"
  "command"
  "daughter-shell"

The 'list' command would need to be updated to have
a similar extension format.

What do you think?  What other extensions should we
define here?  What other types ofrestrictions should
be defined?

- Joseph




Home | Main Index | Thread Index | Old Index