I'm always adding the SSH-2.0- line (I see no value in letting the user supply ...
10 years, 5 months ago
(2013-10-14 13:01:48 UTC)
#5
I'm always adding the SSH-2.0- line (I see no value in letting the user supply
it). I did clarify the comment to distinguish between the version line
("SSH-2.0-Go") and software version ("Go").
This changes the semantics of ClientVersion. I have a server that expects an "XSH-" prefix ...
10 years, 5 months ago
(2013-10-14 18:33:58 UTC)
#6
This changes the semantics of ClientVersion. I have a server that expects an
"XSH-" prefix for legacy reasons -- I need to be able to set the full client
version string.
I also think we should allow the user to validate incoming client version
strings in the same way we allow the user to set the outgoing client version.
On Mon, Oct 14, 2013 at 8:33 PM, <jpsugar@google.com> wrote: > This changes the semantics ...
10 years, 5 months ago
(2013-10-14 19:54:49 UTC)
#7
On Mon, Oct 14, 2013 at 8:33 PM, <jpsugar@google.com> wrote:
> This changes the semantics of ClientVersion. I have a server that
> expects an "XSH-" prefix for legacy reasons -- I need to be able to set
> the full client version string.
Ugh. What happens if you connect to this from a client that discards
lines that don't start with SSH-2.0 ? I''m assuming the server also
advertises itself as XSH-something. Can you connect to this from
OpenSSH ?
> I also think we should allow the user to validate incoming client
> version strings in the same way we allow the user to set the outgoing
> client version.
--
Han-Wen Nienhuys
Google Munich
hanwen@google.com
On Mon, Oct 14, 2013 at 12:54 PM, Han-Wen Nienhuys <hanwen@google.com> wrote: > Ugh. What ...
10 years, 5 months ago
(2013-10-14 19:59:00 UTC)
#8
On Mon, Oct 14, 2013 at 12:54 PM, Han-Wen Nienhuys <hanwen@google.com> wrote:
> Ugh. What happens if you connect to this from a client that discards
> lines that don't start with SSH-2.0 ? I''m assuming the server also
> advertises itself as XSH-something. Can you connect to this from
> OpenSSH ?
Nope. Doesn't work with OpenSSH, but this library worked before. Is
the rationale here to enable parsing of the version strings? If so, I
can understand. It is possible for me to support my use case by
interposing at a lower layer and rewriting the version strings in
flight.
- JP
The RFC says to ignore everything before the SSH-2.0- line, for the following reason. The ...
10 years, 5 months ago
(2013-10-14 20:14:17 UTC)
#9
The RFC says to ignore everything before the SSH-2.0- line, for the
following reason.
The primary use of this feature is to allow TCP-
wrappers to display an error message before disconnecting.
currently, Go SSH does not work correctly with such a configuration
(do people still use TCP wrappers, though?)
We could make the prefix configurable, but I think your use-case may
be better dealt with by writing your own net.Conn wrapper?
On Mon, Oct 14, 2013 at 9:58 PM, JP Sugarbroad <jpsugar@google.com> wrote:
> On Mon, Oct 14, 2013 at 12:54 PM, Han-Wen Nienhuys <hanwen@google.com> wrote:
>> Ugh. What happens if you connect to this from a client that discards
>> lines that don't start with SSH-2.0 ? I''m assuming the server also
>> advertises itself as XSH-something. Can you connect to this from
>> OpenSSH ?
>
> Nope. Doesn't work with OpenSSH, but this library worked before. Is
> the rationale here to enable parsing of the version strings? If so, I
> can understand. It is possible for me to support my use case by
> interposing at a lower layer and rewriting the version strings in
> flight.
>
> - JP
--
Han-Wen Nienhuys
Google Munich
hanwen@google.com
On Mon, Oct 14, 2013 at 1:13 PM, Han-Wen Nienhuys <hanwen@google.com> wrote: > We could ...
10 years, 5 months ago
(2013-10-14 20:17:12 UTC)
#10
On Mon, Oct 14, 2013 at 1:13 PM, Han-Wen Nienhuys <hanwen@google.com> wrote:
> We could make the prefix configurable, but I think your use-case may
> be better dealt with by writing your own net.Conn wrapper?
Agreed. I'll switch to rewriting instead, although it adds a touch of overhead.
- JP
On 2nd thought JP's problem means that we have to accept any and all version ...
10 years, 5 months ago
(2013-10-15 05:13:26 UTC)
#12
On 2nd thought JP's problem means that we have to accept any and all
version strings, because the whole string goes into the session hash,
so you can't fake it. OTOH, the behavior that the RFC describes
(ignoring lines that do not start with SSH-2.0- ) is something that
can be done by an external wrapper. Please hold on while I rework this
patch.
On Tue, Oct 15, 2013 at 6:46 AM, <dave@cheney.net> wrote:
> not lgtm.
>
> If the client supplies a version that should be passed through
> unmolested to the server. If that version string is invalid, then the
> remote server can tell us that.
>
>
>
>
> https://codereview.appspot.com/14641044/diff/29001/ssh/client_test.go
> File ssh/client_test.go (right):
>
>
https://codereview.appspot.com/14641044/diff/29001/ssh/client_test.go#newcode34
> ssh/client_test.go:34: testClientVersion(t, &ClientConfig{},
> "SSH-2.0-Go")
> why isn't this
>
> testClientVersion(t, &ClientConfig{}, packageVersion) ?
>
> https://codereview.appspot.com/14641044/
--
Han-Wen Nienhuys
Google Munich
hanwen@google.com
Thank you. Some small nits. agl/jp, I'd like you to review this. https://codereview.appspot.com/14641044/diff/48002/ssh/transport.go File ssh/transport.go ...
10 years, 5 months ago
(2013-10-16 01:44:20 UTC)
#14
No objections. https://codereview.appspot.com/14641044/diff/53001/ssh/transport.go File ssh/transport.go (right): https://codereview.appspot.com/14641044/diff/53001/ssh/transport.go#newcode412 ssh/transport.go:412: // all of it (version and comments) ...
10 years, 5 months ago
(2013-10-16 13:46:22 UTC)
#17
https://codereview.appspot.com/14641044/diff/53001/ssh/transport.go File ssh/transport.go (right): https://codereview.appspot.com/14641044/diff/53001/ssh/transport.go#newcode412 ssh/transport.go:412: // all of it (version and comments) go into ...
10 years, 5 months ago
(2013-10-16 16:40:18 UTC)
#18
*** Submitted as https://code.google.com/p/go/source/detail?r=5ff5636e18c9&repo=crypto *** go.crypto/ssh: put version exchange in function R=golang-dev, dave, jpsugar, agl ...
10 years, 5 months ago
(2013-10-16 21:57:16 UTC)
#23
Issue 14641044: code review 14641044: go.crypto/ssh: put version exchange in function
(Closed)
Created 10 years, 5 months ago by hanwen-google
Modified 10 years, 5 months ago
Reviewers:
Base URL:
Comments: 35