IETF-SSH archive

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

Re: Eyeballs needed.



Hi,

On Fri, 28 Oct 2005, Jon Bright wrote:

Bill Sommerfeld wrote:

 draft-ietf-secsh-userauth-27.txt:
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4252-diff.html
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4252.txt

Section 5.2: "Otherwise, the server MUST return SSH_MSG_USERAUTH_FAILURE and MAY return it with a list of authentication 'method name' values." - the "that can continue" phrase has been lost here. I remember recently seeing a discussion about OpenSSH being unable to determine what can or cannot continue and hence returning a list of all available methods - but was this change in the draft desired?

Good catch. It's been that way for some time but SSH_MSG_USERAUTH_FAILURE only has 'authentications that can continue' - which can only come from the server.


Section 8, third paragraph: "Systems supporting non-ASCII passwords SHOULD always normalize passwords and user names whenever they are added to the database, or compare them (with or without hashing) to existing entries in the database." - the "username" -> "user name" change seems fine, but the "compared" -> "compare them" change seems to have erroneously changed the meaning of the sentence.

I'll change that.


 draft-ietf-secsh-transport-24.txt:
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4253-diff.html
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4253.txt

Section 4: "SSH works over any 8-bit, clean, binary-transparent transport. The underlying transport SHOULD protect against transmission errors, as such errors cause the SSH connection to terminate." - the first comma added here is incorrect. The transport's "8-bit clean", not "8-bit" and "clean", much as the concept of a clean and tidy transport is appealing. Changing it to "8-bit-clean" might emphasise the point.

I'll go back to "8-bit clean".


6.1: "The maximum of 35000 bytes is an arbitrarily chosen value that is larger than uncompressed size." - should probably be something like "...that is larger than the uncompressed payload length above."

See later note.


6.3: "The ciphers in each direction MUST run independent of each other." -> "The ciphers in each direction MUST run independently of one another."?

I'll stick with the original.


6.5: "SSH maintains its own group identifier space that is logically distinct from Oakley..." - I found "which" much better in this context, but see the note at the start of my previous mail. Maybe it's just me.
 Maybe I'm wrong.

I'll reconsider "which"es and "that"s if someone really presses.


8, paragraph 2: "...and I_S is S's KEXINIT message that have been exchanged before this part begins." - again, I preferred "which". But it should definitely be "has", not "have". "Exchanged" also seems wrong. Maybe "...and I_S is S's KEXINIT message, as previously sent from S to C."?

I made some changes to this paragraph but it really is "have". See my note about the proposed changes.


8, further down: "Either side MUST NOT send or accept 'e' or 'f' values that are not in the range [1, p-1]." should probably be "Values of 'e' or 'f' that are not in the range [1, p-1] MUST NOT be sent or accepted by either side."

Again, I've made some changes.  Please review.


 draft-ietf-secsh-connect-25.txt:
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4254-diff.html
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4254.txt

6.5, penultimate paragraph: "It is RECOMMENDED that the reply for these messages be requested and checked ." - an extra space has crept in here.

Yup.


6.9: "'signal names' will be encoded as discussed in the "exit-signal" SSH_MSG_CHANNEL_REQUEST." - there's no field here called 'signal names'.
 I think the text should be "Signal names..." as before.

Got it.


6.10: If I'm following the quoting conventions used here correctly, 'signal name' here should be swapped for "signal name".

Yup.


7.1: ""localhost" means to listen on all protocol families supported by the SSH implementation on loopback addresses only ([RFC3330] and [RFC3513])." - probably add a "see" at the start of the parenthesis?

OK


7.2: "The 'originator IP address' is the numeric IP address of the machine where the connection request comes from, and the 'originator port' is the port on the host from where the connection originated." - should probably be "...from which the connection originated."?

I made some changes here.


 draft-ietf-secsh-dns-05.txt:
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4255-diff.html
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4255.txt

Not my document.  :)


1: In other places, at least initial references to "SSH" have been expanded to be "Secure Shell (SSH)". Here not.

Didn't spot anything else in this document.

 draft-ietf-secsh-auth-kbdinteract-07.txt:
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4256-diff.html
ftp: //ftp.rfc-editor.org/in-notes/authors/rfc4256.txt

Abstract: "The Secure Shell Protocol (SSH) is..." should perhaps be "The Secure Shell (SSH) protocol is..."?

3.1: If the "language tag is deprecated and SHOULD be the empty string", then it's surely not "as defined in [RFC-3066]"? Should the 3066 reference maybe move down to the "If the language tag is not the empty string..." paragraph?

3.2: As for 3.1 (or not, if I'm wrong). Further down, I'm pretty sure "backended" isn't a word (since "back end"/"backend" isn't a verb). I'm not too vehement on the point, but this may be a stumbling block for non-native English speakers.

3.4: (Similarly to section 8 of userauth.) "Systems supporting non-ASCII passwords SHOULD always normalize passwords and user names whenever they are added to the database, or compare them (with or without hashing) to existing entries in the database.". The "username" -> "user name" change from userauth hasn't been applied here (though that doesn't really bother me). Again, the "compared" -> "compare them" change seems to have erroneously changed the meaning of the sentence.

--
Jon Bright
Silicon Circus Ltd.
http://www.siliconcircus.com


Thanks,
Chris



Home | Main Index | Thread Index | Old Index