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

Issue 12837048: code review 12837048: crypto/ssh: ssh-agent forwarding support

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by jamwt
Modified:
10 years, 1 month ago
Visibility:
Public.

Description

crypto/ssh: ssh-agent forwarding support Add an argument to the client config that creates a connection to the local ssh-agent. If this argument is non-nil, the ssh library will send a channel request to the server to enable agent forwarding when the session is started (session.NewSession()). When the remote shell tries to connect to another host, the server will open an auth channel with the client. If the client has enabled agent forwarding, the client will call the connector and link/proxy the ssh channel with the agent socket. Note: this changeset protects against a malicious server requesting an agent forwarding channel when the client did not request one. The flow of how this all works was cribbed from the openssh sources. Fixes #6223

Patch Set 1 #

Patch Set 2 : diff -r 1e7a3e301825 https://code.google.com/p/go.crypto #

Patch Set 3 : diff -r 1e7a3e301825 https://code.google.com/p/go.crypto #

Patch Set 4 : diff -r 1e7a3e301825 https://code.google.com/p/go.crypto #

Total comments: 3

Patch Set 5 : diff -r 1e7a3e301825 https://code.google.com/p/go.crypto #

Total comments: 6

Patch Set 6 : diff -r 1e7a3e301825 https://code.google.com/p/go.crypto #

Patch Set 7 : diff -r 1e7a3e301825 https://code.google.com/p/go.crypto #

Patch Set 8 : diff -r 1e7a3e301825 https://code.google.com/p/go.crypto #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -10 lines) Patch
M ssh/client.go View 1 2 3 4 5 6 7 5 chunks +67 lines, -7 lines 1 comment Download
M ssh/session.go View 1 2 3 4 5 3 chunks +22 lines, -3 lines 1 comment Download

Messages

