Unify key generation self-signature

- Serialized signature sub-packets are now ordered by their ID. - If zlib is preferred for compression, ZIP is also added to the preferences. - The creation time, issuer key ID, and key flag sub-packets are now critical. - The user-ID self-signature is now of type "generic" instead of "positive" certification.
ProtonMail · Jun 20, 2024 · dd431f6 · dd431f6
1 parent f4e1152
commit dd431f6
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 91 deletions.
diff --git a/openpgp/key_generation.go b/openpgp/key_generation.go
@@ -122,6 +122,10 @@ func writeKeyProperties(selfSignature *packet.Signature, creationTime time.Time,
 	selfSignature.PreferredCompression = []uint8{uint8(packet.CompressionNone)}
 	if config.Compression() != packet.CompressionNone {
 		selfSignature.PreferredCompression = append(selfSignature.PreferredCompression, uint8(config.Compression()))
+		if config.Compression() == packet.CompressionZLIB {
+			// If zlib is supported also add zip
+			selfSignature.PreferredCompression = append(selfSignature.PreferredCompression, uint8(packet.CompressionZIP))
+		}
 	}
 
 	// And for DefaultMode.
@@ -151,7 +155,7 @@ func (t *Entity) addUserId(name, comment, email string, config *packet.Config, c
 
 	primary := t.PrivateKey
 	isPrimaryId := len(t.Identities) == 0
-	selfSignature := createSignaturePacket(&primary.PublicKey, packet.SigTypePositiveCert, config)
+	selfSignature := createSignaturePacket(&primary.PublicKey, packet.SigTypeGenericCert, config)
 	if writeProperties {
 		err := writeKeyProperties(selfSignature, creationTime, keyLifetimeSecs, config)
 		if err != nil {

diff --git a/openpgp/packet/signature.go b/openpgp/packet/signature.go
@@ -1182,28 +1182,68 @@ type outputSubpacket struct {
 func (sig *Signature) buildSubpackets(issuer PublicKey) (subpackets []outputSubpacket, err error) {
 	creationTime := make([]byte, 4)
 	binary.BigEndian.PutUint32(creationTime, uint32(sig.CreationTime.Unix()))
-	subpackets = append(subpackets, outputSubpacket{true, creationTimeSubpacket, false, creationTime})
-
+	// Signature Creation Time
+	subpackets = append(subpackets, outputSubpacket{true, creationTimeSubpacket, true, creationTime})
+	// Signature Expiration Time
+	if sig.SigLifetimeSecs != nil && *sig.SigLifetimeSecs != 0 {
+		sigLifetime := make([]byte, 4)
+		binary.BigEndian.PutUint32(sigLifetime, *sig.SigLifetimeSecs)
+		subpackets = append(subpackets, outputSubpacket{true, signatureExpirationSubpacket, true, sigLifetime})
+	}
+	// Trust Signature
+	if sig.TrustLevel != 0 {
+		subpackets = append(subpackets, outputSubpacket{true, trustSubpacket, true, []byte{byte(sig.TrustLevel), byte(sig.TrustAmount)}})
+	}
+	// Regular Expression
+	if sig.TrustRegularExpression != nil {
+		// RFC specifies the string should be null-terminated; add a null byte to the end
+		subpackets = append(subpackets, outputSubpacket{true, regularExpressionSubpacket, true, []byte(*sig.TrustRegularExpression + "\000")})
+	}
+	// Key Expiration Time
+	if sig.KeyLifetimeSecs != nil && *sig.KeyLifetimeSecs != 0 {
+		keyLifetime := make([]byte, 4)
+		binary.BigEndian.PutUint32(keyLifetime, *sig.KeyLifetimeSecs)
+		subpackets = append(subpackets, outputSubpacket{true, keyExpirationSubpacket, true, keyLifetime})
+	}
+	// Preferred Symmetric Ciphers for v1 SEIPD
+	if len(sig.PreferredSymmetric) > 0 {
+		subpackets = append(subpackets, outputSubpacket{true, prefSymmetricAlgosSubpacket, false, sig.PreferredSymmetric})
+	}
+	// Issuer Key ID
 	if sig.IssuerKeyId != nil && sig.Version == 4 {
 		keyId := make([]byte, 8)
 		binary.BigEndian.PutUint64(keyId, *sig.IssuerKeyId)
-		subpackets = append(subpackets, outputSubpacket{true, issuerSubpacket, false, keyId})
+		subpackets = append(subpackets, outputSubpacket{true, issuerSubpacket, true, keyId})
 	}
-	if sig.IssuerFingerprint != nil {
-		contents := append([]uint8{uint8(issuer.Version)}, sig.IssuerFingerprint...)
-		subpackets = append(subpackets, outputSubpacket{true, issuerFingerprintSubpacket, sig.Version >= 5, contents})
+	// Notation Data
+	for _, notation := range sig.Notations {
+		subpackets = append(
+			subpackets,
+			outputSubpacket{
+				true,
+				notationDataSubpacket,
+				notation.IsCritical,
+				notation.getData(),
+			})
 	}
-	if sig.SignerUserId != nil {
-		subpackets = append(subpackets, outputSubpacket{true, signerUserIdSubpacket, false, []byte(*sig.SignerUserId)})
+	// Preferred Hash Algorithms
+	if len(sig.PreferredHash) > 0 {
+		subpackets = append(subpackets, outputSubpacket{true, prefHashAlgosSubpacket, false, sig.PreferredHash})
 	}
-	if sig.SigLifetimeSecs != nil && *sig.SigLifetimeSecs != 0 {
-		sigLifetime := make([]byte, 4)
-		binary.BigEndian.PutUint32(sigLifetime, *sig.SigLifetimeSecs)
-		subpackets = append(subpackets, outputSubpacket{true, signatureExpirationSubpacket, true, sigLifetime})
+	// Preferred Compression Algorithms
+	if len(sig.PreferredCompression) > 0 {
+		subpackets = append(subpackets, outputSubpacket{true, prefCompressionSubpacket, false, sig.PreferredCompression})
 	}
-
+	// 	Primary User ID
+	if sig.IsPrimaryId != nil && *sig.IsPrimaryId {
+		subpackets = append(subpackets, outputSubpacket{true, primaryUserIdSubpacket, false, []byte{1}})
+	}
+	// Policy URI
+	if len(sig.PolicyURI) > 0 {
+		subpackets = append(subpackets, outputSubpacket{true, policyUriSubpacket, false, []uint8(sig.PolicyURI)})
+	}
+	// Key Flags
 	// Key flags may only appear in self-signatures or certification signatures.
-
 	if sig.FlagsValid {
 		var flags byte
 		if sig.FlagCertify {
@@ -1227,80 +1267,56 @@ func (sig *Signature) buildSubpackets(issuer PublicKey) (subpackets []outputSubp
 		if sig.FlagGroupKey {
 			flags |= KeyFlagGroupKey
 		}
-		subpackets = append(subpackets, outputSubpacket{true, keyFlagsSubpacket, false, []byte{flags}})
+		subpackets = append(subpackets, outputSubpacket{true, keyFlagsSubpacket, true, []byte{flags}})
 	}
-
-	for _, notation := range sig.Notations {
-		subpackets = append(
-			subpackets,
-			outputSubpacket{
-				true,
-				notationDataSubpacket,
-				notation.IsCritical,
-				notation.getData(),
-			})
+	// Signer's User ID
+	if sig.SignerUserId != nil {
+		subpackets = append(subpackets, outputSubpacket{true, signerUserIdSubpacket, false, []byte(*sig.SignerUserId)})
 	}
-
-	for _, recipient := range sig.IntendedRecipients {
-		subpackets = append(
-			subpackets,
-			outputSubpacket{
-				true,
-				intendedRecipientSubpacket,
-				false,
-				recipient.Serialize(),
-			})
+	// Reason for Revocation
+	// Revocation reason appears only in revocation signatures and is serialized as per section 5.2.3.23.
+	if sig.RevocationReason != nil {
+		subpackets = append(subpackets, outputSubpacket{true, reasonForRevocationSubpacket, true,
+			append([]uint8{uint8(*sig.RevocationReason)}, []uint8(sig.RevocationReasonText)...)})
 	}
-
-	// The following subpackets may only appear in self-signatures.
-
+	// Features
 	var features = byte(0x00)
 	if sig.SEIPDv1 {
 		features |= 0x01
 	}
 	if sig.SEIPDv2 {
 		features |= 0x08
 	}
-
 	if features != 0x00 {
 		subpackets = append(subpackets, outputSubpacket{true, featuresSubpacket, false, []byte{features}})
 	}
-
-	if sig.TrustLevel != 0 {
-		subpackets = append(subpackets, outputSubpacket{true, trustSubpacket, true, []byte{byte(sig.TrustLevel), byte(sig.TrustAmount)}})
-	}
-
-	if sig.TrustRegularExpression != nil {
-		// RFC specifies the string should be null-terminated; add a null byte to the end
-		subpackets = append(subpackets, outputSubpacket{true, regularExpressionSubpacket, true, []byte(*sig.TrustRegularExpression + "\000")})
-	}
-
-	if sig.KeyLifetimeSecs != nil && *sig.KeyLifetimeSecs != 0 {
-		keyLifetime := make([]byte, 4)
-		binary.BigEndian.PutUint32(keyLifetime, *sig.KeyLifetimeSecs)
-		subpackets = append(subpackets, outputSubpacket{true, keyExpirationSubpacket, true, keyLifetime})
-	}
-
-	if sig.IsPrimaryId != nil && *sig.IsPrimaryId {
-		subpackets = append(subpackets, outputSubpacket{true, primaryUserIdSubpacket, false, []byte{1}})
-	}
-
-	if len(sig.PreferredSymmetric) > 0 {
-		subpackets = append(subpackets, outputSubpacket{true, prefSymmetricAlgosSubpacket, false, sig.PreferredSymmetric})
-	}
-
-	if len(sig.PreferredHash) > 0 {
-		subpackets = append(subpackets, outputSubpacket{true, prefHashAlgosSubpacket, false, sig.PreferredHash})
+	// Embedded Signature
+	// EmbeddedSignature appears only in subkeys capable of signing and is serialized as per section 5.2.3.26.
+	if sig.EmbeddedSignature != nil {
+		var buf bytes.Buffer
+		err = sig.EmbeddedSignature.serializeBody(&buf)
+		if err != nil {
+			return
+		}
+		subpackets = append(subpackets, outputSubpacket{true, embeddedSignatureSubpacket, true, buf.Bytes()})
 	}
-
-	if len(sig.PreferredCompression) > 0 {
-		subpackets = append(subpackets, outputSubpacket{true, prefCompressionSubpacket, false, sig.PreferredCompression})
+	// Issuer Fingerprint
+	if sig.IssuerFingerprint != nil {
+		contents := append([]uint8{uint8(issuer.Version)}, sig.IssuerFingerprint...)
+		subpackets = append(subpackets, outputSubpacket{true, issuerFingerprintSubpacket, sig.Version >= 5, contents})
 	}
-
-	if len(sig.PolicyURI) > 0 {
-		subpackets = append(subpackets, outputSubpacket{true, policyUriSubpacket, false, []uint8(sig.PolicyURI)})
+	// Intended Recipient Fingerprint
+	for _, recipient := range sig.IntendedRecipients {
+		subpackets = append(
+			subpackets,
+			outputSubpacket{
+				true,
+				intendedRecipientSubpacket,
+				false,
+				recipient.Serialize(),
+			})
 	}
-
+	// Preferred AEAD Ciphersuites
 	if len(sig.PreferredCipherSuites) > 0 {
 		serialized := make([]byte, len(sig.PreferredCipherSuites)*2)
 		for i, cipherSuite := range sig.PreferredCipherSuites {
@@ -1309,23 +1325,6 @@ func (sig *Signature) buildSubpackets(issuer PublicKey) (subpackets []outputSubp
 		}
 		subpackets = append(subpackets, outputSubpacket{true, prefCipherSuitesSubpacket, false, serialized})
 	}
-
-	// Revocation reason appears only in revocation signatures and is serialized as per section 5.2.3.23.
-	if sig.RevocationReason != nil {
-		subpackets = append(subpackets, outputSubpacket{true, reasonForRevocationSubpacket, true,
-			append([]uint8{uint8(*sig.RevocationReason)}, []uint8(sig.RevocationReasonText)...)})
-	}
-
-	// EmbeddedSignature appears only in subkeys capable of signing and is serialized as per section 5.2.3.26.
-	if sig.EmbeddedSignature != nil {
-		var buf bytes.Buffer
-		err = sig.EmbeddedSignature.serializeBody(&buf)
-		if err != nil {
-			return
-		}
-		subpackets = append(subpackets, outputSubpacket{true, embeddedSignatureSubpacket, true, buf.Bytes()})
-	}
-
 	return
 }
 

diff --git a/openpgp/v2/key_generation.go b/openpgp/v2/key_generation.go
@@ -193,6 +193,10 @@ func writeKeyProperties(selfSignature *packet.Signature, selectedKeyProperties *
 	selfSignature.PreferredCompression = []uint8{uint8(packet.CompressionNone)}
 	if selectedKeyProperties.compression != packet.CompressionNone {
 		selfSignature.PreferredCompression = append(selfSignature.PreferredCompression, uint8(selectedKeyProperties.compression))
+		if selectedKeyProperties.compression == packet.CompressionZLIB {
+			// If zlib is supported also add zip
+			selfSignature.PreferredCompression = append(selfSignature.PreferredCompression, uint8(packet.CompressionZIP))
+		}
 	}
 
 	// And for DefaultMode.
@@ -222,7 +226,7 @@ func (t *Entity) addUserId(userIdData userIdData, config *packet.Config, selecte
 
 	primary := t.PrivateKey
 	isPrimaryId := len(t.Identities) == 0
-	selfSignature := createSignaturePacket(&primary.PublicKey, packet.SigTypePositiveCert, config)
+	selfSignature := createSignaturePacket(&primary.PublicKey, packet.SigTypeGenericCert, config)
 	if selectedKeyProperties != nil {
 		err := writeKeyProperties(selfSignature, selectedKeyProperties)
 		if err != nil {