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 broken relaxation #1762

Merged
merged 2 commits into from
Dec 20, 2022
Merged

Conversation

wiktor-k
Copy link
Contributor

Hi Steven,

After a round of extensive integration testing and discussions on the OpenSSL mailing list it seems my "improvement" in #1733 was actually based on unsound promise. That is the num parameter does not reflect the internal cache size (it's always zero and used only internally).

I've basically reverted the old PR but kept the introduced function not to break the API (if you think it would be OK to remove minimal_output_size or #[deprecate] it I'll adjust). As for my client code I'll build a safe API around *_unchecked calls by keeping the additional cache state in my structs.

I'm super, super sorry for the annoyance I caused 🙇 I should've spent more time integration testing my previous PR but now I've caused you more trouble than necessary. Once again: my deepest apologies 🙏 !

@sfackler
Copy link
Owner

minimal_output_size is only in one release, so I think we can just yank that release and remove the function.

Sadly the condition used to relax output buffer checks that depended
on the `num` parameter does not really hold so this change effectively
reverts PR sfackler#1733.

As clarified on the OpenSSL mailing list [0] and during integration tests
the `num` parameter does not reflect the internal buffer cache size thus
one needs to pessimistically assume that each call to `cipher_update`
will need sufficient size to contain one additional block. Streaming
ciphers are not affected by this revert.

[0]: https://mta.openssl.org/pipermail/openssl-users/2022-December/015727.html
This mirrors the `Cipher::cipher_update_unchecked` API call for clients
that want to manually track the state of internal OpenSSL cipher buffer
size.
@sfackler sfackler merged commit f0ff8a7 into sfackler:master Dec 20, 2022
@wiktor-k wiktor-k deleted the fix-broken-relaxation branch December 20, 2022 14:09
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.

3 participants