FAQ
Let me think for a bit about eating newkeys altogether, to make sure it
doesnt open holes.


https://codereview.appspot.com/14494058/diff/45001/ssh/client.go
File ssh/client.go (right):

https://codereview.appspot.com/14494058/diff/45001/ssh/client.go#newcode84
ssh/client.go:84: // discard newkeys packet.
On 2013/10/17 17:56:55, jpsugar wrote:
Do we want to ensure that it's a newkeys packet?
Done.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go
File ssh/handshake.go (right):

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode16
ssh/handshake.go:16: //const debug = true
On 2013/10/17 17:56:55, jpsugar wrote:
Replace this with an explanation of what this does?
it's for debugging, obviously. It's probably better to delete before
submitting.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode47
ssh/handshake.go:47: hostKeys []Signer // If hostKeys are given, we are
the server
On 2013/10/17 17:56:55, jpsugar wrote:
Missing period on comment.
I think this is a really important thing to note in the protocol.
Accidentally
missing hostKeys causes us to completely flip our role in the
exchange, which
would probably produce weird errors. It may be simpler and safer to
have a bool
isServer or similar.
The initialization takes care of it, though: either you run
newClientTransport, or newServerTransport.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode80
ssh/handshake.go:80: func newClientTransport(conn keyingTransport,
clientVersion, serverVersion []byte,
On 2013/10/17 17:56:55, jpsugar wrote:
Wrap at 80, 120, or don't wrap. Wrapping at 82 is weird. :-) I prefer
no wrap.

Done.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode91
ssh/handshake.go:91: func newServerTransport(
On 2013/10/17 17:56:55, jpsugar wrote:
Why are the args on a new line?
Done.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode154
ssh/handshake.go:154: // This is not completely accurate, as the debug
On 2013/10/17 17:56:55, jpsugar wrote:
Add a TODO?
Done.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode154
ssh/handshake.go:154: // This is not completely accurate, as the debug
On 2013/10/17 17:56:55, jpsugar wrote:
Add a TODO?
I've moved the filtering into this layer up, so we get this correct now.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode228
ssh/handshake.go:228: scramble := make([]byte, len(packet))
On 2013/10/17 17:56:55, jpsugar wrote:
I'm not a fan of "scramble". "packetCopy"?
Done.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode256
ssh/handshake.go:256: t.mu.Unlock()
On 2013/10/17 17:56:55, jpsugar wrote:
defer this?
I recall that defer are a little expensive, since it does a heap
allocation, hence the attempt to avoid it. There are no early returns,
so defer doesn't make the code cleaner.

https://codereview.appspot.com/14494058/diff/45001/ssh/handshake.go#newcode278
ssh/handshake.go:278: clientInit := otherInit
On 2013/10/17 17:56:55, jpsugar wrote:
If I'm reading this correctly, these four vars only really affect the magics.
Can we fold this directly into the init of the magics?
the message is used for determining algorithms, see line 289.

https://codereview.appspot.com/14494058/

--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedOct 20, '13 at 8:55p
activeOct 20, '13 at 8:55p
posts1
users1
websitegolang.org

1 user in discussion

Hanwen: 1 post

People

Translate

site design / logo © 2021 Grokbase