Skip to content

reject unsuccessful ocsp responses that carry response bytes - #15901

Merged
alex merged 2 commits into
pyca:mainfrom
jmestwa-coder:ocsp-unsuccessful-response-bytes
Nov 14, 2026
Merged

alex merged 2 commits into
pyca:mainfrom
jmestwa-coder:ocsp-unsuccessful-response-bytes

Conversation

@jmestwa-coder

Copy link
Copy Markdown
Contributor

load_der_ocsp_response rejects a successful response with no responseBytes, but not the reverse:

  • a tryLater, unauthorized or other non-successful status that still carries a BasicOCSPResponse loads fine
  • every BasicResponse property then returns the embedded data instead of raising ValueError as documented, e.g. resp-sha256.der with the status flipped to tryLater reports TRY_LATER and certificate_status GOOD
  • RFC 6960 4.2.1 only allows responseBytes with a successful status, and responseStatus sits outside the signed data

Now raises ValueError at load time, with a regression test and a changelog entry.

Comment thread tests/x509/test_ocsp.py Outdated
# Change the responseStatus from successful (0) to tryLater (3),
# leaving the BasicOCSPResponse in place.
assert data[4:7] == b"\x0a\x01\x00"
data[6] = 3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not the right way to do this -- we want a checked in vector with teh right bytes, not a mutation of an existing vector.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Added x509/ocsp/resp-unsuccessful-response-bytes.der (resp-sha256.der with the status set to tryLater), documented it in test-vectors.rst, and the test now just loads it.

alex
alex previously approved these changes Nov 7, 2026
@alex

alex commented Nov 7, 2026

Copy link
Copy Markdown
Member

merge conflict -- I think you can just remove the changelog entry entirely.

@jmestwa-coder

Copy link
Copy Markdown
Contributor Author

Rebased on main and dropped the changelog entry, so the conflict's gone.

@alex
alex merged commit 4770314 into pyca:main Nov 14, 2026
67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants