Unify key generation self-signature (#208)

- Serialized signature sub-packets are now ordered by their ID. - The creation time, issuer key ID, and key flag sub-packets are now critical.
ProtonMail · Jun 24, 2024 · 7f9da72 · 7f9da72
1 parent dc5f72b
commit 7f9da72
Showing 1 changed file with 88 additions and 89 deletions.
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
 }