Skip to content

fix(auth): accept PEM trust-chain preambles - #18555

Open
dhairyajangir wants to merge 2 commits into
googleapis:mainfrom
dhairyajangir:fix/auth-trust-chain-preamble
Open

dhairyajangir wants to merge 2 commits into
googleapis:mainfrom
dhairyajangir:fix/auth-trust-chain-preamble

Conversation

@dhairyajangir

Copy link
Copy Markdown

An X.509 workload trust-chain file with comments or OpenSSL metadata before its first certificate currently raises RefreshError: the loader adds a certificate header to that preamble and tries to parse it as a certificate. Skip the preamble when a certificate header is present, preserving certificate order and the existing handling of malformed, missing and empty certificate data.

The parser continues to work with the declared cryptography 38.0.3 minimum. Tests cover comment and OpenSSL preambles, whitespace, chains with and without the leaf certificate, malformed PEM, files containing no certificate and empty files.

Validation on Python 3.12:

  • Before the fix: 4 comment/preamble regression failures; 4 other selected cases passed.
  • All 92 identity-pool tests pass with cryptography 50.0.2 and separately with the minimum cryptography 38.0.3.
  • Full package unit suite: 2,149 passed, 7 skipped. Aggregate coverage is 97%; every changed executable line is covered.
  • Package-wide Ruff import/format checks (Ruff 0.14.14), flake8 and strict metadata/RST checks pass.

The supported Python/OS matrix and cloud system tests were not run. This change was developed with AI assistance and tested locally using fixture certificates.

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

The existing issue predates this patch. The user guide already describes a PEM-formatted trust-chain file, so no documentation correction is needed. Compared with unmodified base 3799568 in the same environment, statement coverage increased from 97.2398% to 97.2406%, and branch coverage increased from 95.8998% to 95.9613%.

Fixes #17623 🦕

Ensure trust chain data is processed correctly by skipping empty certificate blocks.
@dhairyajangir
dhairyajangir requested review from a team as code owners October 2, 2026 18:54
@google-cla

google-cla Bot commented Oct 2, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the _read_trust_chain method in identity_pool.py to correctly ignore any preamble text that appears before the first PEM certificate header (-----BEGIN CERTIFICATE-----). It also adds corresponding unit tests in test_identity_pool.py to cover various scenarios involving preambles, invalid trust chains, and empty trust chains. There are no review comments, so I have no feedback to provide.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

google-auth: gracefully handle leading whitespace in identity pool PEM parsing

1 participant