gh-Mbed-TLS/TF-PSA-Crypto

Watch

Fork 0

psa_cipher_update() with ECB_NO_PADDING accepts input == output when a partial block is pending #891

New issue

Open

opened 2026-09-01 07:54:49 -05:00 by Pranav1904 · 1 comment

Pranav1904 commented

2026-09-01 07:54:49 -05:00

(Migrated from github.com)

Summary

psa_cipher_update() with PSA_ALG_ECB_NO_PADDING accepts input == output while a partial block from an earlier call is pending. mbedtls_cipher_update() rejects that exact combination with MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA (drivers/builtin/src/cipher.c:422), but the check never fires, because psa_cipher_update_ecb() moves the two pointers one byte apart before calling into it. Every call returns PSA_SUCCESS and the resulting ciphertext is wrong.

The call is accepted in both configurations, and I have included a step below that shows it in a default build as well, so this is not specific to one build option:

With MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS, the write lands on the caller's
own buffer and destroys input that has not been read yet, so the blocks that follow encrypt data that was never supplied.
In a default build, the input copy taken by the dispatch layer keeps the
plaintext intact, so the ciphertext blocks are correct and only their position is wrong. The call is still accepted and the result is still not the expected ciphertext.

I am aware that the documentation of MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS states that overlap between input and output buffers is not supported when it is enabled, and that a caller who reuses the input offset as the output pointer has made a mistake. My request is not that the operation be made to work. It is that it be rejected rather than silently corrupted, which is what the legacy API already does for the same condition, and what some PSA drivers already do.

The practical problem this causes is that the outcome depends on the driver. I work on the Mbed TLS integration in ESP-IDF, where our hardware AES driver rejects this buffer layout and the built-in software driver silently accepts it. The same PSA call, with the same key and the same data, therefore either returns a clean error or produces wrong ciphertext depending on which driver is compiled in. That is not something an application can be written against, and it is what led me to this report.

System information

TF-PSA-Crypto version (number or commit id): ec56e6a6ffccf6f8982d271d2d3ddac2de84906f (development, 2026-08-27), as the submodule of Mbed TLS 522e2e4651db0758dcc7f5da6737239ce45cab22 Operating system and version: macOS 26.4 (Darwin 25.4.0, arm64), and Ubuntu 22.04 (x86-64); reproduces identically on both Configuration (if not default, please attach crypto_config.h): default crypto_config.h; the call is accepted both with and without MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS, and overwrites unread input when it is enabled Compiler and options (if you used a pre-built binary, please indicate how you obtained it): Apple clang 21.0.0 and gcc 11.4.0, cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_FLAGS="-DMBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS -g -O0" Additional environment information: reproduces on the plain C AES path and on the AESNI path, and on an Espressif target through the Mbed TLS port in ESP-IDF, so it is not specific to one host, compiler or AES backend.

Expected behavior

psa_cipher_update() should return an error when input == output and a partial block is pending, as mbedtls_cipher_update() does. Any error is better than the current behaviour; PSA_ERROR_INVALID_ARGUMENT seems the natural choice.

The combination cannot be executed correctly, because completing a pending block writes a full block of output while consuming fewer than a full block of input, so the write necessarily lands on input that has not been read. There is nothing to preserve here, and no caller can be relying on the current results.

Actual behavior

All four psa_cipher_update() calls return PSA_SUCCESS and report plausible output lengths, and the output does not match the ciphertext that the same key and plaintext produce. psa_cipher_finish() also succeeds. Pass 3 of the reproducer below, on the unfixed library with MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS:

=== pass 3: in place, output pointer equal to input pointer ===
  update(in=  0, len=32, out=  0) -> 0, output_length=32   [input == output]
  update(in= 32, len=33, out= 32) -> 0, output_length=32   [input == output]
  update(in= 65, len=32, out= 65) -> 0, output_length=32   [input == output]
  update(in= 97, len=15, out= 97) -> 0, output_length=16   [input == output]
  first mismatch at byte 64: got c5, expected 1c
  VERDICT: output differs from the OpenSSL reference

Comparing the two hex strings the program prints, with spaces added here for alignment:

expected  ...cecdd402cb3d980    1c63ed939f7c17f7c95964062ef7f1c1  e30b1afae8da5e79b39151a09beb53c2  03adaf98b114be1d1bb5bdaa113f822a
got       ...cecdd402cb3d980 c5 1c63ed939f7c17f7c95964062ef7f1c1  282ead2ef56812ca909877bdcd51449e  4aeb138e3e8de7b737e422f6674340

0xc5 at offset 64 is the original plaintext byte, which was never overwritten. The block 1c63ed...f1c1 that belongs there follows one byte late. From the next block onwards the output is unrelated to the plaintext: 282ead2e... is not the encryption of anything the caller supplied.

What happens inside the call

The second call supplies 33 bytes, so one byte is left pending in ctx->unprocessed_data and buf[64] is left untouched in the buffer. The third call is entered with input == output == buf+65 and does two things in psa_cipher_update_ecb():

It completes the pending block, which needs 15 more bytes. It reads buf[65..79] and writes 16 bytes to buf[65..80]. buf[80] held the plaintext byte 0x36; it now holds 0xc1, the last byte of that ciphertext block. 2. It advances input by 15 to buf+80 and output by 16 to buf+81, then calls mbedtls_cipher_update(input=buf+80, ilen=16, output=buf+81) for the remaining full blocks. buf[80] was destroyed in step 1, so this block encrypts a byte the caller never supplied, and every later block inherits the damage.

