diff --git a/openpgp/keys.go b/openpgp/keys.go index a79a8c1..efe6e73 100644 --- a/openpgp/keys.go +++ b/openpgp/keys.go @@ -333,7 +333,6 @@ func ReadEntity(packets *packet.Reader) (*Entity, error) { return nil, errors.StructuralError("primary key cannot be used for signatures") } - var current *Identity var revocations []*packet.Signature EachPacket: for { @@ -349,7 +348,7 @@ EachPacket: // Make a new Identity object, that we might wind up throwing away. // We'll only add it if we get a valid self-signature over this // userID. - current = new(Identity) + current := new(Identity) current.Name = pkt.Id current.UserId = pkt @@ -384,11 +383,9 @@ EachPacket: // TODO: RFC4880 5.2.1 permits signatures // directly on keys (eg. to bind additional // revocation keys). - } else if current == nil { - return nil, errors.StructuralError("signature packet found before user id packet") - } else { - current.Signatures = append(current.Signatures, pkt) } + // Else, ignoring the signature as it does not follow anything + // we would know to attach it to. case *packet.PrivateKey: if pkt.IsSubkey == false { packets.Unread(p) @@ -433,26 +430,45 @@ func addSubkey(e *Entity, packets *packet.Reader, pub *packet.PublicKey, priv *p var subKey Subkey subKey.PublicKey = pub subKey.PrivateKey = priv - p, err := packets.Next() - if err == io.EOF { - return io.ErrUnexpectedEOF + + for { + p, err := packets.Next() + if err == io.EOF { + break + } else if err != nil { + return errors.StructuralError("subkey signature invalid: " + err.Error()) + } + + sig, ok := p.(*packet.Signature) + if !ok { + packets.Unread(p) + break + } + + if sig.SigType != packet.SigTypeSubkeyBinding && sig.SigType != packet.SigTypeSubkeyRevocation { + return errors.StructuralError("subkey signature with wrong type") + } + + if err := e.PrimaryKey.VerifyKeySignature(subKey.PublicKey, sig); err != nil { + return errors.StructuralError("subkey signature invalid: " + err.Error()) + } + + switch sig.SigType { + case packet.SigTypeSubkeyRevocation: + subKey.Sig = sig + case packet.SigTypeSubkeyBinding: + if subKey.Sig == nil { + subKey.Sig = sig + } + } } - if err != nil { - return errors.StructuralError("subkey signature invalid: " + err.Error()) - } - var ok bool - subKey.Sig, ok = p.(*packet.Signature) - if !ok { + + if subKey.Sig == nil { return errors.StructuralError("subkey packet not followed by signature") } - if subKey.Sig.SigType != packet.SigTypeSubkeyBinding && subKey.Sig.SigType != packet.SigTypeSubkeyRevocation { - return errors.StructuralError("subkey signature with wrong type") - } - err = e.PrimaryKey.VerifyKeySignature(subKey.PublicKey, subKey.Sig) - if err != nil { - return errors.StructuralError("subkey signature invalid: " + err.Error()) - } + e.Subkeys = append(e.Subkeys, subKey) + return nil } diff --git a/openpgp/keys_test.go b/openpgp/keys_test.go index d877589..46225d4 100644 --- a/openpgp/keys_test.go +++ b/openpgp/keys_test.go @@ -180,6 +180,44 @@ func TestKeyRevocation(t *testing.T) { } } +func TestKeyWithRevokedSubKey(t *testing.T) { + // This key contains a revoked sub key: + // pub rsa1024/0x4CBD826C39074E38 2018-06-14 [SC] + // Key fingerprint = 3F95 169F 3FFA 7D3F 2B47 6F0C 4CBD 826C 3907 4E38 + // uid Golang Gopher + // sub rsa1024/0x945DB1AF61D85727 2018-06-14 [S] [revoked: 2018-06-14] + + keys, err := ReadArmoredKeyRing(bytes.NewBufferString(keyWithSubKey)) + if err != nil { + t.Fatal(err) + } + + if len(keys) != 1 { + t.Fatal("Failed to read key with a sub key") + } + + identity := keys[0].Identities["Golang Gopher "] + + // Test for an issue where Subkey Binding Signatures (RFC 4880 5.2.1) were added to the identity + // preceding the Subkey Packet if the Subkey Packet was followed by more than one signature. + // For example, the current key has the following layout: + // PUBKEY UID SELFSIG SUBKEY REV SELFSIG + // The last SELFSIG would be added to the UID's signatures. This is wrong. + if numIdentitySigs, numExpected := len(identity.Signatures), 0; numIdentitySigs != numExpected { + t.Fatalf("got %d identity signatures, expected %d", numIdentitySigs, numExpected) + } + + if numSubKeys, numExpected := len(keys[0].Subkeys), 1; numSubKeys != numExpected { + t.Fatalf("got %d subkeys, expected %d", numSubKeys, numExpected) + } + + subKey := keys[0].Subkeys[0] + if subKey.Sig == nil { + t.Fatalf("subkey signature is nil") + } + +} + func TestSubkeyRevocation(t *testing.T) { kring, err := ReadKeyRing(readerFromHex(revokedSubkeyHex)) if err != nil { @@ -547,3 +585,31 @@ heiQvzkApQup5c+BhH5zFDFdKJ2CBByxw9+7QjMFI/wgLixKuE0Ob2kAokXf7RlB 7qTZOahrETw= =IKnw -----END PGP PUBLIC KEY BLOCK-----` + +const keyWithSubKey = `-----BEGIN PGP PUBLIC KEY BLOCK----- + +mI0EWyKwKQEEALwXhKBnyaaNFeK3ljfc/qn9X/QFw+28EUfgZPHjRmHubuXLE2uR +s3ZoSXY2z7Dkv+NyHYMt8p+X8q5fR7JvUjK2XbPyKoiJVnHINll83yl67DaWfKNL +EjNoO0kIfbXfCkZ7EG6DL+iKtuxniGTcnGT47e+HJSqb/STpLMnWwXjBABEBAAG0 +I0dvbGFuZyBHb3BoZXIgPG5vLXJlcGx5QGdvbGFuZy5jb20+iM4EEwEKADgWIQQ/ +lRafP/p9PytHbwxMvYJsOQdOOAUCWyKwKQIbAwULCQgHAwUVCgkICwUWAgMBAAIe +AQIXgAAKCRBMvYJsOQdOOOsFBAC62mXww8XuqvYLcVOvHkWLT6mhxrQOJXnlfpn7 +2uBV9CMhoG/Ycd43NONsJrB95Apr9TDIqWnVszNbqPCuBhZQSGLdbiDKjxnCWBk0 +69qv4RNtkpOhYB7jK4s8F5oQZqId6JasT/PmJTH92mhBYhhTQr0GYFuPX2UJdkw9 +Sn9C67iNBFsisDUBBAC3A+Yo9lgCnxi/pfskyLrweYif6kIXWLAtLTsM6g/6jt7b +wTrknuCPyTv0QKGXsAEe/cK/Xq3HvX9WfXPGIHc/X56ZIsHQ+RLowbZV/Lhok1IW +FAuQm8axr/by80cRwFnzhfPc/ukkAq2Qyj4hLsGblu6mxeAhzcp8aqmWOO2H9QAR +AQABiLYEKAEKACAWIQQ/lRafP/p9PytHbwxMvYJsOQdOOAUCWyK16gIdAAAKCRBM +vYJsOQdOOB1vA/4u4uLONsE+2GVOyBsHyy7uTdkuxaR9b54A/cz6jT/tzUbeIzgx +22neWhgvIEghnUZd0vEyK9k1wy5vbDlEo6nKzHso32N1QExGr5upRERAxweDxGOj +7luDwNypI7QcifE64lS/JmlnunwRCdRWMKc0Fp+7jtRc5mpwyHN/Suf5RokBagQY +AQoAIBYhBD+VFp8/+n0/K0dvDEy9gmw5B044BQJbIrA1AhsCAL8JEEy9gmw5B044 +tCAEGQEKAB0WIQSNdnkaWY6t62iX336UXbGvYdhXJwUCWyKwNQAKCRCUXbGvYdhX +JxJSA/9fCPHP6sUtGF1o3G1a3yvOUDGr1JWcct9U+QpbCt1mZoNopCNDDQAJvDWl +mvDgHfuogmgNJRjOMznvahbF+wpTXmB7LS0SK412gJzl1fFIpK4bgnhu0TwxNsO1 +8UkCZWqxRMgcNUn9z6XWONK8dgt5JNvHSHrwF4CxxwjL23AAtK+FA/UUoi3U4kbC +0XnSr1Sl+mrzQi1+H7xyMe7zjqe+gGANtskqexHzwWPUJCPZ5qpIa2l8ghiUim6b +4ymJ+N8/T8Yva1FaPEqfMzzqJr8McYFm0URioXJPvOAlRxdHPteZ0qUopt/Jawxl +Xt6B9h1YpeLoJwjwsvbi98UTRs0jXwoY +=3fWu +-----END PGP PUBLIC KEY BLOCK-----`