code review 4639049: crypto/openpgp: add ElGamal support.

crypto/openpgp: add ElGamal support.

[agl1, 2011-06-19 14:43:44]

Hello bradfitz@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2011-06-19 19:42:53]

LGTM

But one confusing comment...

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
File src/pkg/crypto/openpgp/elgamal/elgamal.go (right):

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
src/pkg/crypto/openpgp/elgamal/elgamal.go:117: // In tests, the PRNG may return
all zeros so we do
I don't quite follow this comment. Is the PRNG broken, does this only affect
tests, ... ?

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/private_key.go (right):

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/private_key.go:290: x, _, err := readMPI(buf)
will this fully consume buf? worth returning an error if buf.Len() != 0 before
the final return nil?

[r, 2011-06-19 22:02:38]

LGTM
nice

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
File src/pkg/crypto/openpgp/elgamal/elgamal.go (right):

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
src/pkg/crypto/openpgp/elgamal/elgamal.go:10: // This form of ElGamal embeds
PKCS#1 v1.5 padding which may make it unsuitable
s/padding/padding,/

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
src/pkg/crypto/openpgp/elgamal/elgamal.go:70: // returns the plaintext of the
message. An error can only result if the
s/only result/result only/

[agl1, 2011-06-22 01:01:02]

*** Submitted as http://code.google.com/p/go/source/detail?r=a30da34972bf ***

crypto/openpgp: add ElGamal support.

R=bradfitz, r
CC=golang-dev
http://codereview.appspot.com/4639049

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
File src/pkg/crypto/openpgp/elgamal/elgamal.go (right):

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
src/pkg/crypto/openpgp/elgamal/elgamal.go:10: // This form of ElGamal embeds
PKCS#1 v1.5 padding which may make it unsuitable
On 2011/06/19 22:02:38, r wrote:
> s/padding/padding,/

Done.

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
src/pkg/crypto/openpgp/elgamal/elgamal.go:70: // returns the plaintext of the
message. An error can only result if the
On 2011/06/19 22:02:38, r wrote:
> s/only result/result only/

Done.

http://codereview.appspot.com/4639049/diff/7001/src/pkg/crypto/openpgp/elgama...
src/pkg/crypto/openpgp/elgamal/elgamal.go:117: // In tests, the PRNG may return
all zeros so we do
On 2011/06/19 19:42:53, bradfitz wrote:
> I don't quite follow this comment. Is the PRNG broken, does this only affect
> tests, ... ? 

The idea is that the test might pass in a PRNG that always returns zeros. Thus,
looping waiting for a non-zero value will hang the test.

I haven't done that in these tests, however, so I've just removed this bit.