At step 2 the pointers are one byte apart, so the input == output test in mbedtls_cipher_update() is false and the existing guard cannot fire. The check is correct; it is simply no longer shown the aliasing by the time it runs.

In-place operation itself is not affected

This matters for the shape of the fix, so I checked it rather than assuming it. A caller that advances its output offset by the returned output_length, instead of reusing the input offset, gets the correct ciphertext on the unfixed library. Pass 2 of the reproducer does this.

It also holds in general, not just for one chunking. Under that idiom the invariant in_off - out_off == unprocessed_len holds, so when a pending block is completed the write ends at out_off + block_size and the read ends at in_off + (block_size - unprocessed_len), which is the same address. The write never overtakes the read. I confirmed this over 3000 random chunkings on the unfixed library, in both configurations, with no failures.

So the check I am proposing rejects only the combination that is already broken, and no working caller is affected by it.

Behaviour by configuration

Build	Pass 1, separate buffers	Pass 2, in place, offsets tracked	Pass 3, in place, `output == input`
`MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS`	correct	correct	`PSA_SUCCESS`, unread input destroyed, later blocks encrypt data never supplied
default	correct	correct	`PSA_SUCCESS`, blocks correct but displaced by one byte

To be precise about the second row: in a default build the library writes the correct ciphertext at the pointer it was given, so the displaced result there follows from the caller's own offset choice rather than from memory being overwritten, and I am not reporting it as memory corruption. I include it because it shows the call is accepted in every configuration, which is the part I would like to see changed.

Steps to reproduce

Everything below is against ec56e6a6ffccf6f8982d271d2d3ddac2de84906f on development.

1. Get the sources.

git clone https://github.com/Mbed-TLS/TF-PSA-Crypto.git
cd TF-PSA-Crypto
git checkout ec56e6a6ffccf6f8982d271d2d3ddac2de84906f
git submodule update --init --recursive
python3 -m pip install -r scripts/basic.requirements.txt

The last line is needed; without Jinja2 the build fails while generating core/psa_crypto_driver_wrappers.h.

2. Build the library with MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS.

cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug \
      -DENABLE_TESTING=Off -DENABLE_PROGRAMS=Off \
      -DCMAKE_C_FLAGS="-DMBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS -g -O0"
cmake --build build -j --target tfpsacrypto

3. Derive the reference ciphertext independently, with OpenSSL.

The reproducer compares against a vector produced outside this library, so that a failure cannot be attributed to the reference. The plaintext is plain[i] = i * 7 + 1 + i / 16 for 112 bytes; the key is the FIPS-197 AES-128 test key.

python3 -c "import sys; sys.stdout.buffer.write(bytes(((i*7+1+i//16)&0xff) for i in range(112)))" > plain.bin
openssl enc -aes-128-ecb -nopad -K 2b7e151628aed2a6abf7158809cf4f3c -in plain.bin | xxd -p -c 16

This prints the following, which is what the openssl_ct array in the program below contains:

d1c7fde03d0710e6310b47ff8c16ceaf
00a21a01eb4916008b2c1168e3336d09
f75445fea33cfa397d38a4bfb1294f9c
196c919340774e625cecdd402cb3d980
1c63ed939f7c17f7c95964062ef7f1c1
e30b1afae8da5e79b39151a09beb53c2
03adaf98b114be1d1bb5bdaa113f822a

4. Build and run the reproducer.

cc -DMBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS -g -O0 \
   -Iinclude -Idrivers/builtin/include \
   ecb_inplace.c -o ecb_inplace build/core/libtfpsacrypto.a
./ecb_inplace; echo "exit=$?"

It exits 0 if the library behaves and 1 if the defect is present.

ecb_inplace.c

/* Reproducer for: psa_cipher_update() with PSA_ALG_ECB_NO_PADDING accepts
 * input == output while a partial block from an earlier call is pending, and
 * silently produces wrong output. mbedtls_cipher_update() rejects the same
 * combination with MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA.
 *
 * Three passes over the same 112-byte plaintext, split 32 / 33 / 32 / 15:
 *
 *   pass 1  separate input and output buffers          expected: correct
 *   pass 2  in place, output offset tracked separately expected: correct
 *   pass 3  in place, output == input on every call    expected: rejected
 *
 * Pass 2 is the supported in-place idiom and is here to show that it keeps
 * working; only pass 3 is the defect.
 */
#include <psa/crypto.h>
#include <stdio.h>
#include <string.h>

#define LEN 112

/* AES-128-ECB of the plaintext built below, under the key below, produced by
 * OpenSSL rather than by this library so that the comparison does not depend
 * on the code under test. The exact command is in the issue text. */
