crypto/openpgp: better handling of keyrings.

  * Accept armored private key blocks
  * If an armored block is missing, return an InvalidArgumentError,
    rather than ignoring it.
  * If every key in a block is skipped due to being unsupported,
    return the last unsupported error.
  * Include the numeric type of unsupported public keys.
  * Don't assume that the self-signature comes immediately after the
    user id packet.

R=bradfitzgo
CC=golang-dev
https://golang.org/cl/4434048

diff --git a/src/pkg/crypto/openpgp/keys.go b/src/pkg/crypto/openpgp/keys.go
index ecaa86f2828adc9157d44a48591e88ae9e4fcd49..6c03f882831d128ae55db3c860f72eedfd537793 100644 (file)
--- a/src/pkg/crypto/openpgp/keys.go
+++ b/src/pkg/crypto/openpgp/keys.go
@@ -5,6 +5,7 @@
 package openpgp
 
 import (
+       "crypto/openpgp/armor"
        "crypto/openpgp/error"
        "crypto/openpgp/packet"
        "io"
@@ -13,6 +14,8 @@ import (
 
 // PublicKeyType is the armor type for a PGP public key.
 var PublicKeyType = "PGP PUBLIC KEY BLOCK"
+// PrivateKeyType is the armor type for a PGP private key.
+var PrivateKeyType = "PGP PRIVATE KEY BLOCK"
 
 // An Entity represents the components of an OpenPGP key: a primary public key
 // (which must be a signing key), one or more identities claimed by that key,
@@ -101,37 +104,50 @@ func (el EntityList) DecryptionKeys() (keys []Key) {
 
 // ReadArmoredKeyRing reads one or more public/private keys from an armor keyring file.
 func ReadArmoredKeyRing(r io.Reader) (EntityList, os.Error) {
-       body, err := readArmored(r, PublicKeyType)
+       block, err := armor.Decode(r)
+       if err == os.EOF {
+               return nil, error.InvalidArgumentError("no armored data found")
+       }
        if err != nil {
                return nil, err
        }
+       if block.Type != PublicKeyType && block.Type != PrivateKeyType {
+               return nil, error.InvalidArgumentError("expected public or private key block, got: " + block.Type)
+       }
 
-       return ReadKeyRing(body)
+       return ReadKeyRing(block.Body)
 }
 
-// ReadKeyRing reads one or more public/private keys, ignoring unsupported keys.
+// ReadKeyRing reads one or more public/private keys. Unsupported keys are
+// ignored as long as at least a single valid key is found.
 func ReadKeyRing(r io.Reader) (el EntityList, err os.Error) {
        packets := packet.NewReader(r)
+       var lastUnsupportedError os.Error
 
        for {
                var e *Entity
                e, err = readEntity(packets)
                if err != nil {
                        if _, ok := err.(error.UnsupportedError); ok {
+                               lastUnsupportedError = err
                                err = readToNextPublicKey(packets)
                        }
                        if err == os.EOF {
                                err = nil
-                               return
+                               break
                        }
                        if err != nil {
                                el = nil
-                               return
+                               break
                        }
                } else {
                        el = append(el, e)
                }
        }
+
+       if len(el) == 0 && err == nil {
+               err = lastUnsupportedError
+       }
        return
 }
 
@@ -197,25 +213,28 @@ EachPacket:
                        current.Name = pkt.Id
                        current.UserId = pkt
                        e.Identities[pkt.Id] = current
-                       p, err = packets.Next()
-                       if err == os.EOF {
-                               err = io.ErrUnexpectedEOF
-                       }
-                       if err != nil {
-                               if _, ok := err.(error.UnsupportedError); ok {
+
+                       for {
+                               p, err = packets.Next()
+                               if err == os.EOF {
+                                       return nil, io.ErrUnexpectedEOF
+                               } else if err != nil {
                                        return nil, err
                                }
-                               return nil, error.StructuralError("identity self-signature invalid: " + err.String())
-                       }
-                       current.SelfSignature, ok = p.(*packet.Signature)
-                       if !ok {
-                               return nil, error.StructuralError("user ID packet not followed by self signature")
-                       }
-                       if current.SelfSignature.SigType != packet.SigTypePositiveCert {
-                               return nil, error.StructuralError("user ID self-signature with wrong type")
-                       }
-                       if err = e.PrimaryKey.VerifyUserIdSignature(pkt.Id, current.SelfSignature); err != nil {
-                               return nil, error.StructuralError("user ID self-signature invalid: " + err.String())
+
+                               sig, ok := p.(*packet.Signature)
+                               if !ok {
+                                       return nil, error.StructuralError("user ID packet not followed by self-signature")
+                               }
+
+                               if sig.SigType == packet.SigTypePositiveCert && sig.IssuerKeyId != nil && *sig.IssuerKeyId == e.PrimaryKey.KeyId {
+                                       if err = e.PrimaryKey.VerifyUserIdSignature(pkt.Id, sig); err != nil {
+                                               return nil, error.StructuralError("user ID self-signature invalid: " + err.String())
+                                       }
+                                       current.SelfSignature = sig
+                                       break
+                               }
+                               current.Signatures = append(current.Signatures, sig)
                        }
                case *packet.Signature:
                        if current == nil {

diff --git a/src/pkg/crypto/openpgp/packet/public_key.go b/src/pkg/crypto/openpgp/packet/public_key.go
index ebef481fb7f19c6c398b2ba8806b845e10e3e01a..cd4a9aebb6083f40d8410185e97fb52e003ee562 100644 (file)
--- a/src/pkg/crypto/openpgp/packet/public_key.go
+++ b/src/pkg/crypto/openpgp/packet/public_key.go
@@ -15,6 +15,7 @@ import (
        "hash"
        "io"
        "os"
+       "strconv"
 )
 
 // PublicKey represents an OpenPGP public key. See RFC 4880, section 5.5.2.
@@ -47,7 +48,7 @@ func (pk *PublicKey) parse(r io.Reader) (err os.Error) {
        case PubKeyAlgoDSA:
                err = pk.parseDSA(r)
        default:
-               err = error.UnsupportedError("public key type")
+               err = error.UnsupportedError("public key type: " + strconv.Itoa(int(pk.PubKeyAlgo)))
        }
        if err != nil {
                return

diff --git a/src/pkg/crypto/openpgp/read_test.go b/src/pkg/crypto/openpgp/read_test.go
index 6218d9990dd381cefadf82b2416bb348bc3be99a..423c85b0f27b969e686c5c3d793ef390e73eadab 100644 (file)
--- a/src/pkg/crypto/openpgp/read_test.go
+++ b/src/pkg/crypto/openpgp/read_test.go
@@ -230,6 +230,23 @@ func TestDetachedSignatureDSA(t *testing.T) {
        testDetachedSignature(t, kring, readerFromHex(detachedSignatureDSAHex), signedInput, "binary", testKey3KeyId)
 }
 
+func TestReadingArmoredPrivateKey(t *testing.T) {
+       el, err := ReadArmoredKeyRing(bytes.NewBufferString(armoredPrivateKeyBlock))
+       if err != nil {
+               t.Error(err)
+       }
+       if len(el) != 1 {
+               t.Errorf("got %d entities, wanted 1\n", len(el))
+       }
+}
+
+func TestNoArmoredData(t *testing.T) {
+       _, err := ReadArmoredKeyRing(bytes.NewBufferString("foo"))
+       if _, ok := err.(error.InvalidArgumentError); !ok {
+               t.Errorf("error was not an InvalidArgumentError: %s", err)
+       }
+}
+
 const testKey1KeyId = 0xA34D7E18C20C31BB
 const testKey3KeyId = 0x338934250CCC0360
 
@@ -259,3 +276,37 @@ const symmetricallyEncryptedCompressedHex = "8c0d04030302eb4a03808145d0d260c92f7
 const dsaTestKeyHex = "9901a2044d6c49de110400cb5ce438cf9250907ac2ba5bf6547931270b89f7c4b53d9d09f4d0213a5ef2ec1f26806d3d259960f872a4a102ef1581ea3f6d6882d15134f21ef6a84de933cc34c47cc9106efe3bd84c6aec12e78523661e29bc1a61f0aab17fa58a627fd5fd33f5149153fbe8cd70edf3d963bc287ef875270ff14b5bfdd1bca4483793923b00a0fe46d76cb6e4cbdc568435cd5480af3266d610d303fe33ae8273f30a96d4d34f42fa28ce1112d425b2e3bf7ea553d526e2db6b9255e9dc7419045ce817214d1a0056dbc8d5289956a4b1b69f20f1105124096e6a438f41f2e2495923b0f34b70642607d45559595c7fe94d7fa85fc41bf7d68c1fd509ebeaa5f315f6059a446b9369c277597e4f474a9591535354c7e7f4fd98a08aa60400b130c24ff20bdfbf683313f5daebf1c9b34b3bdadfc77f2ddd72ee1fb17e56c473664bc21d66467655dd74b9005e3a2bacce446f1920cd7017231ae447b67036c9b431b8179deacd5120262d894c26bc015bffe3d827ba7087ad9b700d2ca1f6d16cc1786581e5dd065f293c31209300f9b0afcc3f7c08dd26d0a22d87580b4db41054657374204b65792033202844534129886204131102002205024d6c49de021b03060b090807030206150802090a0b0416020301021e01021780000a0910338934250ccc03607e0400a0bdb9193e8a6b96fc2dfc108ae848914b504481f100a09c4dc148cb693293a67af24dd40d2b13a9e36794"
 
 const dsaTestKeyPrivateHex = "9501bb044d6c49de110400cb5ce438cf9250907ac2ba5bf6547931270b89f7c4b53d9d09f4d0213a5ef2ec1f26806d3d259960f872a4a102ef1581ea3f6d6882d15134f21ef6a84de933cc34c47cc9106efe3bd84c6aec12e78523661e29bc1a61f0aab17fa58a627fd5fd33f5149153fbe8cd70edf3d963bc287ef875270ff14b5bfdd1bca4483793923b00a0fe46d76cb6e4cbdc568435cd5480af3266d610d303fe33ae8273f30a96d4d34f42fa28ce1112d425b2e3bf7ea553d526e2db6b9255e9dc7419045ce817214d1a0056dbc8d5289956a4b1b69f20f1105124096e6a438f41f2e2495923b0f34b70642607d45559595c7fe94d7fa85fc41bf7d68c1fd509ebeaa5f315f6059a446b9369c277597e4f474a9591535354c7e7f4fd98a08aa60400b130c24ff20bdfbf683313f5daebf1c9b34b3bdadfc77f2ddd72ee1fb17e56c473664bc21d66467655dd74b9005e3a2bacce446f1920cd7017231ae447b67036c9b431b8179deacd5120262d894c26bc015bffe3d827ba7087ad9b700d2ca1f6d16cc1786581e5dd065f293c31209300f9b0afcc3f7c08dd26d0a22d87580b4d00009f592e0619d823953577d4503061706843317e4fee083db41054657374204b65792033202844534129886204131102002205024d6c49de021b03060b090807030206150802090a0b0416020301021e01021780000a0910338934250ccc03607e0400a0bdb9193e8a6b96fc2dfc108ae848914b504481f100a09c4dc148cb693293a67af24dd40d2b13a9e36794"
+
+const armoredPrivateKeyBlock = `-----BEGIN PGP PRIVATE KEY BLOCK-----
+Version: GnuPG v1.4.10 (GNU/Linux)
+
+lQHYBE2rFNoBBADFwqWQIW/DSqcB4yCQqnAFTJ27qS5AnB46ccAdw3u4Greeu3Bp
+idpoHdjULy7zSKlwR1EA873dO/k/e11Ml3dlAFUinWeejWaK2ugFP6JjiieSsrKn
+vWNicdCS4HTWn0X4sjl0ZiAygw6GNhqEQ3cpLeL0g8E9hnYzJKQ0LWJa0QARAQAB
+AAP/TB81EIo2VYNmTq0pK1ZXwUpxCrvAAIG3hwKjEzHcbQznsjNvPUihZ+NZQ6+X
+0HCfPAdPkGDCLCb6NavcSW+iNnLTrdDnSI6+3BbIONqWWdRDYJhqZCkqmG6zqSfL
+IdkJgCw94taUg5BWP/AAeQrhzjChvpMQTVKQL5mnuZbUCeMCAN5qrYMP2S9iKdnk
+VANIFj7656ARKt/nf4CBzxcpHTyB8+d2CtPDKCmlJP6vL8t58Jmih+kHJMvC0dzn
+gr5f5+sCAOOe5gt9e0am7AvQWhdbHVfJU0TQJx+m2OiCJAqGTB1nvtBLHdJnfdC9
+TnXXQ6ZXibqLyBies/xeY2sCKL5qtTMCAKnX9+9d/5yQxRyrQUHt1NYhaXZnJbHx
+q4ytu0eWz+5i68IYUSK69jJ1NWPM0T6SkqpB3KCAIv68VFm9PxqG1KmhSrQIVGVz
+dCBLZXmIuAQTAQIAIgUCTasU2gIbAwYLCQgHAwIGFQgCCQoLBBYCAwECHgECF4AA
+CgkQO9o98PRieSoLhgQAkLEZex02Qt7vGhZzMwuN0R22w3VwyYyjBx+fM3JFETy1
+ut4xcLJoJfIaF5ZS38UplgakHG0FQ+b49i8dMij0aZmDqGxrew1m4kBfjXw9B/v+
+eIqpODryb6cOSwyQFH0lQkXC040pjq9YqDsO5w0WYNXYKDnzRV0p4H1pweo2VDid
+AdgETasU2gEEAN46UPeWRqKHvA99arOxee38fBt2CI08iiWyI8T3J6ivtFGixSqV
+bRcPxYO/qLpVe5l84Nb3X71GfVXlc9hyv7CD6tcowL59hg1E/DC5ydI8K8iEpUmK
+/UnHdIY5h8/kqgGxkY/T/hgp5fRQgW1ZoZxLajVlMRZ8W4tFtT0DeA+JABEBAAEA
+A/0bE1jaaZKj6ndqcw86jd+QtD1SF+Cf21CWRNeLKnUds4FRRvclzTyUMuWPkUeX
+TaNNsUOFqBsf6QQ2oHUBBK4VCHffHCW4ZEX2cd6umz7mpHW6XzN4DECEzOVksXtc
+lUC1j4UB91DC/RNQqwX1IV2QLSwssVotPMPqhOi0ZLNY7wIA3n7DWKInxYZZ4K+6
+rQ+POsz6brEoRHwr8x6XlHenq1Oki855pSa1yXIARoTrSJkBtn5oI+f8AzrnN0BN
+oyeQAwIA/7E++3HDi5aweWrViiul9cd3rcsS0dEnksPhvS0ozCJiHsq/6GFmy7J8
+QSHZPteedBnZyNp5jR+H7cIfVN3KgwH/Skq4PsuPhDq5TKK6i8Pc1WW8MA6DXTdU
+nLkX7RGmMwjC0DBf7KWAlPjFaONAX3a8ndnz//fy1q7u2l9AZwrj1qa1iJ8EGAEC
+AAkFAk2rFNoCGwwACgkQO9o98PRieSo2/QP/WTzr4ioINVsvN1akKuekmEMI3LAp
+BfHwatufxxP1U+3Si/6YIk7kuPB9Hs+pRqCXzbvPRrI8NHZBmc8qIGthishdCYad
+AHcVnXjtxrULkQFGbGvhKURLvS9WnzD/m1K2zzwxzkPTzT9/Yf06O6Mal5AdugPL
+VrM0m72/jnpKo04=
+=zNCn
+-----END PGP PRIVATE KEY BLOCK-----`