code review 86190043: go.crypto/ssh: import gosshnew.

go.crypto/ssh: import gosshnew.

See https://groups.google.com/d/msg/Golang-nuts/AoVxQ4bB5XQ/i8kpMxdbVlEJ

[agl1, 2014-04-09 20:56:32]

FYI: this is an import of gosshnew with the gosshnew paths changed to go.crypto.
No other changes.

[agl1, 2014-04-09 20:57:50]

Hello hanwen@google.com (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.crypto

[agl1, 2014-04-09 21:06:06]

*** Submitted as
https://code.google.com/p/go/source/detail?r=c0fc595a2cb5&repo=crypto ***

go.crypto/ssh: import gosshnew.

See https://groups.google.com/d/msg/Golang-nuts/AoVxQ4bB5XQ/i8kpMxdbVlEJ

R=hanwen
CC=golang-codereviews
https://codereview.appspot.com/86190043

[dave_cheney.net, 2014-04-09 21:30:41]

Has there been a mistake, this code doesn't compile

FAIL code.google.com/p/go.crypto/ssh [build failed]
# code.google.com/p/go.crypto/ssh
src/code.google.com/p/go.crypto/ssh/agent.go:103: assignment count
mismatch: 3 = 2
src/code.google.com/p/go.crypto/ssh/agent.go:176: undefined: marshal
src/code.google.com/p/go.crypto/ssh/agent.go:194: undefined: ParseError
src/code.google.com/p/go.crypto/ssh/agent.go:202: undefined:
UnexpectedMessageError
src/code.google.com/p/go.crypto/ssh/agent.go:208: undefined: marshal
src/code.google.com/p/go.crypto/ssh/agent.go:209: undefined: MarshalPublicKey
src/code.google.com/p/go.crypto/ssh/agent.go:224: undefined:
UnexpectedMessageError
src/code.google.com/p/go.crypto/ssh/agent.go:231: undefined: ParseError
src/code.google.com/p/go.crypto/ssh/agent.go:244: undefined:
UnexpectedMessageError
src/code.google.com/p/go.crypto/ssh/agent.go:246: undefined: unmarshal
src/code.google.com/p/go.crypto/ssh/agent.go:246: too many errors

On Thu, Apr 10, 2014 at 7:06 AM,  <agl@golang.org> wrote:
> *** Submitted as
> https://code.google.com/p/go/source/detail?r=c0fc595a2cb5&repo=crypto
> ***
>
> go.crypto/ssh: import gosshnew.
>
> See https://groups.google.com/d/msg/Golang-nuts/AoVxQ4bB5XQ/i8kpMxdbVlEJ
>
> R=hanwen
> CC=golang-codereviews
> https://codereview.appspot.com/86190043
>
>
>
> https://codereview.appspot.com/86190043/
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[agl1, 2014-04-09 21:33:02]

On Wed, Apr 9, 2014 at 2:30 PM, Dave Cheney <dave@cheney.net> wrote:
> Has there been a mistake, this code doesn't compile

(Shakes fist at Mercurial again!)

hg didn't notice the removed files. Sorry. Fixing now.


Cheers

AGL

[agl1, 2014-04-09 21:39:21]

On Wed, Apr 9, 2014 at 2:32 PM, Adam Langley <agl@golang.org> wrote:
> hg didn't notice the removed files. Sorry. Fixing now.

Should be fixed now.


Cheers

AGL

[dave_cheney.net, 2014-04-09 22:20:50]

Thanks!

> On 10 Apr 2014, at 7:39, Adam Langley <agl@golang.org> wrote:
> 
>> On Wed, Apr 9, 2014 at 2:32 PM, Adam Langley <agl@golang.org> wrote:
>> hg didn't notice the removed files. Sorry. Fixing now.
> 
> Should be fixed now.
> 
> 
> Cheers
> 
> AGL

[hanwen-google, 2014-04-10 07:41:51]

LGTM

at what changeset of gosshnew did you do the copy?

what do you suggest for SSH changes going forward? (apply to
go.crypto/ssh or continue development in gosshnew?)


On Wed, Apr 9, 2014 at 10:56 PM,  <agl@golang.org> wrote:
> Reviewers: hanwen-google,
>
> Message:
> FYI: this is an import of gosshnew with the gosshnew paths changed to
> go.crypto. No other changes.
>
> Description:
> go.crypto/ssh: import gosshnew.
>
> See https://groups.google.com/d/msg/Golang-nuts/AoVxQ4bB5XQ/i8kpMxdbVlEJ
>
> Please review this at https://codereview.appspot.com/86190043/
>
> Affected files (+7398, -3784 lines):
>   A ssh/agent/client.go
>   A ssh/agent/client_test.go
>   A ssh/agent/forward.go
>   A ssh/agent/keyring.go
>   A ssh/agent/server.go
>   A ssh/agent/server_test.go
>   A ssh/agent/testdata_test.go
>   A ssh/benchmark_test.go
>   M ssh/buffer.go
>   ssh/buffer_test.go
>   M ssh/certs.go
>   ssh/certs_test.go
>   M ssh/channel.go
>   M ssh/cipher.go
>   ssh/cipher_test.go
>   M ssh/client.go
>   M ssh/client_auth.go
>   M ssh/client_auth_test.go
>   ssh/client_test.go
>   ssh/common.go
>   A ssh/connection.go
>   M ssh/doc.go
>   M ssh/example_test.go
>   A ssh/handshake.go
>   ssh/handshake_test.go
>   M ssh/kex.go
>   M ssh/kex_test.go
>   ssh/keys.go
>   ssh/keys_test.go
>   M ssh/mac.go
>   ssh/mempipe_test.go
>   M ssh/messages.go
>   M ssh/messages_test.go
>   ssh/mux.go
>   A ssh/mux_test.go
>   ssh/server.go
>   M ssh/session.go
>   ssh/session_test.go
>   ssh/tcpip.go
>   ssh/tcpip_test.go
>   A ssh/terminal/util_windows.go
>   A ssh/test/agent_unix_test.go
>   ssh/test/cert_test.go
>   ssh/test/forward_unix_test.go
>   M ssh/test/session_test.go
>   ssh/test/tcpip_test.go
>   M ssh/test/test_unix_test.go
>   A ssh/test/testdata_test.go
>   ssh/testdata/doc.go
>   ssh/testdata/keys.go
>   A ssh/testdata_test.go
>   ssh/transport.go
>   ssh/transport_test.go
>
>



-- 
Han-Wen Nienhuys
Google Munich
hanwen@google.com

[agl1, 2014-04-10 17:01:16]

On Thu, Apr 10, 2014 at 12:41 AM, Han-Wen Nienhuys <hanwen@google.com> wrote:
> LGTM
>
> at what changeset of gosshnew did you do the copy?

161:4c5439767eb8

> what do you suggest for SSH changes going forward? (apply to
> go.crypto/ssh or continue development in gosshnew?)

I think development should happen in go.crypto now. gosshnew contains
the history, which is useful, but I think it should be frozen now.


Cheers.

AGL

[adg, 2014-04-15 02:52:29]

The go.crypto build has been broken since this was imported, yet work
continues on go.crypto/ssh (3 CLs so far)

Can we get the build fixed? Is there an issue #?

[agl1, 2014-04-15 03:50:36]

On Mon, Apr 14, 2014 at 7:51 PM, Andrew Gerrand <adg@golang.org> wrote:
> The go.crypto build has been broken since this was imported, yet work
> continues on go.crypto/ssh (3 CLs so far)
>
> Can we get the build fixed? Is there an issue #?

It appears to be broken on 32-bit. Will fix tomorrow. I think it's
just the case that nobody noticed.


Cheers

AGL

[dave_cheney.net, 2014-04-15 03:53:55]

I noticed, sorry, I should have prioritised fixing this sooner. 

> On 15 Apr 2014, at 13:50, Adam Langley <agl@golang.org> wrote:
> 
>> On Mon, Apr 14, 2014 at 7:51 PM, Andrew Gerrand <adg@golang.org> wrote:
>> The go.crypto build has been broken since this was imported, yet work
>> continues on go.crypto/ssh (3 CLs so far)
>> 
>> Can we get the build fixed? Is there an issue #?
> 
> It appears to be broken on 32-bit. Will fix tomorrow. I think it's
> just the case that nobody noticed.
> 
> 
> Cheers
> 
> AGL
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[adg, 2014-04-15 03:58:17]

It's also broken on darwin-386/amd64 but for different reasons.


On 15 April 2014 13:50, Adam Langley <agl@golang.org> wrote:

> On Mon, Apr 14, 2014 at 7:51 PM, Andrew Gerrand <adg@golang.org> wrote:
> > The go.crypto build has been broken since this was imported, yet work
> > continues on go.crypto/ssh (3 CLs so far)
> >
> > Can we get the build fixed? Is there an issue #?
>
> It appears to be broken on 32-bit. Will fix tomorrow. I think it's
> just the case that nobody noticed.
>
>
> Cheers
>
> AGL
>

[hanwen-google, 2014-04-15 07:45:56]

thanks for the heads up, I'll see what I can do.

On Tue, Apr 15, 2014 at 5:57 AM, Andrew Gerrand <adg@golang.org> wrote:
> It's also broken on darwin-386/amd64 but for different reasons.
>
>
> On 15 April 2014 13:50, Adam Langley <agl@golang.org> wrote:
>>
>> On Mon, Apr 14, 2014 at 7:51 PM, Andrew Gerrand <adg@golang.org> wrote:
>> > The go.crypto build has been broken since this was imported, yet work
>> > continues on go.crypto/ssh (3 CLs so far)
>> >
>> > Can we get the build fixed? Is there an issue #?
>>
>> It appears to be broken on 32-bit. Will fix tomorrow. I think it's
>> just the case that nobody noticed.
>>
>>
>> Cheers
>>
>> AGL
>
>



-- 
Han-Wen Nienhuys
Google Munich
hanwen@google.com

[hanwen-google, 2014-04-16 11:39:31]

On Tue, Apr 15, 2014 at 4:51 AM, Andrew Gerrand <adg@golang.org> wrote:
> The go.crypto build has been broken since this was imported, yet work
> continues on go.crypto/ssh (3 CLs so far)
>
> Can we get the build fixed? Is there an issue #?

current issues:

* we can't seem to insert keys into ssh-agent on Darwin, both ECDSA
(but DSA, RSA work) and RSA cert.
* Linux ARM builder has no homedirectory configured. This makes all
sshd tests barf with:

    agent_unix_test.go:40: running ssh-add: Process exited with: 1.
Reason was:  (), out Could not chdir to home directory /nonexistent:
No such file or directory
This account is currently not available.

* Darwin/race
(http://build.golang.org/log/d334ba602057939af214f99329f2d2221c59e689),
cannot read the ECDSA host key, so all sshd tests fail.  Is this a
different machine/different sshd  from the normal Darwin build?

* NetBSD only fails on terminal modes sshd test,
http://build.golang.org/log/d334ba602057939af214f99329f2d2221c59e689

--- FAIL: TestValidTerminalMode (0.42 seconds)
session_test.go:274: terminal mode failure: expected -echo in stty
output, got Last login: Thu Mar 13 09:31:36 2014
..
lflags: icanon isig iexten echo echoe -echok echoke -echonl echoctl
       -echoprt -altwerase -noflsh -tostop -flusho pendin -nokerninfo
       -extproc

* dragonfly: looks like some file has an inconsistent build tag. What
tags apply to dragonfly?
* plan9:

  *** Test killed: ran too long (10m0s).
  FAIL code.google.com/p/go.crypto/ssh 600.064s

looks like some test was hung. Can we have stacktraces or verbose output?

-- 
Google Germany GmbH - ABC-Str. 19 - 20354 Hamburg
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg - Geschäftsführer: Graham Law,
Christine Elizabeth Flores

[hanwen-google, 2014-04-16 11:41:16]

almost all issues are incompatibilities with the locally installed
sshd/ssh-agent. I would really like to continue having these tests, as
they're the primary service we're interacting with, but we need some
assurance that the builders have the sane versions of the tools.

Can we disable the sshd tests for problematic builders, and if so, how ?

On Wed, Apr 16, 2014 at 1:39 PM, Han-Wen Nienhuys <hanwen@google.com> wrote:
> On Tue, Apr 15, 2014 at 4:51 AM, Andrew Gerrand <adg@golang.org> wrote:
>> The go.crypto build has been broken since this was imported, yet work
>> continues on go.crypto/ssh (3 CLs so far)
>>
>> Can we get the build fixed? Is there an issue #?
>
> current issues:
>
> * we can't seem to insert keys into ssh-agent on Darwin, both ECDSA
> (but DSA, RSA work) and RSA cert.
> * Linux ARM builder has no homedirectory configured. This makes all
> sshd tests barf with:
>
>     agent_unix_test.go:40: running ssh-add: Process exited with: 1.
> Reason was:  (), out Could not chdir to home directory /nonexistent:
> No such file or directory
> This account is currently not available.
>
> * Darwin/race
(http://build.golang.org/log/d334ba602057939af214f99329f2d2221c59e689),
> cannot read the ECDSA host key, so all sshd tests fail.  Is this a
> different machine/different sshd  from the normal Darwin build?
>
> * NetBSD only fails on terminal modes sshd test,
> http://build.golang.org/log/d334ba602057939af214f99329f2d2221c59e689
>
> --- FAIL: TestValidTerminalMode (0.42 seconds)
> session_test.go:274: terminal mode failure: expected -echo in stty
> output, got Last login: Thu Mar 13 09:31:36 2014
> ..
> lflags: icanon isig iexten echo echoe -echok echoke -echonl echoctl
>        -echoprt -altwerase -noflsh -tostop -flusho pendin -nokerninfo
>        -extproc
>
> * dragonfly: looks like some file has an inconsistent build tag. What
> tags apply to dragonfly?
> * plan9:
>
>   *** Test killed: ran too long (10m0s).
>   FAIL code.google.com/p/go.crypto/ssh 600.064s
>
> looks like some test was hung. Can we have stacktraces or verbose output?
>
> --
> Google Germany GmbH - ABC-Str. 19 - 20354 Hamburg
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg - Geschäftsführer: Graham Law,
> Christine Elizabeth Flores



-- 
Han-Wen Nienhuys
Google Munich
hanwen@google.com

[0intro, 2014-04-16 14:07:21]

> * plan9:
>
>   *** Test killed: ran too long (10m0s).
>   FAIL code.google.com/p/go.crypto/ssh 600.064s
>
> looks like some test was hung. Can we have stacktraces or verbose output?

The ssh tests on Plan 9 shouldn't go very far on Plan 9 since sshd is
not present.
I'll provide you more information when I'll be back home.

-- 
David du Colombier

[hanwen-google, 2014-04-16 16:53:53]

On Wed, Apr 16, 2014 at 4:07 PM, David du Colombier <0intro@gmail.com> wrote:
>> * plan9:
>>
>>   *** Test killed: ran too long (10m0s).
>>   FAIL code.google.com/p/go.crypto/ssh 600.064s
>>
>> looks like some test was hung. Can we have stacktraces or verbose output?
>
> The ssh tests on Plan 9 shouldn't go very far on Plan 9 since sshd is
> not present.
> I'll provide you more information when I'll be back home.

the sshd tests run Skip if sshd is not there.

the test that is hanging is the hermetic tests, which runs a Go client
against a Go server.

-- 
Han-Wen Nienhuys
Google Munich
hanwen@google.com