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

Undocumented breaking change in v0.9: expose-internals removed with no replacement #351

Closed
tchebb opened this issue Jul 21, 2023 · 1 comment

Comments

@tchebb
Copy link
Contributor

tchebb commented Jul 21, 2023

In v0.8 and prior, the expose-internals feature, when enabled, would export several low-level functions that generally shouldn't be used directly. Keeping those functions behind an "I know what I'm doing" feature flag was IMO a good compromise, since it gave the crate secure defaults but also let users who really need those functions access them without patching the crate.

Unfortunately, with v0.9 and #304, those functions were moved to various other places and hidden behind multiple layers of pub(crate), meaning downstream users of rsa have no way at all to use them anymore (even though this commit message acknowledges that "it is expected that the functions inside algorithms crates might be useful (and used) by other parties"). The expose-internals feature flag was removed too, but none of this was documented in CHANGELOG.md.

I now find myself unable to upgrade one of my projects from v0.8 to v0.9 because it relies on the raw encrypt() and decrypt() functions. The project (which unfortunately isn't open source, or I'd link to it) decrypts firmware images for a commercial piece of hardware, the format of which I reverse engineered. As such, I have no control over what crypto operations it uses: the encryption scheme was defined by the vendor, and my code needs to implement it as is—bad choices and all. One of the bad choices the vendor made was to encrypt blocks of data with no padding whatsoever, which is why I need encrypt().

Would you accept a PR to bring expose-internals back? The functions themselves don't have to move back—internals could just re-export them from their new locations, and we could remove pub(crate) selectively on only the ones that make up the internal API. I personally only need encrypt() and decrypt(), so I'd be fine leaving the rest hidden unless/until someone demonstrates a compelling need for them.

@tchebb tchebb changed the title Undocumented breaking change in 0.9: expose-internals removed with no replacement Undocumented breaking change in v0.9: expose-internals removed with no replacement Jul 21, 2023
@tarcieri
Copy link
Member

tarcieri commented Jul 21, 2023

It seems OK to re-expose, although it might make sense to consider "hazmat" branding for it like we've done in some other crates: https://github.com/RustCrypto/meta/blob/master/HAZMAT.md

Here is an example of a hazmat module and associated documentation: https://docs.rs/aes/latest/aes/hazmat/index.html

Feel free to send a PR for a CHANGELOG entry as well.

tchebb added a commit to tchebb/RSA that referenced this issue Jul 21, 2023
External access to these functions was removed in RustCrypto#304 when the old
`internals` module and `expose-internals` feature were removed. There
are some valid use cases for them, though (see RustCrypto#351), so let's bring
back a subset of what was in `internals` using the same naming and
documentation conventions that the aes crate uses for its hazardous
functions.

Much of the added or changed documentation is derived from that in aes.

Fixes RustCrypto#351.
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

No branches or pull requests

2 participants