PEM: validate PKCS padding in constant time #919

Open
ManoharPaturi wants to merge 1 commit from ManoharPaturi/fix-pem-ct-padding into development
ManoharPaturi commented 2026-09-29 10:57:25 -05:00 (Migrated from github.com)

Problem

pem_check_pkcs_padding() (utilities/pem.c) validates RFC 1421 CBC padding with data-dependent control flow:

if (pad_len > input_len) { return ...MISMATCH; }        /* branch on secret-derived length */
for (i = *data_len; i < input_len; i++) {
    if (input[i] != pad_len) { return ...MISMATCH; }    /* early return on first mismatch */
}

Early-returning at the first differing padding byte leaks its position through timing. For Proc-Type: 4,ENCRYPTED PEM blobs — which carry no MAC — that is a Vaudenay-style CBC padding oracle in remote-oracle threat models. The cipher layer already validates PKCS padding in constant time (mbedtls_cipher_get_pkcs_padding); this path reimplements it non-constant-time, inconsistent with the library's own bar.

Fix

Accumulate a difference mask over all padding bytes and branch once on the result; keep the pad_len range check as a single aggregate comparison.

Verification

Branch builds clean; accept/reject behavior for well-formed and malformed padding is unchanged (verified against the encrypted-PEM data files), only the timing profile changes.

## Problem `pem_check_pkcs_padding()` (utilities/pem.c) validates RFC 1421 CBC padding with data-dependent control flow: ```c if (pad_len > input_len) { return ...MISMATCH; } /* branch on secret-derived length */ for (i = *data_len; i < input_len; i++) { if (input[i] != pad_len) { return ...MISMATCH; } /* early return on first mismatch */ } ``` Early-returning at the first differing padding byte leaks its position through timing. For `Proc-Type: 4,ENCRYPTED` PEM blobs — which carry no MAC — that is a Vaudenay-style CBC padding oracle in remote-oracle threat models. The cipher layer already validates PKCS padding in constant time (`mbedtls_cipher_get_pkcs_padding`); this path reimplements it non-constant-time, inconsistent with the library's own bar. ## Fix Accumulate a difference mask over all padding bytes and branch once on the result; keep the `pad_len` range check as a single aggregate comparison. ## Verification Branch builds clean; accept/reject behavior for well-formed and malformed padding is unchanged (verified against the encrypted-PEM data files), only the timing profile changes.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-09-29 10:57:33 -05:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'ManoharPaturi/fix-pem-ct-padding:ManoharPaturi/fix-pem-ct-padding'
git switch 'ManoharPaturi/fix-pem-ct-padding'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'development'
git merge --no-ff 'ManoharPaturi/fix-pem-ct-padding'
git switch 'ManoharPaturi/fix-pem-ct-padding'
git rebase 'development'
git switch 'development'
git merge --ff-only 'ManoharPaturi/fix-pem-ct-padding'
git switch 'ManoharPaturi/fix-pem-ct-padding'
git rebase 'development'
git switch 'development'
git merge --no-ff 'ManoharPaturi/fix-pem-ct-padding'
git switch 'development'
git merge --squash 'ManoharPaturi/fix-pem-ct-padding'
git switch 'development'
git merge --ff-only 'ManoharPaturi/fix-pem-ct-padding'
git switch 'development'
git merge 'ManoharPaturi/fix-pem-ct-padding'
git push origin 'development'
Sign in to join this conversation.
No reviewers
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!919
No description provided.