From 5720941c69e707f5ec491eba3e04d634d8bda559 Mon Sep 17 00:00:00 2001 From: Lukas Burkhalter <10532077+lubux@users.noreply.github.com> Date: Fri, 14 Jun 2024 10:41:38 +0200 Subject: [PATCH] feat(armor): Rework armor checksum handling (#284) * feat(armor): Armor checksum handling according to the crypto refresh * docs(readme): Add info about v2 support * refactor(armor): Improve checksum naming * refactor(armor): Rename global checksum setting variable --- CHANGELOG.md | 8 +++ README.md | 9 +++ armor/armor.go | 48 ++++++++++++--- constants/armor.go | 21 ++++--- crypto/encrypt_decrypt_test.go | 30 ++++++++++ crypto/encryption_handle.go | 104 +++++++++++++++++++++++++-------- crypto/key.go | 17 ++++-- crypto/message.go | 18 +++++- crypto/sign_handle.go | 25 ++++++-- crypto/sign_verify_test.go | 19 ++++++ 10 files changed, 246 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50909985..ef50558c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased +### Added +- API to armor data with the option to remove the checksum + +### Changed +- All armor functions append a checksum per default for compatibility with certain libraries although the crypto-refresh advises not to. +- `Encryption` and `Sign` handle now append a checksum when armoring. If the produced OpenPGP packets are crypto-refresh packets, the checksum is not appended as mandated by the crypto-refresh. + ## [3.0.0-alpha.2] 2024-04-12 ### Added - API to serialize KeyRings to binary data: diff --git a/README.md b/README.md index 4488e487..a67a9f48 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,7 @@ crypto library](https://github.com/ProtonMail/go-crypto/tree/version-2). - [GopenPGP V3](#gopenpgp-v3) + - [GopenPGP V2 support](#gopenpgp-v2-support) - [Download/Install](#downloadinstall) - [Documentation](#documentation) - [Examples](#examples) @@ -21,6 +22,14 @@ crypto library](https://github.com/ProtonMail/go-crypto/tree/version-2). +## GopenPGP V2 support + +While GopenPGP V3 introduces a new API with significant enhancements, it is not backward compatible with GopenPGP V2. +Although we recommend upgrading to V3 for the latest features and improvements, we continue to support GopenPGP V2. +Our support includes ongoing bug fixes and minor feature updates to ensure stability and functionality for existing users. + +You can access GopenPGP V2 on the [v2 branch of this repository](https://github.com/ProtonMail/gopenpgp/tree/v2). + ## Download/Install To use GopenPGP with [Go Modules](https://github.com/golang/go/wiki/Modules) just run diff --git a/armor/armor.go b/armor/armor.go index 37043950..9a9a7c18 100644 --- a/armor/armor.go +++ b/armor/armor.go @@ -20,7 +20,14 @@ func ArmorKey(input []byte) (string, error) { // ArmorWriterWithType returns a io.WriteCloser which, when written to, writes // armored data to w with the given armorType. func ArmorWriterWithType(w io.Writer, armorType string) (io.WriteCloser, error) { - return armor.EncodeWithChecksumOption(w, armorType, internal.ArmorHeaders, false) + return armor.EncodeWithChecksumOption(w, armorType, internal.ArmorHeaders, constants.ArmorChecksumEnabled) +} + +// ArmorWriterWithTypeChecksum returns a io.WriteCloser which, when written to, writes +// armored data to w with the given armorType. +// The checksum determines if an armor checksum is written at the end. +func ArmorWriterWithTypeChecksum(w io.Writer, armorType string, checksum bool) (io.WriteCloser, error) { + return armor.EncodeWithChecksumOption(w, armorType, internal.ArmorHeaders, checksum) } // ArmorWriterWithTypeAndCustomHeaders returns a io.WriteCloser, @@ -33,12 +40,18 @@ func ArmorWriterWithTypeAndCustomHeaders(w io.Writer, armorType, version, commen if comment != "" { headers["Comment"] = comment } - return armor.EncodeWithChecksumOption(w, armorType, headers, false) + return armor.EncodeWithChecksumOption(w, armorType, headers, constants.ArmorChecksumEnabled) } // ArmorWithType armors input with the given armorType. func ArmorWithType(input []byte, armorType string) (string, error) { - buffer, err := armorWithTypeAndHeaders(input, armorType, internal.ArmorHeaders) + return ArmorWithTypeChecksum(input, armorType, constants.ArmorChecksumEnabled) +} + +// ArmorWithTypeChecksum armors input with the given armorType. +// The checksum option determines if an armor checksum is written at the end. +func ArmorWithTypeChecksum(input []byte, armorType string, checksum bool) (string, error) { + buffer, err := armorWithTypeAndHeaders(input, armorType, internal.ArmorHeaders, checksum) if err != nil { return "", err } @@ -47,7 +60,12 @@ func ArmorWithType(input []byte, armorType string) (string, error) { // ArmorWithTypeBytes armors input with the given armorType. func ArmorWithTypeBytes(input []byte, armorType string) ([]byte, error) { - buffer, err := armorWithTypeAndHeaders(input, armorType, internal.ArmorHeaders) + return ArmorWithTypeBytesChecksum(input, armorType, constants.ArmorChecksumEnabled) +} + +// ArmorWithTypeBytesChecksum armors input with the given armorType and checksum option. +func ArmorWithTypeBytesChecksum(input []byte, armorType string, checksum bool) ([]byte, error) { + buffer, err := armorWithTypeAndHeaders(input, armorType, internal.ArmorHeaders, checksum) if err != nil { return nil, err } @@ -57,6 +75,12 @@ func ArmorWithTypeBytes(input []byte, armorType string) ([]byte, error) { // ArmorWithTypeAndCustomHeaders armors input with the given armorType and // headers. func ArmorWithTypeAndCustomHeaders(input []byte, armorType, version, comment string) (string, error) { + return ArmorWithTypeAndCustomHeadersChecksum(input, armorType, version, comment, constants.ArmorChecksumEnabled) +} + +// ArmorWithTypeAndCustomHeadersChecksum armors input with the given armorType and +// headers and checksum option. +func ArmorWithTypeAndCustomHeadersChecksum(input []byte, armorType, version, comment string, checksum bool) (string, error) { headers := make(map[string]string) if version != "" { headers["Version"] = version @@ -64,7 +88,7 @@ func ArmorWithTypeAndCustomHeaders(input []byte, armorType, version, comment str if comment != "" { headers["Comment"] = comment } - buffer, err := armorWithTypeAndHeaders(input, armorType, headers) + buffer, err := armorWithTypeAndHeaders(input, armorType, headers, checksum) if err != nil { return "", err } @@ -81,7 +105,7 @@ func ArmorWithTypeAndCustomHeadersBytes(input []byte, armorType, version, commen if comment != "" { headers["Comment"] = comment } - buffer, err := armorWithTypeAndHeaders(input, armorType, headers) + buffer, err := armorWithTypeAndHeaders(input, armorType, headers, constants.ArmorChecksumEnabled) if err != nil { return nil, err } @@ -132,6 +156,14 @@ func ArmorPGPMessage(signature []byte) (string, error) { return ArmorWithType(signature, constants.PGPMessageHeader) } +func ArmorPGPMessageBytesChecksum(signature []byte, checksum bool) ([]byte, error) { + return ArmorWithTypeBytesChecksum(signature, constants.PGPMessageHeader, checksum) +} + +func ArmorPGPMessageChecksum(signature []byte, checksum bool) (string, error) { + return ArmorWithTypeChecksum(signature, constants.PGPMessageHeader, checksum) +} + const armorPrefix = "-----BEGIN PGP" const maxGarbageBytes = 128 @@ -150,10 +182,10 @@ func IsPGPArmored(in io.Reader) (io.Reader, bool) { return outReader, false } -func armorWithTypeAndHeaders(input []byte, armorType string, headers map[string]string) (*bytes.Buffer, error) { +func armorWithTypeAndHeaders(input []byte, armorType string, headers map[string]string, writeChecksum bool) (*bytes.Buffer, error) { var b bytes.Buffer - w, err := armor.EncodeWithChecksumOption(&b, armorType, headers, false) + w, err := armor.EncodeWithChecksumOption(&b, armorType, headers, writeChecksum) if err != nil { return nil, errors.Wrap(err, "armor: unable to encode armoring") diff --git a/constants/armor.go b/constants/armor.go index 420054c9..e72d8871 100644 --- a/constants/armor.go +++ b/constants/armor.go @@ -3,11 +3,18 @@ package constants // Constants for armored data. const ( - ArmorHeaderEnabled = false // can be enabled for debugging at compile time only - ArmorHeaderVersion = "GopenPGP " + Version - ArmorHeaderComment = "https://gopenpgp.org" - PGPMessageHeader = "PGP MESSAGE" - PGPSignatureHeader = "PGP SIGNATURE" - PublicKeyHeader = "PGP PUBLIC KEY BLOCK" - PrivateKeyHeader = "PGP PRIVATE KEY BLOCK" + // ArmorChecksumEnabled defines the default behavior for adding an armor checksum + // to an armored message. + // + // If set to true, an armor checksum is added to the message. + // + // If set to false, no armor checksum is added. + ArmorChecksumEnabled = true + ArmorHeaderEnabled = false // can be enabled for debugging at compile time only + ArmorHeaderVersion = "GopenPGP " + Version + ArmorHeaderComment = "https://gopenpgp.org" + PGPMessageHeader = "PGP MESSAGE" + PGPSignatureHeader = "PGP SIGNATURE" + PublicKeyHeader = "PGP PUBLIC KEY BLOCK" + PrivateKeyHeader = "PGP PRIVATE KEY BLOCK" ) diff --git a/crypto/encrypt_decrypt_test.go b/crypto/encrypt_decrypt_test.go index a415df38..5eae4c6c 100644 --- a/crypto/encrypt_decrypt_test.go +++ b/crypto/encrypt_decrypt_test.go @@ -6,6 +6,7 @@ import ( "io" "os" "reflect" + "regexp" "strings" "testing" @@ -1006,6 +1007,35 @@ func TestEncryptDecryptPlaintextDetachedArmor(t *testing.T) { } } +func TestEncryptArmor(t *testing.T) { + for _, material := range testMaterialForProfiles { + t.Run(material.profileName, func(t *testing.T) { + isV6 := material.keyRingTestPublic.GetKeys()[0].isV6() + encHandle, _ := material.pgp.Encryption(). + Recipients(material.keyRingTestPublic). + SigningKeys(material.keyRingTestPrivate). + New() + pgpMsg, err := encHandle.Encrypt([]byte(testMessageString)) + if err != nil { + t.Fatal("Expected no error in encryption, got:", err) + } + armoredData, err := pgpMsg.Armor() + if err != nil { + t.Fatal("Armoring failed, got:", err) + } + hasChecksum := containsChecksum(armoredData) + if isV6 && hasChecksum { + t.Fatalf("V6 messages should not have a checksum") + } + }) + } +} + +func containsChecksum(armored string) bool { + re := regexp.MustCompile(`=([A-Za-z0-9+/]{4})\s*-----END PGP MESSAGE-----`) + return re.MatchString(armored) +} + func testEncryptDecrypt( t *testing.T, messageBytes []byte, diff --git a/crypto/encryption_handle.go b/crypto/encryption_handle.go index 3205b0dc..b58851e8 100644 --- a/crypto/encryption_handle.go +++ b/crypto/encryption_handle.go @@ -109,7 +109,8 @@ func (eh *encryptionHandle) Encrypt(message []byte) (*PGPMessage, error) { if err != nil { return nil, err } - return pgpMessageBuffer.PGPMessageWithDetached(eh.PlainDetachedSignature), nil + checksum := eh.armorChecksumRequired() + return pgpMessageBuffer.PGPMessageWithOptions(eh.PlainDetachedSignature, !checksum), nil } // EncryptSessionKey encrypts a session key with the encryption handle. @@ -150,6 +151,44 @@ func (eh *encryptionHandle) validate() error { return nil } +// armorChecksumRequired determines if an armor checksum should be appended or not. +// The OpenPGP Crypto-Refresh mandates that no checksum should be appended with the new packets. +func (eh *encryptionHandle) armorChecksumRequired() bool { + if !constants.ArmorChecksumEnabled { + // If the default behavior is no checksum, we can ignore + // the logic for the crypto refresh check. + return false + } + encryptionConfig := eh.profile.EncryptionConfig() + if encryptionConfig.AEADConfig == nil { + return true + } + checkTime := eh.clock() + if eh.Recipients != nil { + for _, recipient := range eh.Recipients.entities { + primarySelfSignature, err := recipient.PrimarySelfSignature(checkTime) + if err != nil { + return true + } + if !primarySelfSignature.SEIPDv2 { + return true + } + } + } + if eh.HiddenRecipients != nil { + for _, recipient := range eh.HiddenRecipients.entities { + primarySelfSignature, err := recipient.PrimarySelfSignature(checkTime) + if err != nil { + return true + } + if !primarySelfSignature.SEIPDv2 { + return true + } + } + } + return false +} + type armoredWriteCloser struct { armorWriter WriteCloser messageWriter WriteCloser @@ -185,11 +224,47 @@ func (eh *encryptionHandle) ClearPrivateParams() { } } +func (eh *encryptionHandle) handleArmor(keys, data, detachedSignature Writer) ( + dataOut Writer, + detachedSignatureOut Writer, + armorWriter WriteCloser, + armorSigWriter WriteCloser, + err error, +) { + writeChecksum := eh.armorChecksumRequired() + detachedSignatureOut = detachedSignature + // Wrap armored writer + if eh.ArmorHeaders == nil { + eh.ArmorHeaders = internal.ArmorHeaders + } + armorWriter, err = armor.EncodeWithChecksumOption(data, constants.PGPMessageHeader, eh.ArmorHeaders, writeChecksum) + dataOut = armorWriter + if err != nil { + return nil, nil, nil, nil, err + } + if eh.DetachedSignature { + armorSigWriter, err = armor.EncodeWithChecksumOption(detachedSignature, constants.PGPMessageHeader, eh.ArmorHeaders, writeChecksum) + detachedSignatureOut = armorSigWriter + if err != nil { + return nil, nil, nil, nil, err + } + } else if eh.PlainDetachedSignature { + armorSigWriter, err = armor.EncodeWithChecksumOption(detachedSignature, constants.PGPSignatureHeader, eh.ArmorHeaders, writeChecksum) + detachedSignatureOut = armorSigWriter + if err != nil { + return nil, nil, nil, nil, err + } + } + if keys != nil { + return nil, nil, nil, nil, errors.New("gopenpgp: armor is not allowed if key packets are written separately") + } + return dataOut, detachedSignatureOut, armorWriter, armorSigWriter, nil +} + func (eh *encryptionHandle) encryptingWriters(keys, data, detachedSignature Writer, meta *LiteralMetadata, armorOutput bool) (messageWriter WriteCloser, err error) { var armorWriter WriteCloser var armorSigWriter WriteCloser - err = eh.validate() - if err != nil { + if err = eh.validate(); err != nil { return nil, err } @@ -199,31 +274,10 @@ func (eh *encryptionHandle) encryptingWriters(keys, data, detachedSignature Writ } if armorOutput { - // Wrap armored writer - if eh.ArmorHeaders == nil { - eh.ArmorHeaders = internal.ArmorHeaders - } - armorWriter, err = armor.EncodeWithChecksumOption(data, constants.PGPMessageHeader, eh.ArmorHeaders, false) - data = armorWriter + data, detachedSignature, armorWriter, armorSigWriter, err = eh.handleArmor(keys, data, detachedSignature) if err != nil { return nil, err } - if eh.DetachedSignature { - armorSigWriter, err = armor.EncodeWithChecksumOption(detachedSignature, constants.PGPMessageHeader, eh.ArmorHeaders, false) - detachedSignature = armorSigWriter - if err != nil { - return nil, err - } - } else if eh.PlainDetachedSignature { - armorSigWriter, err = armor.EncodeWithChecksumOption(detachedSignature, constants.PGPSignatureHeader, eh.ArmorHeaders, false) - detachedSignature = armorSigWriter - if err != nil { - return nil, err - } - } - if keys != nil { - return nil, errors.New("gopenpgp: armor is not allowed if key packets are written separately") - } } if keys == nil { // No writer for key packets provided, diff --git a/crypto/key.go b/crypto/key.go index f11a2623..df5669a9 100644 --- a/crypto/key.go +++ b/crypto/key.go @@ -224,10 +224,10 @@ func (key *Key) Armor() (string, error) { } if key.IsPrivate() { - return armor.ArmorWithType(serialized, constants.PrivateKeyHeader) + return armor.ArmorWithTypeChecksum(serialized, constants.PrivateKeyHeader, !key.isV6()) } - return armor.ArmorWithType(serialized, constants.PublicKeyHeader) + return armor.ArmorWithTypeChecksum(serialized, constants.PublicKeyHeader, !key.isV6()) } // ArmorWithCustomHeaders returns the armored key as a string, with @@ -238,7 +238,7 @@ func (key *Key) ArmorWithCustomHeaders(comment, version string) (string, error) return "", err } - return armor.ArmorWithTypeAndCustomHeaders(serialized, constants.PrivateKeyHeader, version, comment) + return armor.ArmorWithTypeAndCustomHeadersChecksum(serialized, constants.PrivateKeyHeader, version, comment, !key.isV6()) } // GetArmoredPublicKey returns the armored public keys from this keyring. @@ -248,7 +248,7 @@ func (key *Key) GetArmoredPublicKey() (s string, err error) { return "", err } - return armor.ArmorWithType(serialized, constants.PublicKeyHeader) + return armor.ArmorWithTypeChecksum(serialized, constants.PublicKeyHeader, !key.isV6()) } // GetArmoredPublicKeyWithCustomHeaders returns the armored public key as a string, with @@ -259,7 +259,7 @@ func (key *Key) GetArmoredPublicKeyWithCustomHeaders(comment, version string) (s return "", err } - return armor.ArmorWithTypeAndCustomHeaders(serialized, constants.PublicKeyHeader, version, comment) + return armor.ArmorWithTypeAndCustomHeadersChecksum(serialized, constants.PublicKeyHeader, version, comment, !key.isV6()) } // GetPublicKey returns the unarmored public keys from this keyring. @@ -433,6 +433,13 @@ func (key *Key) ToPublic() (publicKey *Key, err error) { return } +func (key *Key) isV6() bool { + if key == nil || key.entity == nil { + return false + } + return key.entity.PrimaryKey.Version == 6 +} + // --- Internal methods // getSHA256FingerprintBytes computes the SHA256 fingerprint of a public key diff --git a/crypto/message.go b/crypto/message.go index 6f737cdf..29cd38b0 100644 --- a/crypto/message.go +++ b/crypto/message.go @@ -36,6 +36,8 @@ type PGPMessage struct { DetachedSignature []byte // detachedSignatureIsPlain indicates if the detached signature is not encrypted. detachedSignatureIsPlain bool + // Signals that no armor checksum must be appended when armoring + omitArmorChecksum bool } type PGPMessageBuffer struct { @@ -139,6 +141,9 @@ func (msg *PGPMessage) Armor() (string, error) { if msg.KeyPacket == nil { return "", errors.New("gopenpgp: missing key packets in pgp message") } + if msg.omitArmorChecksum { + return armor.ArmorPGPMessageChecksum(msg.Bytes(), false) + } return armor.ArmorPGPMessage(msg.Bytes()) } @@ -147,12 +152,18 @@ func (msg *PGPMessage) ArmorBytes() ([]byte, error) { if msg.KeyPacket == nil { return nil, errors.New("gopenpgp: missing key packets in pgp message") } + if msg.omitArmorChecksum { + return armor.ArmorPGPMessageBytesChecksum(msg.Bytes(), false) + } return armor.ArmorPGPMessageBytes(msg.Bytes()) } // ArmorWithCustomHeaders returns the armored message as a string, with // the given headers. Empty parameters are omitted from the headers. func (msg *PGPMessage) ArmorWithCustomHeaders(comment, version string) (string, error) { + if msg.omitArmorChecksum { + return armor.ArmorWithTypeAndCustomHeadersChecksum(msg.Bytes(), constants.PGPMessageHeader, version, comment, false) + } return armor.ArmorWithTypeAndCustomHeaders(msg.Bytes(), constants.PGPMessageHeader, version, comment) } @@ -355,12 +366,12 @@ func (mb *PGPMessageBuffer) Write(b []byte) (n int, err error) { // PGPMessage returns the PGPMessage extracted from the internal buffers. func (mb *PGPMessageBuffer) PGPMessage() *PGPMessage { - return mb.PGPMessageWithDetached(false) + return mb.PGPMessageWithOptions(false, false) } -// PGPMessageWithDetached returns the PGPMessage extracted from the internal buffers. +// PGPMessageWithOptions returns the PGPMessage extracted from the internal buffers. // The isPlain flag indicates wether the detached signature is encrypted or plaintext, if any. -func (mb *PGPMessageBuffer) PGPMessageWithDetached(isPlain bool) *PGPMessage { +func (mb *PGPMessageBuffer) PGPMessageWithOptions(isPlain, omitArmorChecksum bool) *PGPMessage { var detachedSignature []byte if mb.signature.Len() > 0 { detachedSignature = mb.signature.Bytes() @@ -375,6 +386,7 @@ func (mb *PGPMessageBuffer) PGPMessageWithDetached(isPlain bool) *PGPMessage { DataPacket: mb.data.Bytes(), DetachedSignature: detachedSignature, detachedSignatureIsPlain: isPlain, + omitArmorChecksum: omitArmorChecksum, } } diff --git a/crypto/sign_handle.go b/crypto/sign_handle.go index 32c2eca3..70b8468f 100644 --- a/crypto/sign_handle.go +++ b/crypto/sign_handle.go @@ -46,19 +46,17 @@ func (sh *signatureHandle) SigningWriter(outputWriter Writer, encoding int8) (me var armorWriter WriteCloser armorOutput := armorOutput(encoding) if armorOutput { + writeChecksum := sh.armorChecksumRequired() var err error header := constants.PGPMessageHeader if sh.Detached { header = constants.PGPSignatureHeader - // Append checksum for GnuPG detached signature compatibility - armorWriter, err = armor.EncodeWithChecksumOption(outputWriter, header, sh.ArmorHeaders, true) - } else { - armorWriter, err = armor.EncodeWithChecksumOption(outputWriter, header, sh.ArmorHeaders, false) } - outputWriter = armorWriter + armorWriter, err = armor.EncodeWithChecksumOption(outputWriter, header, sh.ArmorHeaders, writeChecksum) if err != nil { return nil, err } + outputWriter = armorWriter } if sh.Detached { // Detached signature @@ -133,6 +131,23 @@ func (sh *signatureHandle) validate() error { return nil } +func (sh *signatureHandle) armorChecksumRequired() bool { + if !constants.ArmorChecksumEnabled { + // If the default behavior is no checksum, we can ignore + // the logic for the crypto refresh check. + return false + } + if sh.SignKeyRing == nil { + return true + } + for _, signer := range sh.SignKeyRing.entities { + if signer.PrimaryKey.Version != 6 { + return true + } + } + return false +} + func (sh *signatureHandle) signCleartext(message []byte) ([]byte, error) { config := sh.profile.SignConfig() config.Time = NewConstantClock(sh.clock().Unix()) diff --git a/crypto/sign_verify_test.go b/crypto/sign_verify_test.go index d045e470..b9289ed9 100644 --- a/crypto/sign_verify_test.go +++ b/crypto/sign_verify_test.go @@ -178,6 +178,25 @@ func TestSignVerifyCleartext(t *testing.T) { } } +func TestSignArmor(t *testing.T) { + for _, material := range testMaterialForProfiles { + t.Run(material.profileName, func(t *testing.T) { + isV6 := material.keyRingTestPrivate.GetKeys()[0].isV6() + signer, _ := material.pgp.Sign(). + SigningKeys(material.keyRingTestPrivate). + New() + armoredSignature, err := signer.Sign([]byte(testMessageString), Armor) + if err != nil { + t.Fatal("Expected no error in singing, got:", err) + } + hasChecksum := containsChecksum(string(armoredSignature)) + if isV6 && hasChecksum { + t.Fatalf("V6 messages should not have a checksum") + } + }) + } +} + func testSignVerify( t *testing.T, signer PGPSign,