static const uint8_t openssl_ct[LEN] = {
    0xd1, 0xc7, 0xfd, 0xe0, 0x3d, 0x07, 0x10, 0xe6,
    0x31, 0x0b, 0x47, 0xff, 0x8c, 0x16, 0xce, 0xaf,
    0x00, 0xa2, 0x1a, 0x01, 0xeb, 0x49, 0x16, 0x00,
    0x8b, 0x2c, 0x11, 0x68, 0xe3, 0x33, 0x6d, 0x09,
    0xf7, 0x54, 0x45, 0xfe, 0xa3, 0x3c, 0xfa, 0x39,
    0x7d, 0x38, 0xa4, 0xbf, 0xb1, 0x29, 0x4f, 0x9c,
    0x19, 0x6c, 0x91, 0x93, 0x40, 0x77, 0x4e, 0x62,
    0x5c, 0xec, 0xdd, 0x40, 0x2c, 0xb3, 0xd9, 0x80,
    0x1c, 0x63, 0xed, 0x93, 0x9f, 0x7c, 0x17, 0xf7,
    0xc9, 0x59, 0x64, 0x06, 0x2e, 0xf7, 0xf1, 0xc1,
    0xe3, 0x0b, 0x1a, 0xfa, 0xe8, 0xda, 0x5e, 0x79,
    0xb3, 0x91, 0x51, 0xa0, 0x9b, 0xeb, 0x53, 0xc2,
    0x03, 0xad, 0xaf, 0x98, 0xb1, 0x14, 0xbe, 0x1d,
    0x1b, 0xb5, 0xbd, 0xaa, 0x11, 0x3f, 0x82, 0x2a,
};

static const uint8_t key[16] = {
    0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6,
    0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c
};

/* 33 is not a multiple of the block size, so the second call leaves one byte
 * pending and the third call has to complete that block. That is the case
 * which breaks. */
static const size_t parts[4] = { 32, 33, 32, 15 };

/* The exact number of bytes each call produces. Passing a larger output_size
 * would be legal, but in a default build the dispatch layer copies output_size
 * bytes back over the caller's buffer, which destroys input that has not been
 * consumed yet and would confuse this reproducer. Using exact sizes keeps all
 * three passes comparable between a default build and one with
 * MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS. */
static const size_t outsz[4] = { 32, 32, 32, 16 };

static uint8_t plain[LEN];
static mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT;

static void hexdump(const char *label, const uint8_t *p)
{
    size_t i;
    printf("  %-9s ", label);
    for (i = 0; i < LEN; i++) {
        printf("%02x", p[i]);
    }
    printf("\n");
}

static int check(const uint8_t *got)
{
    size_t i;
    for (i = 0; i < LEN; i++) {
        if (got[i] != openssl_ct[i]) {
            printf("  first mismatch at byte %zu: got %02x, expected %02x\n",
                   i, got[i], openssl_ct[i]);
            hexdump("expected:", openssl_ct);
            hexdump("got:", got);
            printf("  VERDICT: output differs from the OpenSSL reference\n");
            return 1;
        }
    }
    printf("  VERDICT: matches the OpenSSL reference\n");
    return 0;
}

/* mode 0: separate buffers.  mode 1: in place, output offset tracked.
 * mode 2: in place, output pointer equal to the input pointer. */
static int run(int mode, const char *title, uint8_t *out)
{
    psa_cipher_operation_t op = PSA_CIPHER_OPERATION_INIT;
    size_t olen, in_off = 0, out_off = 0, i;
    psa_status_t status;
    const uint8_t *in;
    uint8_t *dst;

    printf("=== pass %d: %s ===\n", mode + 1, title);

    if (mode == 0) {
        in = plain;
    } else {
        memcpy(out, plain, LEN);
        in = out;
    }

    psa_cipher_encrypt_setup(&op, key_id, PSA_ALG_ECB_NO_PADDING);
    for (i = 0; i < 4; i++) {
        dst = (mode == 2) ? out + in_off : out + out_off;
        status = psa_cipher_update(&op, in + in_off, parts[i],
                                   dst, outsz[i], &olen);
        printf("  update(in=%3zu, len=%2zu, out=%3zu) -> %d, output_length=%zu%s\n",
               in_off, parts[i], (size_t) (dst - out), (int) status, olen,
               (in + in_off == dst) ? "   [input == output]" : "");
        if (status != PSA_SUCCESS) {
            printf("  VERDICT: rejected before producing output, "
                   "no silent corruption\n");
            psa_cipher_abort(&op);
            return 0;
        }
        in_off += parts[i];
        out_off += olen;
    }
    psa_cipher_finish(&op, out + out_off, LEN - out_off, &olen);
    psa_cipher_abort(&op);

    return check(out);
}

int main(void)
{
    psa_key_attributes_t attr = PSA_KEY_ATTRIBUTES_INIT;
    /* Slack, so that a short output buffer can never mask the defect. */
    uint8_t out[LEN + 16];
    size_t i;
    int rc = 0;

    for (i = 0; i < LEN; i++) {
        plain[i] = (uint8_t) (i * 7 + 1 + i / 16);
    }

    if (psa_crypto_init() != PSA_SUCCESS) {
        return 2;
    }
    psa_set_key_usage_flags(&attr, PSA_KEY_USAGE_ENCRYPT);
    psa_set_key_algorithm(&attr, PSA_ALG_ECB_NO_PADDING);
    psa_set_key_type(&attr, PSA_KEY_TYPE_AES);
    if (psa_import_key(&attr, key, sizeof(key), &key_id) != PSA_SUCCESS) {
        return 2;
    }

    rc |= run(0, "separate input and output buffers", out);
    printf("\n");
    rc |= run(1, "in place, output offset tracked separately", out);
    printf("\n");
    rc |= run(2, "in place, output pointer equal to input pointer", out);

    psa_destroy_key(key_id);
    mbedtls_psa_crypto_free();
    printf("\n%s\n", rc == 0 ? "ALL PASSES OK" : "DEFECT REPRODUCED");
    return rc;
}

5. Result with MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS.

Passes 1 and 2 match the OpenSSL vector, pass 3 does not, and the program exits 1 with DEFECT REPRODUCED:

=== pass 1: separate input and output buffers ===
  update(in=  0, len=32, out=  0) -> 0, output_length=32
  update(in= 32, len=33, out= 32) -> 0, output_length=32
  update(in= 65, len=32, out= 64) -> 0, output_length=32
  update(in= 97, len=15, out= 96) -> 0, output_length=16
  VERDICT: matches the OpenSSL reference

=== pass 2: in place, output offset tracked separately ===
  update(in=  0, len=32, out=  0) -> 0, output_length=32   [input == output]
  update(in= 32, len=33, out= 32) -> 0, output_length=32   [input == output]
  update(in= 65, len=32, out= 64) -> 0, output_length=32
  update(in= 97, len=15, out= 96) -> 0, output_length=16
  VERDICT: matches the OpenSSL reference

=== pass 3: in place, output pointer equal to input pointer ===
  update(in=  0, len=32, out=  0) -> 0, output_length=32   [input == output]
  update(in= 32, len=33, out= 32) -> 0, output_length=32   [input == output]
  update(in= 65, len=32, out= 65) -> 0, output_length=32   [input == output]
  update(in= 97, len=15, out= 97) -> 0, output_length=16   [input == output]
  first mismatch at byte 64: got c5, expected 1c
  VERDICT: output differs from the OpenSSL reference

DEFECT REPRODUCED

6. The same in a default build, to show this is not specific to one configuration.

Rebuild both the library and the reproducer without the macro:

cmake -S . -B build-default -DCMAKE_BUILD_TYPE=Debug \
      -DENABLE_TESTING=Off -DENABLE_PROGRAMS=Off \
      -DCMAKE_C_FLAGS="-g -O0"
cmake --build build-default -j --target tfpsacrypto
cc -g -O0 -Iinclude -Idrivers/builtin/include \
   ecb_inplace.c -o ecb_inplace_default build-default/core/libtfpsacrypto.a
./ecb_inplace_default; echo "exit=$?"

Pass 3 is still accepted, still returns PSA_SUCCESS on every call, and still fails at byte 64. The program again exits 1:

=== pass 3: in place, output pointer equal to input pointer ===
  update(in=  0, len=32, out=  0) -> 0, output_length=32   [input == output]
  update(in= 32, len=33, out= 32) -> 0, output_length=32   [input == output]
  update(in= 65, len=32, out= 65) -> 0, output_length=32   [input == output]
  update(in= 97, len=15, out= 97) -> 0, output_length=16   [input == output]
  first mismatch at byte 64: got c5, expected 1c
  VERDICT: output differs from the OpenSSL reference

DEFECT REPRODUCED

The difference from step 5 is in the tail of the buffer rather than in whether the call is accepted. Here the blocks are correct and merely displaced by one byte, because the dispatch layer's input copy protected the plaintext; with MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS the later blocks are unrelated to the plaintext. Passes 1 and 2 are correct in both builds.

The chunk sizes matter throughout. The defect needs one call that leaves a partial block pending, which is the second call because 33 is not a multiple of the block size, followed by an in-place call that completes it, which is the third.

Additional information

I have a fix and a regression test ready and will open a pull request that references this issue. The fix rejects the call with PSA_ERROR_INVALID_ARGUMENT when input == output and unprocessed_len != 0, mirroring the existing check in mbedtls_cipher_update(). With it, pass 3 is refused on the third call, passes 1 and 2 are unaffected, and the 3000-chunking check above still passes.

One question on placement, which I would like your view on before the pull request. I put the check in psa_cipher_update_ecb(), which is reached only after LOCAL_INPUT_ALLOC and LOCAL_OUTPUT_ALLOC have substituted internal copies in a default build. It therefore fires only when MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS is enabled, which is where the memory damage happens, but it leaves a default build returning PSA_SUCCESS for the same call, as step 6 shows. If you would prefer the combination to be rejected consistently in every configuration, the test needs to move up to psa_cipher_update() in core/psa_crypto.c, before the local copies are made. I am happy to write it either way.

A note on the reproducer: it passes the exact number of output bytes each call produces rather than a generous output_size. A larger output_size brings in a second, unrelated effect in a default build that would obscure this report. I will raise that separately.

Related to Mbed-TLS/mbedtls#3266, which tracks overlapping input and output buffers more generally.

