Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(15)

Issue 4962064: code review 4962064: net/ssh: new package. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 11 months ago by agl1
Modified:
13 years, 11 months ago
Reviewers:
CC:
bradfitz, borman, dave_cheney.net, gustavo_niemeyer.net, dsymonds, r, adg, rsc, rog, lvd, kevlar, raul.san, golang-dev
Visibility:
Public.

Description

exp/ssh: new package. The typical UNIX method for controlling long running process is to send the process signals. Since this doesn't get you very far, various ad-hoc, remote-control protocols have been used over time by programs like Apache and BIND. Implementing an SSH server means that Go code will have a standard, secure way to do this in the future.

Patch Set 1 #

Patch Set 2 : diff -r 1b800cd636c0 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 1b800cd636c0 https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r 1b800cd636c0 https://go.googlecode.com/hg/ #

Total comments: 14

Patch Set 5 : diff -r 1b800cd636c0 https://go.googlecode.com/hg/ #

Total comments: 34

Patch Set 6 : diff -r 1b800cd636c0 https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 7 : diff -r a15d18698352 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2742 lines, -0 lines) Patch
A src/pkg/exp/ssh/Makefile View 1 2 3 4 1 chunk +16 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/channel.go View 1 2 3 4 5 1 chunk +317 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/common.go View 1 2 3 4 1 chunk +96 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/doc.go View 1 2 3 4 5 1 chunk +79 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/messages.go View 1 2 3 4 5 1 chunk +557 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/messages_test.go View 1 2 3 4 1 chunk +125 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/server.go View 1 2 3 4 1 chunk +711 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/server_shell.go View 1 2 3 4 5 6 1 chunk +399 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/server_shell_test.go View 1 2 3 4 1 chunk +134 lines, -0 lines 0 comments Download
A src/pkg/exp/ssh/transport.go View 1 2 3 4 1 chunk +308 lines, -0 lines 0 comments Download

Messages

