IETF-SSH archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Core IDs - last review
Hi,
Very good catches. Comments in line.
On Sat, 3 Dec 2005, Jacob Nevins wrote:
Chris Lonvick writes:
I've placed a set of diffs from what the RFC Editor had previously
proposed to what is currently being proposed here:
http://www.employees.org/~lonvick/secsh-wg/2005-nov-30/4250diff.html
http://www.employees.org/~lonvick/secsh-wg/2005-nov-30/4251diff.html
http://www.employees.org/~lonvick/secsh-wg/2005-nov-30/4252diff.html
http://www.employees.org/~lonvick/secsh-wg/2005-nov-30/4253diff.html
http://www.employees.org/~lonvick/secsh-wg/2005-nov-30/4254diff.html
Following is a straight comparison of the change requests you posted to
the WG mailing list and the edits actually made by RFC-Editor, based on
the diffs at employees.org. (This assumes that what was posted here is
what was sent to RFC-Editor, modulo the corrections also posted here.)
I've also highlighted a few nits with the original change requests.
Executive summary: everything I've picked up is a tedious little nit
which won't affect understanding (with the possible exception of RFC4252
comment 11). Not worth the documents languishing for another month,
anyway.
RFC4250 (assignednumbers): pathetically tiny nits:
4)
Section 4.5.1, first paragraph
- replace the paragraph
OLD:
Protocol packets containing the SSH_MSG_CHANNEL_REQUEST message with
a "pty-req" string MUST contain "encoded terminal modes" with an
opcode of 1 byte. The opcode values are in the range of 1 to 255.
Opcodes 1 to 159 have a single uint32 argument. Opcodes 160 to 255
are not yet defined.
NEW:
Protocol packets containing the SSH_MSG_CHANNEL_REQUEST message with
a "pty-req" string MUST contain a 'encoded terminal modes' value.
The opcode values consist of a single byte and are in the range of 1
to 255. Opcodes 1 to 159 have a uint32 argument. Opcodes 160 to
255 are not yet defined.
Nit: was edited as requested, but >>a 'encoded terminal modes' value<<
should be >>an 'encoded terminal modes' value<< in the new text
(2nd line).
I think it's "a value" (or "a blob value") rather than "an value". I'll
leave this unless I get an additional request to change it.
8)
Section 4.6.1, first paragraph
- add a clause
OLD:
at-sign ("@"), a comma (","), whitespace, or control characters
^^^
(ASCII codes 32 or less). Names are case-sensitive, and MUST NOT
NEW:
at-sign ("@"), a comma (","), whitespace, control characters
(ASCII codes 32 or less), or the ASCII code 127 (DEL). Names are
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
case-sensitive, and MUST NOT ...
Nit: old text did not say >>a comma (",")<<, it said just
comma (",")<< (1st line). RFC-Editor has inserted "a". This should be
reverted.
OK
RFC4251 (architecture): pathetically tiny nits:
3)
Section 4.1, last paragraph
- restore the previous wording
OLD:
Thus, providing the option to not check the server host key is
^^^^^^
NEW:
Thus, providing the option not to check the server host key is
^^^^^^
The second-to-last paragraph, which contained a similar phrase, was
edited. The last paragraph was not.
OK
4)
Section 4.3, third bullet in the first paragraph
- modify a line
OLD:
depend on the location from which the user is trying to log in.
NEW:
depend on the location from where the client is trying to gain
access.
Only 'which' has been changed to 'where'. (To be honest, I'm not too
bothered about the missing changes.)
It still bothers me. I'll request that it be changed.
6)
Section 4.5, next to last paragraph
- reword the last line
OLD:
names. Straight, bit-wise, binary comparison is RECOMMENDED.
^ ^
NEW:
names. Straight bit-wise binary comparison is RECOMMENDED.
7)
Section 5.4, third paragraph
- remove a comma
OLD:
names. Straight, bit-wise, binary comparison is RECOMMENDED.
^
NEW:
names. Straight bit-wise, binary comparison is RECOMMENDED.
There is no section 5.4, and this phrase appears only once in the
document. RFC-Editor has implemented the second edit to section 4.5.
I caught that before submitting to the RFC Editor. Is
"Straight bit-wise, binary comparison..."
acceptable?
8)
Section 6, first bullet in second paragraph
- add a clause
OLD:
at-sign ("@"), a comma (","), whitespace, or control characters
^^^
(ASCII codes 32 or less). Names are case-sensitive, and MUST NOT
NEW:
at-sign ("@"), a comma (","), whitespace, control characters
(ASCII codes 32 or less), or the ASCII code 127 (DEL). Names are
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
case-sensitive, and MUST NOT
Same comment as assignednumbers re >>a comma<<, except that it does
appear thus in the original version of this document.
OK
RFC4252 (userauth): mostly nits, one borderline-semantic comment:
2)
Section 5, packet description after the first paragrah
- the packet syntax is not consistent with the format used in RFC 4254.
OLD:
byte SSH_MSG_USERAUTH_REQUEST
string user name in ISO-10646 UTF-8 encoding [RFC3629]
string service name in US-ASCII
string method name in US-ASCII
The rest of the packet is method-specific.
^^^^^^^^^^^^^^^^^^^^^^^.. this line still needs to be removed
NEW:
byte SSH_MSG_USERAUTH_REQUEST
string user name in ISO-10646 UTF-8 encoding [RFC3629]
string service name in US-ASCII
string method name in US-ASCII
.... method specific fields follow
RFC-Editor has put just >>method specific fields<<. (*shrug*.)
I'll think abut this.
4)
Section 5.2, second paragraph
- replace the paragraph
OLD:
If no authentication is needed for the user, the server MUST return
SSH_MSG_USERAUTH_SUCCESS. Otherwise, the server MUST return
SSH_MSG_USERAUTH_FAILURE and MAY return it with a list of
authentication 'method name' values.
NEW:
If no authentication is needed for the user, the server MUST return
SSH_MSG_USERAUTH_SUCCESS. Otherwise, the server MUST return
SSH_MSG_USERAUTH_FAILURE and MAY return it a list of methods that
may continue in its 'authentications that can continue' value.
RFC-Editor's edit matches this, but on the third line, I'd
s/return it a list/return with it a list/.
OK
8)
Section 7, Second paragraph
- replace the paragraph
OLD:
With this method, the possession of a private key serves as
authentication. This method works by sending a 'signature' created
with a private key of the user. The server MUST check that the key
is a valid authenticator for the user, and MUST check that the
'signature' is valid. If both hold, the authentication request MUST
be accepted; otherwise, it MUST be rejected. (Note that the server
MAY require additional authentications after successful
authentication.)
NEW:
With this method, the possession of a private key serves as
authentication. This method works by sending a signature created
with a private key of the user. The server MUST check that the key
is a valid authenticator for the user, and MUST check that the
signature is valid. If both hold, the authentication request MUST
be accepted; otherwise, it MUST be rejected. (Note that the server
MAY require additional authentications after successful
authentication.)
Parentheses on the final sentence seem to have been removed.
(*shrug*.)
I did request that as well. I couldn't see why it needs to be in
parentheses.
Extra edits have appeared in sections 7, 8, and 9 in packet
descriptions: s/service/service name/ in a couple of places.
However, there is still one place in section 8 (packet description after
1st paragraph) where it just says 'service'.
The RFC Editor did miss that. I'll re-request it.
11)
Section 8, second paragraph
- change the wording
OLD:
added to the database, or compare them (with or without hashing) to
^^^^^^^^^^^^
NEW:
added to the database, or compared (with or without hashing) to
RFC-Editor hasn't removed the "them". The resulting sentence doesn't
make sense. IIRC the consensus from previous discussion was that the
original edit approached a semantic change, so it would be good to have
this clear.
Got it.
13)
Section 9, in the fourth paragraph
- add a reference
OLD:
key' are defined in the transport layer specification. The 'public
NEW:
key' are defined in the transport layer specification [RFC4253]. The 'public
Actually edited to [SSH-TRANS] (which I prefer, and is consistent with
references elsewhere).
I caught that before submitting to the RFC Editor. It is more descriptive
that way.
RFC4253 (transport): missing edits, but nits really:
5)
Section 6.1, first paragraph
- revise a line
OLD:
larger packets MAY be sent if the version string indicates that the
^^^^^^^
NEW:
larger packets MAY be sent if the identification string indicates that
the
^^^^^^^^^^^^^^
RFC-Editor's version has lost the final "the" (possibly due to confusing
wrapping as above). Text now reads "[...] indicates that other party is
able [...]".
Got it.
An extra, but welcome, edit has appeared in 6.3 third paragraph.
That was discussed after I posted to the list but before I submitted to
the RFC Editor.
13)
Section 7.1, seventh paragraph
- replace the paragraph
OLD:
Note, however, that during a key re-exchange, after sending a KEXINIT
message, each party MUST be prepared to process an arbitrary number
of messages that may be in-flight before receiving a KEXINIT from the
other party.
NEW:
Note, however, that during a key re-exchange, after sending a
SSH_MSG_KEXINIT message, each party MUST be prepared to process an
arbitrary number of messages that may be in-flight before receiving a
SSH_MSG_KEXINIT messages from the other party.
RFC-Editor's text matches this, but on the last line, "messages" should
be just "message".
OK
15)
Section 8, second paragraph with bullets
- replace the paragraph with bullets
- do not break the bulleted sections across page boundaries
OLD:
The following steps are used to exchange a key. In this, C is the
client; S is the server; p is a large safe prime; g is a generator
for a subgroup of GF(p); q is the order of the subgroup; V_S is S's
version string; V_C is C's version string; K_S is S's public host
key; I_C is C's KEXINIT message and I_S is S's KEXINIT message that
have been exchanged before this part begins.
1. C generates a random number x (1 < x < q) and computes e = g^x mod
p. C sends "e" to S.
2. S generates a random number y (0 < y < q) and computes f = g^y mod
p. S receives "e". It computes K = e^y mod p, H = hash(V_C ||
V_S || I_C || I_S || K_S || e || f || K) (these elements are
encoded according to their types; see below), and signature s on H
with its private host key. S sends "K_S || f || s" to C. The
signing operation may involve a second hashing operation.
3. C verifies that K_S really is the host key for S (e.g., using
certificates or a local database). C is also allowed to accept
the key without verification; however, doing so will render the
protocol insecure against active attacks (but may be desirable for
practical reasons in the short term in many environments). C then
computes K = f^x mod p, H = hash(V_C || V_S || I_C || I_S || K_S
|| e || f || K), and verifies the signature s on H.
NEW:
The following steps are used to exchange a key. In this, C is the
client; S is the server; p is a large safe prime; g is a generator
for a subgroup of GF(p); q is the order of the subgroup; V_S is S's
identification string; V_C is C's identification string; K_S is S's
public host key; I_C is C's SSH_MSG_KEXINIT message and I_S is S's
SSH_MSG_KEXINIT message that have been exchanged before this part
begins.
1. C generates a random number x (1 < x < q) and computes
'e' = g^x mod p. C sends 'e' to S.
2. S generates a random number y (0 < y < q) and computes
'f' = g^y mod p. S receives 'e'. It computes K = e^y mod p,
H = hash(V_C || V_S || I_C || I_S || K_S || e || f || K)
(these elements are encoded according to their types; see below),
and s ('signature of H') with its private host key. S sends
"K_S || f || s" to C. The signing operation may involve a
second hashing operation.
3. C verifies that K_S really is the host key for S (e.g., using
certificates or a local database). C is also allowed to accept
the key without verification; however, doing so will render the
protocol insecure against active attacks (but may be desirable for
practical reasons in the short term in many environments). C then
computes K = f^x mod p, H = hash(V_C || V_S || I_C || I_S || K_S
|| e || f || K), and verifies the 'signature of H' (s).
The text change requested here has not been performed.
(The requested changes boil down to:
s/version/identification/;
s/KEXINIT/SSH_MSG_KEXINIT/;
s/signature s on H/s ('signature of H')/ and similar;
quote character changes and quote mark additions)
(It's not clear to me what you meant by your request "replace the
paragraph with bullets". Nothing seems to have happened, anyway.)
It meant, "replace the paragraph and all the associated bullets". The RFC
Editor requested clarification of my requested changes. Having 'e' and
'f' (rather than naked e and f, and "e" and "f") doesn't really work
throughout this section. I'm going to try again.
RFC4254 (connection):
7)
Section 5.2, fourth paragraph
- replace the paragraph
OLD:
The maximum amount of data allowed is the current window size. The
window size is decremented by the amount of data sent. Both parties
MAY ignore all extra data sent after the allowed window is empty.
NEW:
The maximum amount of data allowed is determined by the maximum
packet size for the channel, and the current window size, whichever
is smaller. The window size is decremented by the amount of data
sent. Both parties MAY ignore all extra data sent after the allowed
window is empty.
Implementations are expected to have some limit on the ssh
transport layer packet size (any limit for rececived packets MUST
be 32768 bytes or larger, as described in [SSH-TRANS]). The
implementation of the SSH connection layer
o MUST NOT advertise a maximum packet size that would result in
transport packets larger than its transport layer is willing to
receive.
o MUST NOT generate data packets larger that its transport layer is
willing to send, even if the remote end would be willing to
accept very large packets.
RFC-Editor's changes match the request (modulo the typo 'rececived'
above). A couple of further nits:
s/ssh transport layer/SSH transport layer/ in the second paragraph;
s/larger that/larger than/ in the second bullet.
OK
An extra edit was made in section 6.10 first paragraph: remove "after
this message".
It's actually still there. The page break makes that look strange.
14)
Section 6.10, fourth paragraph
- do not break a hyphen segmented name
OLD:
Additional 'signal name' values MAY be sent in the format "sig-
name@xyz", where "sig-name" and "xyz" may be anything a particular
NEW:
Additional 'signal name' values MAY be sent in the format
"sig-name@xyz", where "sig-name" and "xyz" may be anything a particular
This edit has not been made.
OK
New set of minor change requests on their way.
Thanks,
Chris
Home |
Main Index |
Thread Index |
Old Index