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

3 files changed, 94 insertions, 23 deletions

diff --git a/src/pkg/crypto/openpgp/keys.go b/src/pkg/crypto/openpgp/keys.go
index ecaa86f282..6c03f88283 100644
--- 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 ebef481fb7..cd4a9aebb6 100644
--- 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 6218d9990d..423c85b0f2 100644
--- 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-----`