crypto/openpgp: add DSA signature support.

crypto/openpgp: add DSA signature support.

[bradfitz, 2011-03-13 18:50:37]

LGTM

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/public_key.go (right):

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/public_key.go:202: if !dsa.Verify(dsaPublicKey,
hashBytes, sig.DSASigR, sig.DSASigS) {
kinda sad that that this returns a bool rather than an os.Error just for
symmetry with rsa

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/signature.go (right):

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/signature.go:423: // SignRSA signs a message with
an RSA private key. The hash, h, must contain
The omission of error details is probably fine & obvious, but you say nothing of
where its signature goes on the success case.

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/signature.go:435: // the hash of the message to be
signed and will be mutated by this function
likewise, and missing trailing period here (you have one above)

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/signature.go:448: return
error.InvalidArgumentError("Signature: need to call SignRSA/DSA before
Serialize")
"SignRSA or SignDSA" ?

[nsz_port70.net, 2011-03-13 21:47:41]

* agl@golang.org <agl@golang.org> [2011-03-13 18:42:43 +0000]:
> Description:
> crypto/openpgp: add DSA signature support.
> 
thanks

hm in keys.go in function ReadKeyRing:

  for {
          var e *Entity
          e, err = readEntity(packets)
          if err != nil {
                  if _, ok := err.(error.UnsupportedError); ok {
                          err = readToNextPublicKey(packets)
                  }

so it seems unsupported keys are discarded
but then if i have a keyring with only one entity
which is unsupported then i get no keys and no error
the doc comment says one or more:

// ReadKeyRing reads one or more public/private keys, ignoring unsupported keys.

eg i have a dsa secret keyring with elgammal (algo 16)
subkey, but elgamal is not supported so ReadKeyRing returns
empty entity list and i don't know the reason

i don't know how to resolve this nicely, but it seems
returning unsupported errors in some way would be
useful

[agl1, 2011-03-15 17:13:30]

On Sun, Mar 13, 2011 at 5:47 PM, Szabolcs Nagy <nsz@port70.net> wrote:
> i don't know how to resolve this nicely, but it seems
> returning unsupported errors in some way would be
> useful

It seems reasonable that, if no keys were read and we got an
Unsupported error, then we return it. I'll make that change.


AGL

[agl1, 2011-03-16 14:50:20]

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

crypto/openpgp: add DSA signature support.

R=bradfitzgo, nsz
CC=golang-dev
http://codereview.appspot.com/4280041

Committer: Adam Langley <agl@golang.org>

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/public_key.go (right):

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/public_key.go:202: if !dsa.Verify(dsaPublicKey,
hashBytes, sig.DSASigR, sig.DSASigS) {
On 2011/03/13 18:50:37, bradfitzgo wrote:
> kinda sad that that this returns a bool rather than an os.Error just for
> symmetry with rsa

Yea, I think returning an os.Error from the RSA code was a mistake and I'll
clean that up.

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
File src/pkg/crypto/openpgp/packet/signature.go (right):

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/signature.go:423: // SignRSA signs a message with
an RSA private key. The hash, h, must contain
On 2011/03/13 18:50:37, bradfitzgo wrote:
> The omission of error details is probably fine & obvious, but you say nothing
of
> where its signature goes on the success case.

Done.

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/signature.go:435: // the hash of the message to be
signed and will be mutated by this function
On 2011/03/13 18:50:37, bradfitzgo wrote:
> likewise, and missing trailing period here (you have one above)

Done.

http://codereview.appspot.com/4280041/diff/2001/src/pkg/crypto/openpgp/packet...
src/pkg/crypto/openpgp/packet/signature.go:448: return
error.InvalidArgumentError("Signature: need to call SignRSA/DSA before
Serialize")
On 2011/03/13 18:50:37, bradfitzgo wrote:
> "SignRSA or SignDSA" ?

Done.