Total messages: 31
jamwt
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.crypto
10 years, 7 months ago (2013-08-22 19:32:49 UTC) #1
jamwt
On 2013/08/22 19:32:49, jamwt wrote: > Hello mailto:golang-dev@googlegroups.com, > > I'd like you to review ...
10 years, 7 months ago (2013-08-22 19:43:00 UTC) #2
jpsugar
I'm not privileged to approve things, but this looks good to me. :-) You just ...
10 years, 7 months ago (2013-08-23 17:24:57 UTC) #3
jpsugar
One possible change to the protocol. Have Connect() accept an io.Reader and io.WriteCloser. If the ...
10 years, 7 months ago (2013-08-23 18:46:49 UTC) #4
jpsugar
https://codereview.appspot.com/12837048/diff/9001/ssh/client.go File ssh/client.go (right): https://codereview.appspot.com/12837048/diff/9001/ssh/client.go#newcode425 ssh/client.go:425: ch.remoteWin.add(msg.PeersWindow) This needs to be moved down. If the ...
10 years, 7 months ago (2013-08-23 18:50:36 UTC) #5
jamwt
https://codereview.appspot.com/12837048/diff/9001/ssh/client.go File ssh/client.go (right): https://codereview.appspot.com/12837048/diff/9001/ssh/client.go#newcode425 ssh/client.go:425: ch.remoteWin.add(msg.PeersWindow) On 2013/08/23 18:50:37, jpsugar wrote: > This needs ...
10 years, 7 months ago (2013-08-23 19:17:46 UTC) #6
jpsugar
https://codereview.appspot.com/12837048/diff/9001/ssh/client.go File ssh/client.go (right): https://codereview.appspot.com/12837048/diff/9001/ssh/client.go#newcode425 ssh/client.go:425: ch.remoteWin.add(msg.PeersWindow) On 2013/08/23 19:17:46, jamwt wrote: > On 2013/08/23 ...
10 years, 7 months ago (2013-08-23 19:53:54 UTC) #7
jpsugar
FYI my use case for this involves an in-process synthetic agent, so I have a ...
10 years, 7 months ago (2013-08-23 19:55:35 UTC) #8
jamwt
On 2013/08/23 19:55:35, jpsugar wrote: > FYI my use case for this involves an in-process ...
10 years, 7 months ago (2013-08-24 04:59:41 UTC) #9
jpsugar
On 2013/08/24 04:59:41, jamwt wrote: > On the API front, I'm inclined to leave it ...
10 years, 7 months ago (2013-08-24 20:38:13 UTC) #10
jpsugar
https://codereview.appspot.com/12837048/diff/21001/ssh/client.go File ssh/client.go (right): https://codereview.appspot.com/12837048/diff/21001/ssh/client.go#newcode222 ssh/client.go:222: go io.Copy(agentSock, ch.stdout) You need to EOF agentSock somehow ...
10 years, 7 months ago (2013-08-26 16:56:33 UTC) #11
agl1
https://codereview.appspot.com/12837048/diff/21001/ssh/client.go File ssh/client.go (right): https://codereview.appspot.com/12837048/diff/21001/ssh/client.go#newcode41 ssh/client.go:41: // AgentConnector connects to the proper ssh-agent Rename AgentDialer, ...
10 years, 7 months ago (2013-08-26 17:27:41 UTC) #12
jpsugar
Also need periods at the ends of all the sentences in comments.
10 years, 7 months ago (2013-08-26 17:45:22 UTC) #13
jamwt
On 2013/08/26 17:45:22, jpsugar wrote: > Also need periods at the ends of all the ...
10 years, 7 months ago (2013-08-26 18:27:25 UTC) #14
jamwt
On 2013/08/26 18:27:25, jamwt wrote: > On 2013/08/26 17:45:22, jpsugar wrote: > > Also need ...
10 years, 7 months ago (2013-08-26 18:28:39 UTC) #15
jamwt
On 2013/08/26 18:28:39, jamwt wrote: > On 2013/08/26 18:27:25, jamwt wrote: > > On 2013/08/26 ...
10 years, 7 months ago (2013-08-26 18:31:56 UTC) #16
jpsugar
https://codereview.appspot.com/12837048/diff/33002/ssh/client.go File ssh/client.go (right): https://codereview.appspot.com/12837048/diff/33002/ssh/client.go#newcode41 ssh/client.go:41: // AgentDialer connects to the proper ssh-agent Un-wrap this ...
10 years, 7 months ago (2013-08-26 19:13:33 UTC) #17
jamwt
On 2013/08/26 19:13:33, jpsugar wrote: > https://codereview.appspot.com/12837048/diff/33002/ssh/client.go > File ssh/client.go (right): > > https://codereview.appspot.com/12837048/diff/33002/ssh/client.go#newcode41 > ...
10 years, 7 months ago (2013-08-26 19:41:01 UTC) #18
agl1
This is close enough that I can fix when landing, but I don't see jamwt ...
10 years, 7 months ago (2013-08-27 16:35:29 UTC) #19
jamwt
On 2013/08/27 16:35:29, agl1 wrote: > This is close enough that I can fix when ...
10 years, 7 months ago (2013-08-28 04:29:32 UTC) #20
jamwt
On 2013/08/28 04:29:32, jamwt wrote: > On 2013/08/27 16:35:29, agl1 wrote: > > This is ...
10 years, 5 months ago (2013-10-16 21:37:05 UTC) #21
hanwen-google
is there any way that we can add a test for this?
10 years, 5 months ago (2013-10-16 23:59:59 UTC) #22
dsymonds
A+C change: https://codereview.appspot.com/14765043/
10 years, 5 months ago (2013-10-17 00:31:17 UTC) #23
dfc
Jamie, would you please merge your CL with tip and remail it, hg mail 12837048 ...
10 years, 5 months ago (2013-10-17 00:36:28 UTC) #24
dfc
On 2013/10/17 00:36:28, dfc wrote: > Jamie, would you please merge your CL with tip ...
10 years, 5 months ago (2013-10-19 11:27:57 UTC) #25
dfc
ping, please also take the opportunity to change the description to go.crypto/ssh: add ssh-agent forwarding ...
10 years, 5 months ago (2013-10-26 00:09:20 UTC) #26
gobot
Replacing golang-dev with golang-codereviews.
10 years, 3 months ago (2013-12-20 16:26:03 UTC) #27
gobot
Replacing golang-dev with golang-codereviews.
10 years, 3 months ago (2013-12-20 16:26:04 UTC) #28
jpsugar
This should be migrated to gosshnew. - JP
10 years, 3 months ago (2013-12-20 16:56:55 UTC) #29
dfc
On 2013/12/20 16:56:55, jpsugar wrote: > This should be migrated to gosshnew. > > - ...
10 years, 3 months ago (2013-12-20 22:25:38 UTC) #30
hanwen-google
10 years, 1 month ago (2014-02-12 18:02:57 UTC) #31
On 2013/12/20 22:25:38, dfc wrote:
> On 2013/12/20 16:56:55, jpsugar wrote:
> > This should be migrated to gosshnew.
> > 
> > - JP
> 
> Jamie please re-propose this change on top of the gosshnew repo where most of
> the development is landing at the moment.
> 
> https://code.google.com/p/gosshnew/
> 
> R=close

I've added agent forwarding to gosshnew myself.

R=close
Sign in to reply to this message.

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