Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3: Rework Armor checksum handling #284

Merged
merged 4 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions armor/armor.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ 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, constants.DoChecksum)
return armor.EncodeWithChecksumOption(w, armorType, internal.ArmorHeaders, constants.ArmorChecksumSetting)
}

// ArmorWriterWithTypeChecksum returns a io.WriteCloser which, when written to, writes
Expand All @@ -40,12 +40,12 @@ func ArmorWriterWithTypeAndCustomHeaders(w io.Writer, armorType, version, commen
if comment != "" {
headers["Comment"] = comment
}
return armor.EncodeWithChecksumOption(w, armorType, headers, constants.DoChecksum)
return armor.EncodeWithChecksumOption(w, armorType, headers, constants.ArmorChecksumSetting)
}

// ArmorWithType armors input with the given armorType.
func ArmorWithType(input []byte, armorType string) (string, error) {
return ArmorWithTypeChecksum(input, armorType, constants.DoChecksum)
return ArmorWithTypeChecksum(input, armorType, constants.ArmorChecksumSetting)
}

// ArmorWithTypeChecksum armors input with the given armorType.
Expand All @@ -60,7 +60,7 @@ func ArmorWithTypeChecksum(input []byte, armorType string, checksum bool) (strin

// ArmorWithTypeBytes armors input with the given armorType.
func ArmorWithTypeBytes(input []byte, armorType string) ([]byte, error) {
return ArmorWithTypeBytesChecksum(input, armorType, constants.DoChecksum)
return ArmorWithTypeBytesChecksum(input, armorType, constants.ArmorChecksumSetting)
}

// ArmorWithTypeBytesChecksum armors input with the given armorType and checksum option.
Expand All @@ -75,7 +75,7 @@ func ArmorWithTypeBytesChecksum(input []byte, armorType string, checksum bool) (
// 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.DoChecksum)
return ArmorWithTypeAndCustomHeadersChecksum(input, armorType, version, comment, constants.ArmorChecksumSetting)
}

// ArmorWithTypeAndCustomHeadersChecksum armors input with the given armorType and
Expand Down Expand Up @@ -105,7 +105,7 @@ func ArmorWithTypeAndCustomHeadersBytes(input []byte, armorType, version, commen
if comment != "" {
headers["Comment"] = comment
}
buffer, err := armorWithTypeAndHeaders(input, armorType, headers, constants.DoChecksum)
buffer, err := armorWithTypeAndHeaders(input, armorType, headers, constants.ArmorChecksumSetting)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -182,10 +182,10 @@ func IsPGPArmored(in io.Reader) (io.Reader, bool) {
return outReader, false
}

func armorWithTypeAndHeaders(input []byte, armorType string, headers map[string]string, doChecksum bool) (*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, doChecksum)
w, err := armor.EncodeWithChecksumOption(&b, armorType, headers, writeChecksum)

if err != nil {
return nil, errors.Wrap(err, "armor: unable to encode armoring")
Expand Down
22 changes: 14 additions & 8 deletions constants/armor.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,18 @@ package constants

// Constants for armored data.
const (
DoChecksum = 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"
// ArmorChecksumSetting 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.
ArmorChecksumSetting = true
lubux marked this conversation as resolved.
Show resolved Hide resolved
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"
)
2 changes: 1 addition & 1 deletion crypto/encrypt_decrypt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ 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].isVersionSix()
isV6 := material.keyRingTestPublic.GetKeys()[0].isV6()
encHandle, _ := material.pgp.Encryption().
Recipients(material.keyRingTestPublic).
SigningKeys(material.keyRingTestPrivate).
Expand Down
29 changes: 17 additions & 12 deletions crypto/encryption_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (eh *encryptionHandle) Encrypt(message []byte) (*PGPMessage, error) {
if err != nil {
return nil, err
}
checksum := eh.doArmorChecksum()
checksum := eh.armorChecksumRequired()
return pgpMessageBuffer.PGPMessageWithOptions(eh.PlainDetachedSignature, !checksum), nil
}

Expand Down Expand Up @@ -151,33 +151,38 @@ func (eh *encryptionHandle) validate() error {
return nil
}

// doArmorChecksum determines if an armor checksum should be appended or not.
// 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) doArmorChecksum() bool {
func (eh *encryptionHandle) armorChecksumRequired() bool {
if !constants.ArmorChecksumSetting {
// 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 constants.DoChecksum
return true
}
checkTime := eh.clock()
if eh.Recipients != nil {
for _, recipient := range eh.Recipients.entities {
primarySelfSignature, err := recipient.PrimarySelfSignature(checkTime)
if err != nil {
return constants.DoChecksum
return true
}
if !primarySelfSignature.SEIPDv2 {
return constants.DoChecksum
return true
}
}
}
if eh.HiddenRecipients != nil {
for _, recipient := range eh.HiddenRecipients.entities {
primarySelfSignature, err := recipient.PrimarySelfSignature(checkTime)
if err != nil {
return constants.DoChecksum
return true
}
if !primarySelfSignature.SEIPDv2 {
return constants.DoChecksum
return true
}
}
}
Expand Down Expand Up @@ -226,25 +231,25 @@ func (eh *encryptionHandle) handleArmor(keys, data, detachedSignature Writer) (
armorSigWriter WriteCloser,
err error,
) {
doChecksum := eh.doArmorChecksum()
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, doChecksum)
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, doChecksum)
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, doChecksum)
armorSigWriter, err = armor.EncodeWithChecksumOption(detachedSignature, constants.PGPSignatureHeader, eh.ArmorHeaders, writeChecksum)
detachedSignatureOut = armorSigWriter
if err != nil {
return nil, nil, nil, nil, err
Expand Down
12 changes: 6 additions & 6 deletions crypto/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,10 @@ func (key *Key) Armor() (string, error) {
}

if key.IsPrivate() {
return armor.ArmorWithTypeChecksum(serialized, constants.PrivateKeyHeader, !key.isVersionSix())
return armor.ArmorWithTypeChecksum(serialized, constants.PrivateKeyHeader, !key.isV6())
}

return armor.ArmorWithTypeChecksum(serialized, constants.PublicKeyHeader, !key.isVersionSix())
return armor.ArmorWithTypeChecksum(serialized, constants.PublicKeyHeader, !key.isV6())
}

// ArmorWithCustomHeaders returns the armored key as a string, with
Expand All @@ -238,7 +238,7 @@ func (key *Key) ArmorWithCustomHeaders(comment, version string) (string, error)
return "", err
}

