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

Issue 57720043: code review 57720043: gosshnew/ssh: add AES-GCM cipher. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by hanwen-google
Modified:
10 years, 2 months ago
Reviewers:
agl1, jpsugar
CC:
agl1, dfc, jpsugar, golang-codereviews, hanwen-google
Visibility:
Public.

Description

gosshnew/ssh: add AES-GCM cipher.

Patch Set 1 #

Patch Set 2 : diff -r e92883b2b673 https://hanwen%40google.com@code.google.com/p/gosshnew/ #

Total comments: 16

Patch Set 3 : diff -r 43aaa4cd90a2 https://hanwen%40google.com@code.google.com/p/gosshnew/ #

Total comments: 4

Patch Set 4 : diff -r 43aaa4cd90a2 https://hanwen%40google.com@code.google.com/p/gosshnew/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+187 lines, -65 lines) Patch
M ssh/cipher.go View 1 2 3 5 chunks +113 lines, -3 lines 0 comments Download
M ssh/cipher_test.go View 1 2 2 chunks +41 lines, -44 lines 0 comments Download
M ssh/transport.go View 1 2 3 chunks +33 lines, -18 lines 0 comments Download

Messages

Total messages: 10
hanwen-google
Hello agl1, dave@cheney.net, jpsugar@google.com (cc: agl@golang.org, dave@cheney.net, golang-codereviews@googlegroups.com, hanwen@google.com, jpsugar@google.com), I'd like you to review ...
10 years, 2 months ago (2014-01-28 11:50:41 UTC) #1
jpsugar
Why did you switch to a hard coded cipherlist in the test?
10 years, 2 months ago (2014-01-28 15:42:21 UTC) #2
hanwen-google
On 2014/01/28 15:42:21, jpsugar wrote: > Why did you switch to a hard coded cipherlist ...
10 years, 2 months ago (2014-01-28 15:48:43 UTC) #3
jpsugar
Not LGTM I'm on my tablet right now, so I can't add line comments. I'll ...
10 years, 2 months ago (2014-01-28 16:05:34 UTC) #4
agl1
https://codereview.appspot.com/57720043/diff/20001/ssh/cipher.go File ssh/cipher.go (right): https://codereview.appspot.com/57720043/diff/20001/ssh/cipher.go#newcode98 ssh/cipher.go:98: gcmCipherID: {16, 12, 0, nil}, the nil createFunc probably ...
10 years, 2 months ago (2014-01-28 16:08:42 UTC) #5
jpsugar
https://codereview.appspot.com/57720043/diff/20001/ssh/transport.go File ssh/transport.go (right): https://codereview.appspot.com/57720043/diff/20001/ssh/transport.go#newcode212 ssh/transport.go:212: if algs.Cipher == gcmCipherID { I'm not a fan ...
10 years, 2 months ago (2014-01-28 16:56:54 UTC) #6
hanwen-google
https://codereview.appspot.com/57720043/diff/20001/ssh/cipher.go File ssh/cipher.go (right): https://codereview.appspot.com/57720043/diff/20001/ssh/cipher.go#newcode98 ssh/cipher.go:98: gcmCipherID: {16, 12, 0, nil}, On 2014/01/28 16:08:42, agl1 ...
10 years, 2 months ago (2014-01-28 17:44:03 UTC) #7
agl1
LGTM The special casing does not vex me too much. https://codereview.appspot.com/57720043/diff/40001/ssh/cipher.go File ssh/cipher.go (right): https://codereview.appspot.com/57720043/diff/40001/ssh/cipher.go#newcode272 ...
10 years, 2 months ago (2014-01-28 17:48:19 UTC) #8
hanwen-google
https://codereview.appspot.com/57720043/diff/40001/ssh/cipher.go File ssh/cipher.go (right): https://codereview.appspot.com/57720043/diff/40001/ssh/cipher.go#newcode272 ssh/cipher.go:272: cipherText := c.aead.Seal(c.buf[:0], c.iv, c.buf, c.prefix[:]) On 2014/01/28 17:48:19, ...
10 years, 2 months ago (2014-01-28 17:53:04 UTC) #9
hanwen-google
10 years, 2 months ago (2014-01-28 17:58:58 UTC) #10
*** Submitted as https://code.google.com/p/gosshnew/source/detail?r=fd066bde1818
***

gosshnew/ssh: add AES-GCM cipher.

R=agl, dave, jpsugar
CC=agl, dave, golang-codereviews, hanwen, jpsugar
https://codereview.appspot.com/57720043
Sign in to reply to this message.

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