### Summary `psa_cipher_update()` with `PSA_ALG_ECB_NO_PADDING` accepts `input == output` while a partial block from an earlier call is pending. `mbedtls_cipher_update()` rejects that exact combination with `MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA` (`drivers/builtin/src/cipher.c:422`), but the check never fires, because `psa_cipher_update_ecb()` moves the two pointers one byte apart before calling into it. Every call returns `PSA_SUCCESS` and the resulting ciphertext is wrong. The call is accepted in **both** configurations, and I have included a step below that shows it in a default build as well, so this is not specific to one build option: - With `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS`, the write lands on the caller's own buffer and destroys input that has not been read yet, so the blocks that follow encrypt data that was never supplied. - In a default build, the input copy taken by the dispatch layer keeps the plaintext intact, so the ciphertext blocks are correct and only their position is wrong. The call is still accepted and the result is still not the expected ciphertext. I am aware that the documentation of `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS` states that overlap between input and output buffers is not supported when it is enabled, and that a caller who reuses the input offset as the output pointer has made a mistake. My request is not that the operation be made to work. It is that it be **rejected rather than silently corrupted**, which is what the legacy API already does for the same condition, and what some PSA drivers already do. The practical problem this causes is that the outcome depends on the driver. I work on the Mbed TLS integration in ESP-IDF, where our hardware AES driver rejects this buffer layout and the built-in software driver silently accepts it. The same PSA call, with the same key and the same data, therefore either returns a clean error or produces wrong ciphertext depending on which driver is compiled in. That is not something an application can be written against, and it is what led me to this report. ### System information TF-PSA-Crypto version (number or commit id): `ec56e6a6ffccf6f8982d271d2d3ddac2de84906f` (`development`, 2026-08-27), as the submodule of Mbed TLS `522e2e4651db0758dcc7f5da6737239ce45cab22` Operating system and version: macOS 26.4 (Darwin 25.4.0, arm64), and Ubuntu 22.04 (x86-64); reproduces identically on both Configuration (if not default, please attach `crypto_config.h`): default `crypto_config.h`; the call is accepted both with and without `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS`, and overwrites unread input when it is enabled Compiler and options (if you used a pre-built binary, please indicate how you obtained it): Apple clang 21.0.0 and gcc 11.4.0, `cmake -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_FLAGS="-DMBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS -g -O0"` Additional environment information: reproduces on the plain C AES path and on the AESNI path, and on an Espressif target through the Mbed TLS port in ESP-IDF, so it is not specific to one host, compiler or AES backend. ### Expected behavior `psa_cipher_update()` should return an error when `input == output` and a partial block is pending, as `mbedtls_cipher_update()` does. Any error is better than the current behaviour; `PSA_ERROR_INVALID_ARGUMENT` seems the natural choice. The combination cannot be executed correctly, because completing a pending block writes a full block of output while consuming fewer than a full block of input, so the write necessarily lands on input that has not been read. There is nothing to preserve here, and no caller can be relying on the current results. ### Actual behavior All four `psa_cipher_update()` calls return `PSA_SUCCESS` and report plausible output lengths, and the output does not match the ciphertext that the same key and plaintext produce. `psa_cipher_finish()` also succeeds. Pass 3 of the reproducer below, on the unfixed library with `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS`: ``` === pass 3: in place, output pointer equal to input pointer === update(in= 0, len=32, out= 0) -> 0, output_length=32 [input == output] update(in= 32, len=33, out= 32) -> 0, output_length=32 [input == output] update(in= 65, len=32, out= 65) -> 0, output_length=32 [input == output] update(in= 97, len=15, out= 97) -> 0, output_length=16 [input == output] first mismatch at byte 64: got c5, expected 1c VERDICT: output differs from the OpenSSL reference ``` Comparing the two hex strings the program prints, with spaces added here for alignment: ``` expected ...cecdd402cb3d980 1c63ed939f7c17f7c95964062ef7f1c1 e30b1afae8da5e79b39151a09beb53c2 03adaf98b114be1d1bb5bdaa113f822a got ...cecdd402cb3d980 c5 1c63ed939f7c17f7c95964062ef7f1c1 282ead2ef56812ca909877bdcd51449e 4aeb138e3e8de7b737e422f6674340 ``` `0xc5` at offset 64 is the original plaintext byte, which was never overwritten. The block `1c63ed...f1c1` that belongs there follows one byte late. From the next block onwards the output is unrelated to the plaintext: `282ead2e...` is not the encryption of anything the caller supplied. #### What happens inside the call The second call supplies 33 bytes, so one byte is left pending in `ctx->unprocessed_data` and `buf[64]` is left untouched in the buffer. The third call is entered with `input == output == buf+65` and does two things in `psa_cipher_update_ecb()`: 1. It completes the pending block, which needs 15 more bytes. It **reads** `buf[65..79]` and **writes** 16 bytes to `buf[65..80]`. `buf[80]` held the plaintext byte `0x36`; it now holds `0xc1`, the last byte of that ciphertext block. 2. It advances `input` by 15 to `buf+80` and `output` by 16 to `buf+81`, then calls `mbedtls_cipher_update(input=buf+80, ilen=16, output=buf+81)` for the remaining full blocks. `buf[80]` was destroyed in step 1, so this block encrypts a byte the caller never supplied, and every later block inherits the damage. At step 2 the pointers are one byte apart, so the `input == output` test in `mbedtls_cipher_update()` is false and the existing guard cannot fire. The check is correct; it is simply no longer shown the aliasing by the time it runs. #### In-place operation itself is not affected This matters for the shape of the fix, so I checked it rather than assuming it. A caller that advances its output offset by the returned `output_length`, instead of reusing the input offset, gets the correct ciphertext on the **unfixed** library. Pass 2 of the reproducer does this. It also holds in general, not just for one chunking. Under that idiom the invariant `in_off - out_off == unprocessed_len` holds, so when a pending block is completed the write ends at `out_off + block_size` and the read ends at `in_off + (block_size - unprocessed_len)`, which is the same address. The write never overtakes the read. I confirmed this over 3000 random chunkings on the unfixed library, in both configurations, with no failures. So the check I am proposing rejects only the combination that is already broken, and no working caller is affected by it. #### Behaviour by configuration | Build | Pass 1, separate buffers | Pass 2, in place, offsets tracked | Pass 3, in place, `output == input` | | --- | --- | --- | --- | | `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS` | correct | correct | `PSA_SUCCESS`, unread input destroyed, later blocks encrypt data never supplied | | default | correct | correct | `PSA_SUCCESS`, blocks correct but displaced by one byte | To be precise about the second row: in a default build the library writes the correct ciphertext at the pointer it was given, so the displaced result there follows from the caller's own offset choice rather than from memory being overwritten, and I am not reporting it as memory corruption. I include it because it shows the call is accepted in every configuration, which is the part I would like to see changed. ### Steps to reproduce Everything below is against `ec56e6a6ffccf6f8982d271d2d3ddac2de84906f` on `development`. **1. Get the sources.** ```sh git clone https://github.com/Mbed-TLS/TF-PSA-Crypto.git cd TF-PSA-Crypto git checkout ec56e6a6ffccf6f8982d271d2d3ddac2de84906f git submodule update --init --recursive python3 -m pip install -r scripts/basic.requirements.txt ``` The last line is needed; without Jinja2 the build fails while generating `core/psa_crypto_driver_wrappers.h`. **2. Build the library with `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS`.** ```sh cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug \ -DENABLE_TESTING=Off -DENABLE_PROGRAMS=Off \ -DCMAKE_C_FLAGS="-DMBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS -g -O0" cmake --build build -j --target tfpsacrypto ``` **3. Derive the reference ciphertext independently, with OpenSSL.** The reproducer compares against a vector produced outside this library, so that a failure cannot be attributed to the reference. The plaintext is `plain[i] = i * 7 + 1 + i / 16` for 112 bytes; the key is the FIPS-197 AES-128 test key. ```sh python3 -c "import sys; sys.stdout.buffer.write(bytes(((i*7+1+i//16)&0xff) for i in range(112)))" > plain.bin openssl enc -aes-128-ecb -nopad -K 2b7e151628aed2a6abf7158809cf4f3c -in plain.bin | xxd -p -c 16 ``` This prints the following, which is what the `openssl_ct` array in the program below contains: ``` d1c7fde03d0710e6310b47ff8c16ceaf 00a21a01eb4916008b2c1168e3336d09 f75445fea33cfa397d38a4bfb1294f9c 196c919340774e625cecdd402cb3d980 1c63ed939f7c17f7c95964062ef7f1c1 e30b1afae8da5e79b39151a09beb53c2 03adaf98b114be1d1bb5bdaa113f822a ``` **4. Build and run the reproducer.** ```sh cc -DMBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS -g -O0 \ -Iinclude -Idrivers/builtin/include \ ecb_inplace.c -o ecb_inplace build/core/libtfpsacrypto.a ./ecb_inplace; echo "exit=$?" ``` It exits 0 if the library behaves and 1 if the defect is present. <details> <summary><code>ecb_inplace.c</code></summary> ```c /* Reproducer for: psa_cipher_update() with PSA_ALG_ECB_NO_PADDING accepts * input == output while a partial block from an earlier call is pending, and * silently produces wrong output. mbedtls_cipher_update() rejects the same * combination with MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA. * * Three passes over the same 112-byte plaintext, split 32 / 33 / 32 / 15: * * pass 1 separate input and output buffers expected: correct * pass 2 in place, output offset tracked separately expected: correct * pass 3 in place, output == input on every call expected: rejected * * Pass 2 is the supported in-place idiom and is here to show that it keeps * working; only pass 3 is the defect. */ #include <psa/crypto.h> #include <stdio.h> #include <string.h> #define LEN 112 /* AES-128-ECB of the plaintext built below, under the key below, produced by * OpenSSL rather than by this library so that the comparison does not depend * on the code under test. The exact command is in the issue text. */ static const uint8_t openssl_ct[LEN] = { 0xd1, 0xc7, 0xfd, 0xe0, 0x3d, 0x07, 0x10, 0xe6, 0x31, 0x0b, 0x47, 0xff, 0x8c, 0x16, 0xce, 0xaf, 0x00, 0xa2, 0x1a, 0x01, 0xeb, 0x49, 0x16, 0x00, 0x8b, 0x2c, 0x11, 0x68, 0xe3, 0x33, 0x6d, 0x09, 0xf7, 0x54, 0x45, 0xfe, 0xa3, 0x3c, 0xfa, 0x39, 0x7d, 0x38, 0xa4, 0xbf, 0xb1, 0x29, 0x4f, 0x9c, 0x19, 0x6c, 0x91, 0x93, 0x40, 0x77, 0x4e, 0x62, 0x5c, 0xec, 0xdd, 0x40, 0x2c, 0xb3, 0xd9, 0x80, 0x1c, 0x63, 0xed, 0x93, 0x9f, 0x7c, 0x17, 0xf7, 0xc9, 0x59, 0x64, 0x06, 0x2e, 0xf7, 0xf1, 0xc1, 0xe3, 0x0b, 0x1a, 0xfa, 0xe8, 0xda, 0x5e, 0x79, 0xb3, 0x91, 0x51, 0xa0, 0x9b, 0xeb, 0x53, 0xc2, 0x03, 0xad, 0xaf, 0x98, 0xb1, 0x14, 0xbe, 0x1d, 0x1b, 0xb5, 0xbd, 0xaa, 0x11, 0x3f, 0x82, 0x2a, }; static const uint8_t key[16] = { 0x2b, 0x7e, 0x15, 0x16, 0x28, 0xae, 0xd2, 0xa6, 0xab, 0xf7, 0x15, 0x88, 0x09, 0xcf, 0x4f, 0x3c }; /* 33 is not a multiple of the block size, so the second call leaves one byte * pending and the third call has to complete that block. That is the case * which breaks. */ static const size_t parts[4] = { 32, 33, 32, 15 }; /* The exact number of bytes each call produces. Passing a larger output_size * would be legal, but in a default build the dispatch layer copies output_size * bytes back over the caller's buffer, which destroys input that has not been * consumed yet and would confuse this reproducer. Using exact sizes keeps all * three passes comparable between a default build and one with * MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS. */ static const size_t outsz[4] = { 32, 32, 32, 16 }; static uint8_t plain[LEN]; static mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; static void hexdump(const char *label, const uint8_t *p) { size_t i; printf(" %-9s ", label); for (i = 0; i < LEN; i++) { printf("%02x", p[i]); } printf("\n"); } static int check(const uint8_t *got) { size_t i; for (i = 0; i < LEN; i++) { if (got[i] != openssl_ct[i]) { printf(" first mismatch at byte %zu: got %02x, expected %02x\n", i, got[i], openssl_ct[i]); hexdump("expected:", openssl_ct); hexdump("got:", got); printf(" VERDICT: output differs from the OpenSSL reference\n"); return 1; } } printf(" VERDICT: matches the OpenSSL reference\n"); return 0; } /* mode 0: separate buffers. mode 1: in place, output offset tracked. * mode 2: in place, output pointer equal to the input pointer. */ static int run(int mode, const char *title, uint8_t *out) { psa_cipher_operation_t op = PSA_CIPHER_OPERATION_INIT; size_t olen, in_off = 0, out_off = 0, i; psa_status_t status; const uint8_t *in; uint8_t *dst; printf("=== pass %d: %s ===\n", mode + 1, title); if (mode == 0) { in = plain; } else { memcpy(out, plain, LEN); in = out; } psa_cipher_encrypt_setup(&op, key_id, PSA_ALG_ECB_NO_PADDING); for (i = 0; i < 4; i++) { dst = (mode == 2) ? out + in_off : out + out_off; status = psa_cipher_update(&op, in + in_off, parts[i], dst, outsz[i], &olen); printf(" update(in=%3zu, len=%2zu, out=%3zu) -> %d, output_length=%zu%s\n", in_off, parts[i], (size_t) (dst - out), (int) status, olen, (in + in_off == dst) ? " [input == output]" : ""); if (status != PSA_SUCCESS) { printf(" VERDICT: rejected before producing output, " "no silent corruption\n"); psa_cipher_abort(&op); return 0; } in_off += parts[i]; out_off += olen; } psa_cipher_finish(&op, out + out_off, LEN - out_off, &olen); psa_cipher_abort(&op); return check(out); } int main(void) { psa_key_attributes_t attr = PSA_KEY_ATTRIBUTES_INIT; /* Slack, so that a short output buffer can never mask the defect. */ uint8_t out[LEN + 16]; size_t i; int rc = 0; for (i = 0; i < LEN; i++) { plain[i] = (uint8_t) (i * 7 + 1 + i / 16); } if (psa_crypto_init() != PSA_SUCCESS) { return 2; } psa_set_key_usage_flags(&attr, PSA_KEY_USAGE_ENCRYPT); psa_set_key_algorithm(&attr, PSA_ALG_ECB_NO_PADDING); psa_set_key_type(&attr, PSA_KEY_TYPE_AES); if (psa_import_key(&attr, key, sizeof(key), &key_id) != PSA_SUCCESS) { return 2; } rc |= run(0, "separate input and output buffers", out); printf("\n"); rc |= run(1, "in place, output offset tracked separately", out); printf("\n"); rc |= run(2, "in place, output pointer equal to input pointer", out); psa_destroy_key(key_id); mbedtls_psa_crypto_free(); printf("\n%s\n", rc == 0 ? "ALL PASSES OK" : "DEFECT REPRODUCED"); return rc; } ``` </details> **5. Result with `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS`.** Passes 1 and 2 match the OpenSSL vector, pass 3 does not, and the program exits 1 with `DEFECT REPRODUCED`: ``` === pass 1: separate input and output buffers === update(in= 0, len=32, out= 0) -> 0, output_length=32 update(in= 32, len=33, out= 32) -> 0, output_length=32 update(in= 65, len=32, out= 64) -> 0, output_length=32 update(in= 97, len=15, out= 96) -> 0, output_length=16 VERDICT: matches the OpenSSL reference === pass 2: in place, output offset tracked separately === update(in= 0, len=32, out= 0) -> 0, output_length=32 [input == output] update(in= 32, len=33, out= 32) -> 0, output_length=32 [input == output] update(in= 65, len=32, out= 64) -> 0, output_length=32 update(in= 97, len=15, out= 96) -> 0, output_length=16 VERDICT: matches the OpenSSL reference === pass 3: in place, output pointer equal to input pointer === update(in= 0, len=32, out= 0) -> 0, output_length=32 [input == output] update(in= 32, len=33, out= 32) -> 0, output_length=32 [input == output] update(in= 65, len=32, out= 65) -> 0, output_length=32 [input == output] update(in= 97, len=15, out= 97) -> 0, output_length=16 [input == output] first mismatch at byte 64: got c5, expected 1c VERDICT: output differs from the OpenSSL reference DEFECT REPRODUCED ``` **6. The same in a default build, to show this is not specific to one configuration.** Rebuild both the library and the reproducer without the macro: ```sh cmake -S . -B build-default -DCMAKE_BUILD_TYPE=Debug \ -DENABLE_TESTING=Off -DENABLE_PROGRAMS=Off \ -DCMAKE_C_FLAGS="-g -O0" cmake --build build-default -j --target tfpsacrypto cc -g -O0 -Iinclude -Idrivers/builtin/include \ ecb_inplace.c -o ecb_inplace_default build-default/core/libtfpsacrypto.a ./ecb_inplace_default; echo "exit=$?" ``` Pass 3 is still accepted, still returns `PSA_SUCCESS` on every call, and still fails at byte 64. The program again exits 1: ``` === pass 3: in place, output pointer equal to input pointer === update(in= 0, len=32, out= 0) -> 0, output_length=32 [input == output] update(in= 32, len=33, out= 32) -> 0, output_length=32 [input == output] update(in= 65, len=32, out= 65) -> 0, output_length=32 [input == output] update(in= 97, len=15, out= 97) -> 0, output_length=16 [input == output] first mismatch at byte 64: got c5, expected 1c VERDICT: output differs from the OpenSSL reference DEFECT REPRODUCED ``` The difference from step 5 is in the tail of the buffer rather than in whether the call is accepted. Here the blocks are correct and merely displaced by one byte, because the dispatch layer's input copy protected the plaintext; with `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS` the later blocks are unrelated to the plaintext. Passes 1 and 2 are correct in both builds. The chunk sizes matter throughout. The defect needs one call that leaves a partial block pending, which is the second call because 33 is not a multiple of the block size, followed by an in-place call that completes it, which is the third. ### Additional information I have a fix and a regression test ready and will open a pull request that references this issue. The fix rejects the call with `PSA_ERROR_INVALID_ARGUMENT` when `input == output` and `unprocessed_len != 0`, mirroring the existing check in `mbedtls_cipher_update()`. With it, pass 3 is refused on the third call, passes 1 and 2 are unaffected, and the 3000-chunking check above still passes. One question on placement, which I would like your view on before the pull request. I put the check in `psa_cipher_update_ecb()`, which is reached only after `LOCAL_INPUT_ALLOC` and `LOCAL_OUTPUT_ALLOC` have substituted internal copies in a default build. It therefore fires only when `MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS` is enabled, which is where the memory damage happens, but it leaves a default build returning `PSA_SUCCESS` for the same call, as step 6 shows. If you would prefer the combination to be rejected consistently in every configuration, the test needs to move up to `psa_cipher_update()` in `core/psa_crypto.c`, before the local copies are made. I am happy to write it either way. A note on the reproducer: it passes the exact number of output bytes each call produces rather than a generous `output_size`. A larger `output_size` brings in a second, unrelated effect in a default build that would obscure this report. I will raise that separately. Related to Mbed-TLS/mbedtls#3266, which tracks overlapping input and output buffers more generally.

