Skip to content

Conversation

@baloo
Copy link
Member

@baloo baloo commented Mar 13, 2025

Since base64ct 1.7.0 (#1387), we now rejects zero lengths reads. This happens to trigger with an sha256WithRSAEncryption AlgorithmIdentifier where the parameters are null.

@baloo baloo force-pushed the baloo/der/zero-read-pem branch from 96924aa to 7b6609b Compare March 13, 2025 05:07
@baloo baloo mentioned this pull request Mar 13, 2025
@baloo
Copy link
Member Author

baloo commented Mar 13, 2025

I'm actually wondering if a rollback might not be better though. base64ct 1.7 is going to break der 0.7 too.
Either

--- a/base64ct/src/decoder.rs
+++ b/base64ct/src/decoder.rs
@@ -105,7 +106,7 @@
     /// - `Err(Error::InvalidLength)` if the exact amount of data couldn't be read, or
     ///   if the output buffer has a length of 0
     pub fn decode<'o>(&mut self, out: &'o mut [u8]) -> Result<&'o [u8], Error> {
-        if out.is_empty() {
+        if self.is_finished() && out.is_empty() {
             return Err(InvalidLength);
         }

or:

--- a/base64ct/src/decoder.rs
+++ b/base64ct/src/decoder.rs
@@ -106,7 +107,7 @@
     ///   if the output buffer has a length of 0
     pub fn decode<'o>(&mut self, out: &'o mut [u8]) -> Result<&'o [u8], Error> {
         if out.is_empty() {
-            return Err(InvalidLength);
+            return Ok(out);
         }

         if self.is_finished() {

@baloo baloo force-pushed the baloo/der/zero-read-pem branch 2 times, most recently from 1032ea4 to 3fa431b Compare March 13, 2025 05:18
@tarcieri
Copy link
Member

Yeah, sounds like we should revert the change in base64ct and publish another 1.7 release

tarcieri added a commit that referenced this pull request Mar 13, 2025
This reverts commit a375cbf.

This was a breaking change which is causing issues in the `der` crate.

See #1711
@tarcieri
Copy link
Member

Opened #1714 to revert the change

@tarcieri
Copy link
Member

@baloo I think we can potentially merge this as well as #1714, if you want to fix rustfmt

tarcieri added a commit that referenced this pull request Mar 13, 2025
This reverts commit a375cbf.

This was a breaking change which is causing issues in the `der` crate.

See #1711
baloo added 2 commits March 13, 2025 09:22
Since base64ct 1.7.0 ([RustCrypto#1387]), it now rejects zero lengths reads. This 
happens to trigger with an sha256WithRSAEncryption AlgorithmIdentifier
where the parameters are null.
Since base64ct 1.7.0 ([RustCrypto#1387]), it now rejects zero lengths reads. This 
happens to trigger with an sha256WithRSAEncryption AlgorithmIdentifier
where the parameters are null.
@baloo
Copy link
Member Author

baloo commented Mar 13, 2025

Closed in favor of #1716, the test case was merged there.

@baloo baloo closed this Mar 13, 2025
@baloo baloo deleted the baloo/der/zero-read-pem branch March 13, 2025 18:38
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.

2 participants