Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix spec compliant record size verification #5

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

p1gp1g
Copy link

@p1gp1g p1gp1g commented Feb 4, 2025

Spec compliant records were throwing issues during decryption due to a bug in record size verification: it was verified against the length of the record with the header which isn't what RFC8188 specify, leading to bugs with some push servers. For example:

Spec compliant records were throwing issues during decryption
due to a bug in record size verification
Copy link

google-cla bot commented Feb 4, 2025

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.

The recordSize is get from the encrypted payload. If we check against the exact
value of this.recordSize, and the server sends non-constant RS values, then
we need to parse the message twice: once to get the RS for the builder, and
again during decryption.

As of RFC8818, recordSize is used to set a maximum bound for the encrypted payload
to parse. Therefore, there is no issues if the recordSize we receive is lower than
the one we defined. As long as this recordSize is higher than the actual recode size.

this.recordSize is always < MAX_CIPHERTEXT_SIZE.
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.

1 participant