x/crypto/openpgp: cross-signature validation for signing subkeys.

Section 11.1 of RFC4880 requires that binding signatures on
signing subkeys contain a valid embedded signature that cross-certifies
the primary key. This is to avoid the weakness described at
https://www.gnupg.org/faq/subkey-cross-certify.html

Fixes #10740

Change-Id: Ibe039662497832945957b001a83080ba29213703
Reviewed-on: https://go-review.googlesource.com/9799
Reviewed-by: Adam Langley <agl@golang.org>

openpgp/packet/packet.go

@@ -385,16 +385,17 @@ func Read(r io.Reader) (p Packet, err error) {
 type SignatureType uint8
 
 const (
-	SigTypeBinary           SignatureType = 0
-	SigTypeText                           = 1
-	SigTypeGenericCert                    = 0x10
-	SigTypePersonaCert                    = 0x11
-	SigTypeCasualCert                     = 0x12
-	SigTypePositiveCert                   = 0x13
-	SigTypeSubkeyBinding                  = 0x18
-	SigTypeDirectSignature                = 0x1F
-	SigTypeKeyRevocation                  = 0x20
-	SigTypeSubkeyRevocation               = 0x28
+	SigTypeBinary            SignatureType = 0
+	SigTypeText                            = 1
+	SigTypeGenericCert                     = 0x10
+	SigTypePersonaCert                     = 0x11
+	SigTypeCasualCert                      = 0x12
+	SigTypePositiveCert                    = 0x13
+	SigTypeSubkeyBinding                   = 0x18
+	SigTypePrimaryKeyBinding               = 0x19
+	SigTypeDirectSignature                 = 0x1F
+	SigTypeKeyRevocation                   = 0x20
+	SigTypeSubkeyRevocation                = 0x28
 )
 
 // PublicKeyAlgorithm represents the different public key system specified for

openpgp/packet/public_key.go

@@ -16,13 +16,14 @@ import (
 	_ "crypto/sha512"
 	"encoding/binary"
 	"fmt"
-	"golang.org/x/crypto/openpgp/elgamal"
-	"golang.org/x/crypto/openpgp/errors"
 	"hash"
 	"io"
 	"math/big"
 	"strconv"
 	"time"
+
+	"golang.org/x/crypto/openpgp/elgamal"
+	"golang.org/x/crypto/openpgp/errors"
 )
 
 var (
@@ -564,12 +565,33 @@ func keySignatureHash(pk, signed signingKey, hashFunc crypto.Hash) (h hash.Hash,
 
 // VerifyKeySignature returns nil iff sig is a valid signature, made by this
 // public key, of signed.
-func (pk *PublicKey) VerifyKeySignature(signed *PublicKey, sig *Signature) (err error) {
+func (pk *PublicKey) VerifyKeySignature(signed *PublicKey, sig *Signature) error {
 	h, err := keySignatureHash(pk, signed, sig.Hash)
 	if err != nil {
 		return err
 	}
-	return pk.VerifySignature(h, sig)
+	if err = pk.VerifySignature(h, sig); err != nil {
+		return err
+	}
+
+	if sig.FlagSign {
+		// Signing subkeys must be cross-signed. See
+		// https://www.gnupg.org/faq/subkey-cross-certify.html.
+		if sig.EmbeddedSignature == nil {
+			return errors.StructuralError("signing subkey is missing cross-signature")
+		}
+		// Verify the cross-signature. This is calculated over the same
+		// data as the main signature, so we cannot just recursively
+		// call signed.VerifyKeySignature(...)
+		if h, err = keySignatureHash(pk, signed, sig.EmbeddedSignature.Hash); err != nil {
+			return errors.StructuralError("error while hashing for cross-signature: " + err.Error())
+		}
+		if err := signed.VerifySignature(h, sig.EmbeddedSignature); err != nil {
+			return errors.StructuralError("error while verifying cross-signature: " + err.Error())
+		}
+	}
+
+	return nil
 }
 
 func keyRevocationHash(pk signingKey, hashFunc crypto.Hash) (h hash.Hash, err error) {

openpgp/packet/signature.go

@@ -5,6 +5,7 @@
 package packet
 
 import (
+	"bytes"
 	"crypto"
 	"crypto/dsa"
 	"crypto/rsa"
@@ -68,6 +69,11 @@ type Signature struct {
 	// support for MDC subpackets.
 	MDC bool
 
+	// EmbeddedSignature, if non-nil, is a signature of the parent key, by
+	// this key. This prevents an attacker from claiming another's signing
+	// subkey as their own.
+	EmbeddedSignature *Signature
+
 	outSubpackets []outputSubpacket
 }
 
@@ -196,6 +202,7 @@ const (
 	keyFlagsSubpacket            signatureSubpacketType = 27
 	reasonForRevocationSubpacket signatureSubpacketType = 29
 	featuresSubpacket            signatureSubpacketType = 30
+	embeddedSignatureSubpacket   signatureSubpacketType = 32
 )
 
 // parseSignatureSubpacket parses a single subpacket. len(subpacket) is >= 1.
@@ -355,6 +362,24 @@ func parseSignatureSubpacket(sig *Signature, subpacket []byte, isHashed bool) (r
 		// features. In practice, the subpacket is used exclusively to
 		// indicate support for MDC-protected encryption.
 		sig.MDC = len(subpacket) >= 1 && subpacket[0]&1 == 1
+	case embeddedSignatureSubpacket:
+		// Only usage is in signatures that cross-certify
+		// signing subkeys. section 5.2.3.26 describes the
+		// format, with its usage described in section 11.1
+		if sig.EmbeddedSignature != nil {
+			err = errors.StructuralError("Cannot have multiple embedded signatures")
+			return
+		}
+		sig.EmbeddedSignature = new(Signature)
+		// Embedded signatures are required to be v4 signatures see
+		// section 12.1. However, we only parse v4 signatures in this
+		// file anyway.
+		if err := sig.EmbeddedSignature.parse(bytes.NewBuffer(subpacket)); err != nil {
+			return nil, err
+		}
+		if sigType := sig.EmbeddedSignature.SigType; sigType != SigTypePrimaryKeyBinding {
+			return nil, errors.StructuralError("cross-signature has unexpected type " + strconv.Itoa(int(sigType)))
+		}
 	default:
 		if isCritical {
 			err = errors.UnsupportedError("unknown critical signature subpacket type " + strconv.Itoa(int(packetType)))