Page MenuHome GnuPG

AEAD mode does not properly handle modified cipher text
Closed, ResolvedPublic

Description

Hi,

in the past I found out that the AEAD EAX releases unauthenticated plaintext [1]. Since then, GnuPG was modified to use OCB. I have done a basic bit-flipping test on OCB, and while gcry_cipher_checktag reports a checksum error, it does release unauthenticated plaintext, and also reports a DECRYPTION_OK/GOODMDC status line.

[1] https://mailarchive.ietf.org/arch/msg/openpgp/tN2jWx4hUtiMGSo8-ILRXYNumVY/

I am including a docker file to build GnuPG and generate a test key, as well as a test script to illustrate the problem. When I run this, I get:

# python3 test-aead.py
gpg: NOTE: THIS IS A DEVELOPMENT VERSION!
gpg: It is only intended for test purposes and should NOT be
gpg: used in a production environment or with production keys!
Bit flipped at byte 109863 bit 4
gpg: NOTE: THIS IS A DEVELOPMENT VERSION!
gpg: It is only intended for test purposes and should NOT be
gpg: used in a production environment or with production keys!
[GNUPG:] ENC_TO C449D89F69F97CF9 18 0
[GNUPG:] KEY_CONSIDERED F837EEC1E3043DEA0C6BDA1B5F6F2B595BFD6E2B 0
gpg: encrypted with cv25519 key, ID C449D89F69F97CF9, created 2024-03-13
      "aeadtest"
[GNUPG:] KEY_CONSIDERED F837EEC1E3043DEA0C6BDA1B5F6F2B595BFD6E2B 0
[GNUPG:] KEY_CONSIDERED F837EEC1E3043DEA0C6BDA1B5F6F2B595BFD6E2B 0
[GNUPG:] DECRYPTION_KEY A1E7C406ED9548E816174D68C449D89F69F97CF9 F837EEC1E3043DEA0C6BDA1B5F6F2B595BFD6E2B u
[GNUPG:] BEGIN_DECRYPTION
[GNUPG:] DECRYPTION_INFO 0 9 2
[GNUPG:] PLAINTEXT 62 1710341062 declaration.txt
[GNUPG:] PLAINTEXT_LENGTH 273600
gpg: gcry_cipher_checktag failed: Checksum error
[GNUPG:] DECRYPTION_OKAY
[GNUPG:] GOODMDC
[GNUPG:] END_DECRYPTION
File differs from byte 109702 to byte 109717
File 1: n and conscience
File 2: \xb7\xdc\xd6\xd1\xd5\xc9\xf2\xfa_\x8b!\xc4T\x94
File differs from byte 272950 to byte 273599
File 1:  equal in dignity and rights. They are endowed with reason and conscience and should act towards one another in a spirit of brotherhood.
All human beings are born free and equal in dignity and rights. They are endowed with reason and conscience and should act towards one another in a spirit of brotherhood.
All human beings are born free and equal in dignity and rights. They are endowed with reason and conscience and should act towards one another in a spirit of brotherhood.
All human beings are born free and equal in dignity and rights. They are endowed with reason and conscience and should act towards one another in a spirit of brotherhood.

File 2: \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00

The source code for the test program:

import random
import sys
import os

# Write the first line of the declaration of human rights a lot of times to a file.
def write_file(filename):
    with open(filename, 'w') as f:
        for i in range(1600):
            f.write('All human beings are born free and equal in dignity and rights. They are endowed with reason and conscience and should act towards one another in a spirit of brotherhood.\n')

def encrypt_file(filename, recipient):
    os.system('gpg --compress-algo=0 --encrypt --recipient ' + recipient + ' ' + filename)
    return filename + '.gpg'


# Read a file, flip a random bit in it, and write the result back to a file.
def flip_bit(filename):
    with open(filename, 'rb') as f:
        data = bytearray(f.read())
        bit = random.randint(0, 8 * len(data) - 1)
        byte, bit = divmod(bit, 8)
        data[byte] ^= 1 << bit
        with open(filename, 'wb') as f:
            f.write(data)
        print("Bit flipped at byte", byte, "bit", bit)

def decrypt_file(filename):
    os.system('gpg --status-fd=2 --output ' + filename[:-4] + ".out" + ' --decrypt ' + filename)
    return filename[:-4] + ".out"