return armor.ArmorWithTypeAndCustomHeadersChecksum(serialized, constants.PrivateKeyHeader, version, comment, !key.isVersionSix())
return armor.ArmorWithTypeAndCustomHeadersChecksum(serialized, constants.PrivateKeyHeader, version, comment, !key.isV6())
}

// GetArmoredPublicKey returns the armored public keys from this keyring.
Expand All @@ -248,7 +248,7 @@ func (key *Key) GetArmoredPublicKey() (s string, err error) {
return "", err
}

return armor.ArmorWithTypeChecksum(serialized, constants.PublicKeyHeader, !key.isVersionSix())
return armor.ArmorWithTypeChecksum(serialized, constants.PublicKeyHeader, !key.isV6())
}

// GetArmoredPublicKeyWithCustomHeaders returns the armored public key as a string, with
Expand All @@ -259,7 +259,7 @@ func (key *Key) GetArmoredPublicKeyWithCustomHeaders(comment, version string) (s
return "", err
}

return armor.ArmorWithTypeAndCustomHeadersChecksum(serialized, constants.PublicKeyHeader, version, comment, !key.isVersionSix())
return armor.ArmorWithTypeAndCustomHeadersChecksum(serialized, constants.PublicKeyHeader, version, comment, !key.isV6())
}

// GetPublicKey returns the unarmored public keys from this keyring.
Expand Down Expand Up @@ -433,7 +433,7 @@ func (key *Key) ToPublic() (publicKey *Key, err error) {
return
}

