openpgp/packet: Add support for V3 public keys and signatures

openpgp/packet: Add support for V3 public keys and signatures.

My goal here is to allow read-only access to legacy key material without allowing new V3 keys or signatures.

[cmars, 2013-08-29 23:12:58]

PTAL

[agl1, 2013-08-30 14:43:28]

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/packet.go
File openpgp/packet/packet.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/packet.go#n...
openpgp/packet/packet.go:301: func peekVersion(r io.Reader) (bufr *bufio.Reader,
ver byte, err error) {
comment.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/packet.go#n...
openpgp/packet/packet.go:347: }
isSubkey := tag == packetTypePublicSubkey
if version < 4 {
  p = &PublicKeyV3{
    IsSubkey: isSubkey,
  }
} else {
  p = &PublicKey{
    IsSubkey: isSubkey,
  }
}

Then the type switch can be dropped.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key.go
File openpgp/packet/public_key.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:527: rsaPublicKey, _ :=
pk.PublicKey.(*rsa.PublicKey)
, _ not needed.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:533: dsaPublicKey, _ :=
pk.PublicKey.(*dsa.PublicKey)
ditto.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:551: func keySignatureHash(pk, signed signingKey,
hfn crypto.Hash) (h hash.Hash, err error) {
s/hfn/hashFunc/

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:552: if h = hfn.New(); h == nil {
crypto.Hash.New doesn't return nil. I screwed this up.

if !hashFunc.Available() {
  return nil, errors.UnsupportedError("hash function")
}
h := hashFunc.New()

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:576: func userIdSignatureHash(id string, pk
*PublicKey, hfn crypto.Hash) (h hash.Hash, err error) {
s/hfn/hashFunc/

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:577: if h = hfn.New(); h == nil {
same about New/Available.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
File openpgp/packet/public_key_v3.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:1: // Copyright 2011 The Go Authors. All rights
reserved.
copyright year.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:8: "code.google.com/p/go.crypto/openpgp/errors"
subrepo imports generally go in a block at the end. (I may have been sloppy with
this myself.)

import (
  "crypto"
  ...
  "time"

  "code.google.com/p/go.crypto/openpgp/errors"
)

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:21: // PublicKeyV3 represents older version 3
public keys. These keys are less secure
comma after "older".

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:84: copy(pk.Fingerprint[:],
fingerPrint.Sum(nil))
fingerprint.Sum(pk.Fingerprint[:0])

(can write directly into pk.Fingerprint since it's large enough.)

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:85: pk.KeyId =
binary.BigEndian.Uint64(pk.n.bytes[len(pk.n.bytes)-8 : len(pk.n.bytes)])
the len(pk.n.bytes) after the ':' can be omitted, right?

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:104: E: 0,
E: 0 can be omitted as values are zero by default.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:117: func (pk *PublicKeyV3)
SerializeSignaturePrefix(h hash.Hash) {
s/hash.Hash/io.Writer/

(I've just changed this in the V4 code.)

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
File openpgp/packet/public_key_v3_test.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3_test.go:1: // Copyright 2011 The Go Authors. All
rights reserved.
copyright year.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
File openpgp/packet/signature_v3.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3.go:1: // Copyright 2011 The Go Authors. All rights
reserved.
copyright year.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3.go:8: "code.google.com/p/go.crypto/openpgp/errors"
ditto about subrepo imports in their own block.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
File openpgp/packet/signature_v3_test.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3_test.go:1: // Copyright 2011 The Go Authors. All
rights reserved.
copyright year.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3_test.go:9:
"code.google.com/p/go.crypto/openpgp/armor"
block for subrepo imports.

[cmars, 2013-08-31 06:04:07]

PTAL

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/packet.go
File openpgp/packet/packet.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/packet.go#n...
openpgp/packet/packet.go:301: func peekVersion(r io.Reader) (bufr *bufio.Reader,
ver byte, err error) {
On 2013/08/30 14:43:28, agl1 wrote:
> comment.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/packet.go#n...
openpgp/packet/packet.go:347: }
On 2013/08/30 14:43:28, agl1 wrote:
> isSubkey := tag == packetTypePublicSubkey
> if version < 4 {
>   p = &PublicKeyV3{
>     IsSubkey: isSubkey,
>   }
> } else {
>   p = &PublicKey{
>     IsSubkey: isSubkey,
>   }
> }
> 
> Then the type switch can be dropped.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key.go
File openpgp/packet/public_key.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:527: rsaPublicKey, _ :=
pk.PublicKey.(*rsa.PublicKey)
On 2013/08/30 14:43:28, agl1 wrote:
> , _ not needed.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:533: dsaPublicKey, _ :=
pk.PublicKey.(*dsa.PublicKey)
On 2013/08/30 14:43:28, agl1 wrote:
> ditto.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:551: func keySignatureHash(pk, signed signingKey,
hfn crypto.Hash) (h hash.Hash, err error) {
On 2013/08/30 14:43:28, agl1 wrote:
> s/hfn/hashFunc/

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:552: if h = hfn.New(); h == nil {
On 2013/08/30 14:43:28, agl1 wrote:
> crypto.Hash.New doesn't return nil. I screwed this up.
> 
> if !hashFunc.Available() {
>   return nil, errors.UnsupportedError("hash function")
> }
> h := hashFunc.New()

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:576: func userIdSignatureHash(id string, pk
*PublicKey, hfn crypto.Hash) (h hash.Hash, err error) {
On 2013/08/30 14:43:28, agl1 wrote:
> s/hfn/hashFunc/

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key....
openpgp/packet/public_key.go:577: if h = hfn.New(); h == nil {
On 2013/08/30 14:43:28, agl1 wrote:
> same about New/Available.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
File openpgp/packet/public_key_v3.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:1: // Copyright 2011 The Go Authors. All rights
reserved.
On 2013/08/30 14:43:28, agl1 wrote:
> copyright year.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:8: "code.google.com/p/go.crypto/openpgp/errors"
On 2013/08/30 14:43:28, agl1 wrote:
> subrepo imports generally go in a block at the end. (I may have been sloppy
with
> this myself.)
> 
> import (
>   "crypto"
>   ...
>   "time"
> 
>   "code.google.com/p/go.crypto/openpgp/errors"
> )

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:21: // PublicKeyV3 represents older version 3
public keys. These keys are less secure
On 2013/08/30 14:43:28, agl1 wrote:
> comma after "older".

Done & improved wording a bit.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:84: copy(pk.Fingerprint[:],
fingerPrint.Sum(nil))
On 2013/08/30 14:43:28, agl1 wrote:
> fingerprint.Sum(pk.Fingerprint[:0])
> 
> (can write directly into pk.Fingerprint since it's large enough.)

Done. I like this slice trick.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:85: pk.KeyId =
binary.BigEndian.Uint64(pk.n.bytes[len(pk.n.bytes)-8 : len(pk.n.bytes)])
On 2013/08/30 14:43:28, agl1 wrote:
> the len(pk.n.bytes) after the ':' can be omitted, right?

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:104: E: 0,
On 2013/08/30 14:43:28, agl1 wrote:
> E: 0 can be omitted as values are zero by default.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3.go:117: func (pk *PublicKeyV3)
SerializeSignaturePrefix(h hash.Hash) {
On 2013/08/30 14:43:28, agl1 wrote:
> s/hash.Hash/io.Writer/
> 
> (I've just changed this in the V4 code.)

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
File openpgp/packet/public_key_v3_test.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/public_key_...
openpgp/packet/public_key_v3_test.go:1: // Copyright 2011 The Go Authors. All
rights reserved.
On 2013/08/30 14:43:28, agl1 wrote:
> copyright year.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
File openpgp/packet/signature_v3.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3.go:1: // Copyright 2011 The Go Authors. All rights
reserved.
On 2013/08/30 14:43:28, agl1 wrote:
> copyright year.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3.go:8: "code.google.com/p/go.crypto/openpgp/errors"
On 2013/08/30 14:43:28, agl1 wrote:
> ditto about subrepo imports in their own block.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
File openpgp/packet/signature_v3_test.go (right):

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3_test.go:1: // Copyright 2011 The Go Authors. All
rights reserved.
On 2013/08/30 14:43:28, agl1 wrote:
> copyright year.

Done.

https://codereview.appspot.com/12746046/diff/35001/openpgp/packet/signature_v...
openpgp/packet/signature_v3_test.go:9:
"code.google.com/p/go.crypto/openpgp/armor"
On 2013/08/30 14:43:28, agl1 wrote:
> block for subrepo imports.

Done.

[agl1, 2013-09-03 14:36:22]

*** Submitted as
https://code.google.com/p/go/source/detail?r=387ca0ec2444&repo=crypto ***

openpgp/packet: Add support for V3 public keys and signatures.

My goal here is to allow read-only access to legacy key material without
allowing new V3 keys or signatures.

R=agl
CC=golang-dev
https://codereview.appspot.com/12746046

Committer: Adam Langley <agl@golang.org>