def printable_bytes(data):
    return data.replace(b'\x00', b'\\x00').decode('utf-8', 'backslashreplace')

def compare_files(file1, file2):
    with open(file1, 'rb') as f1, open(file2, 'rb') as f2:
        data1 = f1.read()
        data2 = f2.read()
        differing_bytes = []
        for i in range(len(data1)):
            if data1[i] != data2[i]:
                differing_bytes.append(i)
        # Summarise differing ranges.
        i = 0
        while i < len(differing_bytes):
            start = differing_bytes[i]
            while i < len(differing_bytes) - 1 and differing_bytes[i] + 1 == differing_bytes[i + 1]:
                i += 1
            end = differing_bytes[i]
            if start == end:
                print("File differs at byte", start)
                # Print the differing bytes in a human-readable format, using \xXX if necessary. Special care needs to be taken to represent zero bytes
            else:
                print("File differs from byte", start, "to byte", end)
            print("File 1:", printable_bytes(data1[start:end + 1]))
            print("File 2:", printable_bytes(data2[start:end + 1]))
            i += 1
        if not differing_bytes:
            print("Files are the same")

def main():
    filename = 'declaration.txt'
    recipient = 'aeadtest'
    write_file(filename)
    encrypted_file = encrypt_file(filename, recipient)
    flip_bit(encrypted_file)
    decrypted_file = decrypt_file(encrypted_file)
    compare_files(filename, decrypted_file)

main()

The docker file:

FROM ubuntu
RUN apt-get update && apt-get install -y build-essential git wget \
                       libldap2-dev zlib1g-dev libbz2-dev libsqlite3-dev libreadline-dev \
                       pcscd file ca-certificates bzip2 texinfo bison xfig imagemagick \
                       autogen autoconf gettext pinentry-tty && apt-get clean
RUN git clone git://git.gnupg.org/gnupg.git
RUN git clone git://git.gnupg.org/libgpg-error.git
RUN git clone git://git.gnupg.org/libgcrypt.git
RUN git clone git://git.gnupg.org/libksba.git
RUN git clone git://git.gnupg.org/npth.git
RUN git clone git://git.gnupg.org/gpgme.git
RUN git clone git://git.gnupg.org/libassuan.git
RUN git clone git://git.gnupg.org/ntbtls.git
# https://dev.gnupg.org/T4280
RUN rm /etc/ImageMagick-6/policy.xml
# Optionally pass some flags to gnupg configure by adding: speedo_pkg_gnupg_configure=
RUN (cd gnupg; ./autogen.sh && make -f build-aux/speedo.mk git-native gitrep=/)
# This would be for release build.
#RUN wget -c https://gnupg.org/ftp/gcrypt/gnupg/gnupg-2.2.11.tar.bz2 && tar xjf gnupg-2.2.11.tar.bz2
#RUN (cd gnupg-2.2.11; ./autogen.sh && make -f build-aux/speedo.mk native gitrep=git://git.gnupg.org)
ENV PATH="/gnupg/PLAY/inst/bin:${PATH}"
ENV LD_LIBRARY_PATH="/gnupg/PLAY/inst/lib"

# Example for AEAD
RUN gpg --batch --passphrase "" --quick-generate-key aeadtest
RUN echo hallo | gpg -r aeadtest -eav | gpg -d

Event Timeline

handle_plaintext gets data returned by iobuf_read, and does not check the error status of the iobuf object.

werner added a subscriber: werner.

Thanks for reporting this. Returning error codes to upper layers is not always easy because the original logic is that we have a global error counter to decide whether an operation succeeded. My fix to check the error code before emitting the DECRYPTION_OKAY status,

werner changed the task status from Open to Testing.Mar 14 2024, 9:55 PM
werner claimed this task.

Note that this has also been ported to 2.4 and 2.2 and tested by looking at the status lines.

This fix has the problem that for a signed message where the signing key is not available gpg emits the decryption_failed status line and prints "WARNING: encrypted message has been manipulated". This is because we use log_error to show that the signature could not be verified due to a missing key. The extra check we introduced with rG50e81ad38d2b lloked at the error counter and thus triggered the decryptio failed.

Of course we could use log_info here but that might not be right either and is for sure a regression in gpg's expected behaviour. Thus a better solution is to have a simple flag for the specific error.

werner moved this task from Backlog to Done on the gnupg26 board.