func (key *Key) isVersionSix() bool {
func (key *Key) isV6() bool {
if key == nil || key.entity == nil {
return false
}
Expand Down
12 changes: 6 additions & 6 deletions crypto/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type PGPMessage struct {
// detachedSignatureIsPlain indicates if the detached signature is not encrypted.
detachedSignatureIsPlain bool
// Signals that no armor checksum must be appended when armoring
forceNoArmorChecksum bool
omitArmorChecksum bool
}

type PGPMessageBuffer struct {
Expand Down Expand Up @@ -141,7 +141,7 @@ func (msg *PGPMessage) Armor() (string, error) {
if msg.KeyPacket == nil {
return "", errors.New("gopenpgp: missing key packets in pgp message")
}
if msg.forceNoArmorChecksum {
if msg.omitArmorChecksum {
return armor.ArmorPGPMessageChecksum(msg.Bytes(), false)
}
return armor.ArmorPGPMessage(msg.Bytes())
Expand All @@ -152,7 +152,7 @@ func (msg *PGPMessage) ArmorBytes() ([]byte, error) {
if msg.KeyPacket == nil {
return nil, errors.New("gopenpgp: missing key packets in pgp message")
}
if msg.forceNoArmorChecksum {
if msg.omitArmorChecksum {
return armor.ArmorPGPMessageBytesChecksum(msg.Bytes(), false)
}
return armor.ArmorPGPMessageBytes(msg.Bytes())
Expand All @@ -161,7 +161,7 @@ func (msg *PGPMessage) ArmorBytes() ([]byte, error) {
// 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.forceNoArmorChecksum {
if msg.omitArmorChecksum {
return armor.ArmorWithTypeAndCustomHeadersChecksum(msg.Bytes(), constants.PGPMessageHeader, version, comment, false)
}
return armor.ArmorWithTypeAndCustomHeaders(msg.Bytes(), constants.PGPMessageHeader, version, comment)
Expand Down Expand Up @@ -371,7 +371,7 @@ func (mb *PGPMessageBuffer) PGPMessage() *PGPMessage {

// 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) PGPMessageWithOptions(isPlain, forceNoChecksum bool) *PGPMessage {
func (mb *PGPMessageBuffer) PGPMessageWithOptions(isPlain, omitArmorChecksum bool) *PGPMessage {
var detachedSignature []byte
if mb.signature.Len() > 0 {
detachedSignature = mb.signature.Bytes()
Expand All @@ -386,7 +386,7 @@ func (mb *PGPMessageBuffer) PGPMessageWithOptions(isPlain, forceNoChecksum bool)
DataPacket: mb.data.Bytes(),
DetachedSignature: detachedSignature,
detachedSignatureIsPlain: isPlain,
forceNoArmorChecksum: forceNoChecksum,
omitArmorChecksum: omitArmorChecksum,
}
}

Expand Down
15 changes: 10 additions & 5 deletions crypto/sign_handle.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ func (sh *signatureHandle) SigningWriter(outputWriter Writer, encoding int8) (me
var armorWriter WriteCloser
armorOutput := armorOutput(encoding)
if armorOutput {
doChecksum := sh.doArmorChecksum()
writeChecksum := sh.armorChecksumRequired()
var err error
header := constants.PGPMessageHeader
if sh.Detached {
header = constants.PGPSignatureHeader
}
armorWriter, err = armor.EncodeWithChecksumOption(outputWriter, header, sh.ArmorHeaders, doChecksum)
armorWriter, err = armor.EncodeWithChecksumOption(outputWriter, header, sh.ArmorHeaders, writeChecksum)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -131,13 +131,18 @@ func (sh *signatureHandle) validate() error {
return nil
}

func (sh *signatureHandle) doArmorChecksum() bool {
func (sh *signatureHandle) armorChecksumRequired() bool {
if !constants.ArmorChecksumSetting {
// If the default behavior is no checksum, we can ignore
// the logic for the crypto refresh check.
return false
}
if sh.SignKeyRing == nil {
return constants.DoChecksum
return true
}
for _, signer := range sh.SignKeyRing.entities {
if signer.PrimaryKey.Version != 6 {
return constants.DoChecksum
return true
}
}
return false
Expand Down
2 changes: 1 addition & 1 deletion crypto/sign_verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ 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].isVersionSix()
isV6 := material.keyRingTestPrivate.GetKeys()[0].isV6()
signer, _ := material.pgp.Sign().
SigningKeys(material.keyRingTestPrivate).
New()
Expand Down