gilles-peskine-arm commented

2026-09-01 09:36:16 -05:00

(Migrated from github.com)

I need to think about this. The problem with rejecting overlap is that it's practically impossible to do it in standard C. It's easy to check if input == output, but what about input == output + 1, input == output - 3, etc.? Partial overlap is common when the ciphertext starts with an IV or with some protocol header. For the specific case of ECB, partial overlap is likely not very common, but ECB itself is not very common either (the only reason we have it in the PSA API is to allow callers to make a custom mode that the library doesn't support). Some platforms allow comparisons like input < output + length, but we can't have that in our code, because there are platforms where this faults or can have false positives if input and output are in different regions of memory.

I need to think about this. The problem with rejecting overlap is that it's practically impossible to do it in standard C. It's easy to check if `input == output`, but what about `input == output + 1`, `input == output - 3`, etc.? Partial overlap is common when the ciphertext starts with an IV or with some protocol header. For the specific case of ECB, partial overlap is likely not very common, but ECB itself is not very common either (the only reason we have it in the PSA API is to allow callers to make a custom mode that the library doesn't support). Some platforms allow comparisons like `input < output + length`, but we can't have that in our code, because there are platforms where this faults or can have false positives if `input` and `output` are in different regions of memory.

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Mbed-TLS/TF-PSA-Crypto#891

No description provided.

Rows
Columns