Total messages: 39
agl1
Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
13 years, 11 months ago (2011-09-08 23:33:32 UTC) #1
borman
The package name "ssh" is misleading as this package does not implement an ssh client, ...
13 years, 11 months ago (2011-09-08 23:39:39 UTC) #2
dave_cheney.net
Awesome! I was looking at the old (broken) ssh.go package last night. I would be ...
13 years, 11 months ago (2011-09-08 23:41:08 UTC) #3
dave_cheney.net
I'm sure the client code isn't far behind. On Fri, Sep 9, 2011 at 9:39 ...
13 years, 11 months ago (2011-09-08 23:42:50 UTC) #4
gustavo_niemeyer.net
> The package name "ssh" is misleading as this package does not implement > an ...
13 years, 11 months ago (2011-09-08 23:43:45 UTC) #5
dsymonds
I don't think this belongs in the standard library. It should live in an external ...
13 years, 11 months ago (2011-09-08 23:47:44 UTC) #6
agl1
On Thu, Sep 8, 2011 at 7:43 PM, Gustavo Niemeyer <gustavo@niemeyer.net> wrote: > Isn't it ...
13 years, 11 months ago (2011-09-08 23:47:55 UTC) #7
gustavo_niemeyer.net
>> Isn't it just a matter of time? There's a difference for sure, but >> ...
13 years, 11 months ago (2011-09-08 23:56:19 UTC) #8
r
I would favor doing this in a goinstallable repository. It seems a perfect candidate. That ...
13 years, 11 months ago (2011-09-09 03:18:22 UTC) #9
bradfitz
I'm torn. I like goinstall but the fact that this is crypto makes me nervous ...
13 years, 11 months ago (2011-09-09 03:25:15 UTC) #10
adg
On 9 September 2011 13:18, Rob 'Commander' Pike <r@golang.org> wrote: > I would favor doing ...
13 years, 11 months ago (2011-09-09 03:25:17 UTC) #11
borman
Given Brad's argument, if it is in the standard library I would like to see ...
13 years, 11 months ago (2011-09-09 03:44:37 UTC) #12
dave_cheney.net
I think that having an integrated ssh facility in the standard library would be a ...
13 years, 11 months ago (2011-09-09 03:46:23 UTC) #13
adg
On 9 September 2011 13:25, Brad Fitzpatrick <bradfitz@golang.org> wrote: > I'm torn. > I like ...
13 years, 11 months ago (2011-09-09 03:54:59 UTC) #14
gustavo_niemeyer.net
> I think that having an integrated ssh facility in the standard library > would ...
13 years, 11 months ago (2011-09-09 03:57:35 UTC) #15
rsc
I don't think that holding up Adam's code while we figure out whether or not ...
13 years, 11 months ago (2011-09-09 04:24:43 UTC) #16
dave_cheney.net
+1 achievement unlocked: reasonable compromise. > I propose to put this in exp/ssh for now, ...
13 years, 11 months ago (2011-09-09 04:31:49 UTC) #17
adg
On 9 September 2011 14:24, Russ Cox <rsc@golang.org> wrote: > I propose to put this ...
13 years, 11 months ago (2011-09-09 04:35:36 UTC) #18
adg
http://codereview.appspot.com/4962064/diff/8001/src/pkg/net/ssh/channel.go File src/pkg/net/ssh/channel.go (right): http://codereview.appspot.com/4962064/diff/8001/src/pkg/net/ssh/channel.go#newcode12 src/pkg/net/ssh/channel.go:12: // A Channel is an ordered, reliable, duplux stream ...
13 years, 11 months ago (2011-09-09 05:38:14 UTC) #19
agl1
Moved to exp/ssh and have replied to comments. http://codereview.appspot.com/4962064/diff/8001/src/pkg/net/ssh/channel.go File src/pkg/net/ssh/channel.go (right): http://codereview.appspot.com/4962064/diff/8001/src/pkg/net/ssh/channel.go#newcode12 src/pkg/net/ssh/channel.go:12: // ...
13 years, 11 months ago (2011-09-13 13:04:33 UTC) #20
r
http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/channel.go File src/pkg/exp/ssh/channel.go (right): http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/channel.go#newcode18 src/pkg/exp/ssh/channel.go:18: // other methods on the Channel may be called. ...
13 years, 11 months ago (2011-09-13 17:47:25 UTC) #21
adg
On 13 September 2011 23:04, <agl@golang.org> wrote: > Which, actually, is probably silly. It shouldn't ...
13 years, 11 months ago (2011-09-13 23:28:25 UTC) #22
dave_cheney.net
Looks good. A few suggestions based on the output of 8g -s -m http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/messages.go File ...
13 years, 11 months ago (2011-09-14 11:15:46 UTC) #23
rog
http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/messages.go File src/pkg/exp/ssh/messages.go (right): http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/messages.go#newcode563 src/pkg/exp/ssh/messages.go:563: } that's interesting. the second form is clearer, but ...
13 years, 11 months ago (2011-09-14 11:46:16 UTC) #24
dave_cheney.net
http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/messages.go File src/pkg/exp/ssh/messages.go (right): http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/messages.go#newcode563 src/pkg/exp/ssh/messages.go:563: } On 2011/09/14 11:46:17, rog wrote: > that's interesting. ...
13 years, 11 months ago (2011-09-14 11:52:53 UTC) #25
lvd
On Wed, Sep 14, 2011 at 13:15, <dave@cheney.net> wrote: > Looks good. A few suggestions ...
13 years, 11 months ago (2011-09-14 12:00:58 UTC) #26
agl1
http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/channel.go File src/pkg/exp/ssh/channel.go (right): http://codereview.appspot.com/4962064/diff/13001/src/pkg/exp/ssh/channel.go#newcode18 src/pkg/exp/ssh/channel.go:18: // other methods on the Channel may be called. ...
13 years, 11 months ago (2011-09-14 14:10:14 UTC) #27
rsc
On Wed, Sep 14, 2011 at 10:10, <agl@golang.org> wrote: > It's not ssh specifically: I'm ...
13 years, 11 months ago (2011-09-14 15:09:34 UTC) #28
r
Can you explain why the ssh secure network protocol has to understand cursor up and ...
13 years, 11 months ago (2011-09-14 15:40:13 UTC) #29
rsc
On Wed, Sep 14, 2011 at 11:40, Rob 'Commander' Pike <r@golang.org> wrote: > Can you ...
13 years, 11 months ago (2011-09-14 15:46:46 UTC) #30
agl1
On Wed, Sep 14, 2011 at 11:40 AM, Rob 'Commander' Pike <r@golang.org> wrote: > Can ...
13 years, 11 months ago (2011-09-14 16:10:56 UTC) #31
borman
Why is this done by the ssh server? Normally programs do this by using termcap, ...
13 years, 11 months ago (2011-09-14 16:26:13 UTC) #32
r
Are you trying to tell me that ssh must be a combination TTY driver and ...
13 years, 11 months ago (2011-09-14 16:28:21 UTC) #33
borman
I can see that the ssh server may need to expose WINCH signals, but that ...
13 years, 11 months ago (2011-09-14 16:33:55 UTC) #34
agl1
On Wed, Sep 14, 2011 at 12:33 PM, Paul Borman <borman@google.com> wrote: > I can ...
13 years, 11 months ago (2011-09-14 16:56:14 UTC) #35
borman
Then I would suggest writing a separate terminal package to be used in conjunction with ...
13 years, 11 months ago (2011-09-14 17:12:53 UTC) #36
kevlar
http://codereview.appspot.com/4962064/diff/24001/src/pkg/exp/ssh/channel.go File src/pkg/exp/ssh/channel.go (right): http://codereview.appspot.com/4962064/diff/24001/src/pkg/exp/ssh/channel.go#newcode23 src/pkg/exp/ssh/channel.go:23: Read(data []byte) (int, os.Error) I would suggest that Read ...
13 years, 11 months ago (2011-09-17 04:23:17 UTC) #37
raul.san
On 2011/09/14 17:12:53, borman wrote: > Then I would suggest writing a separate terminal package ...
13 years, 11 months ago (2011-09-17 09:07:55 UTC) #38
agl1
13 years, 11 months ago (2011-09-17 19:57:35 UTC) #39
*** Submitted as http://code.google.com/p/go/source/detail?r=2c36e4180313 ***

exp/ssh: new package.

The typical UNIX method for controlling long running process is to
send the process signals. Since this doesn't get you very far, various
ad-hoc, remote-control protocols have been used over time by programs
like Apache and BIND.

Implementing an SSH server means that Go code will have a standard,
secure way to do this in the future.

R=bradfitz, borman, dave, gustavo, dsymonds, r, adg, rsc, rogpeppe, lvd, kevlar,
raul.san
CC=golang-dev
http://codereview.appspot.com/4962064
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b