Overall, this seems pretty clear.  My notes below are made with the intent of making it clearer and more precise.  However, it's clear that this protocol has solid implementation experience, because it clearly fits together well.  This is an extremely sharp tool to leave lying around, but the security considerations address the sorts of things that matter; or at least I was unable to think of anything that would be necessary to address beyond the clearly identified need for authentication and proper authorization.

# Comments

This is the first document I've seen to include an RFC editor note in the abstract.

There are a number of notes that I would not remove from this document.  Those in S1.1 and S2.1 both contain information that would be useful to a reader.  (A specification exists not just to specify, but to contextualize and these seem to be highly relevant context.)

In the background section (which probably shouldn't be removed as noted), this:

> This agent protocol is already widely used and a de-facto standard, having been implemented by a number of popular SSH clients and servers for many years. The purpose of this document is to describe the protocol as it has been implemented.

This makes me question whether the intended status (proposed standard) and the note are consistent.  Who holds change control of this protocol?  If this is documenting a de facto standard, I'd expect informational status.

In S2, there are three roles mentioned: client, server, and agent.  On first read, it is not clear that the agent in question (which is a very generic term) is the server.  This confusing interchangeable use of server and agent continues throughout.  This is compounded by the statement in the introduction: "Clients (and possibly servers) can invoke the agent".

S2 fails to say what the protocol is *for*.  The description in S1 isn't complete, but I would expect S2 to say what a client (or server?) can do with the aid of a client.

S3.1 describes generic messages, but fails to explain whether these contain more information than the code.  Or even that these byte values are in fact message types.  More words, even if they might be redundant, would help here.

In S3.2, the use of byte[] is novel and not something that is defined in RFC 4251.  I understand this to mean "some unspecified number of bytes", where the specific number is determined by the type of key being transferred.  This means that the recipient will need to know the format of the key type in order to make sense of this and (importantly) later fields.  It's almost not useful to define the generic format of this message given this.  No generic parser will be able to recover the values successfully; the parser needs to know the definition of each key type to parse them (and the constraints...).

In S3.2.7, constraints are formatted using byte[], which means that any unknown constraint makes the entire set of constraints unparseable. This is fine, because the message has to be rejected if there are unknown constraints, but it is worth noting this point.

In S3.2.3, the ENC(A) value is encoded twice.  That seems unnecessary.

For S3.2.7.3, are there any known uses of constraint extensions?  Or can we assume that this extension point is potentially unusable?

Is the "reader id" in S3.4 the same as the "id" in S3.2.6?  The description is the same, but why the change of name?

S3.6.1 defines two flags, but does not define which bit each corresponds to.  ... I see these much further down.  A forward reference would help.  As for the values, does 4 mean 0x1000_0000 (the 4th bit counting from MSB), 0x0000_0008 (the fourth bit counting from LSB), or 0x0000_0004 (the value 4)?  I think that it's the last.

In S3.7, locking and unlocking are idempotent.  Why would attempting to lock an agent fail if the agent were already locked?  That seems like a success case to me (you are not asking it whether it was unlocked).

... "An agent SHOULD take countermeasures against brute-force guessing attacks against the pass-phrase." seems pretty weak in terms of a specification.

In S5, I found "the deployed integration with the SSH protocol uses vendor-specific names" confusing.  I think that this means that the remainder of this section defines the protocol extension, which happens to use a vendor-specific name or two.  This statement could be dropped.

In S7, for new registries, please provide instructions to experts about what criteria they should use in accepting (or rejecting) registrations.  Also, please consider changing to specification required instead.

In S8, there is an implication that a client of the agent might cause the agent to load arbitrary code for smartcards.  I don't believe that this is the case, but it would be good to have that made much clearer.  That is, an agent might have the ability to interact with a pre-arranged set of smartcards, some of which would need code to be loaded.  Loading that code is a risk, but this is a risk that the agent implementation takes on deliberately, hopefully not as directed by an arbitrary client.  (Clients in this protocol have considerable powers, so maybe the security posture could be looser than this, but the whole discussion of potential side channels makes me think otherwise.)  Whatever the story, this could be a little crisper about the threat model.

# Nits

In S8 "prevent its memory being read by other processes to direct theft of loaded keys. This typically include disabling " are two grammatical errors, I think.  "to direct theft" doesn't parse, and s/include/includes/