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 PER encoding of empty octet strings missing the length determinant #393

Merged

Conversation

decryphe
Copy link
Contributor

During implementation of some more advanced ASN.1 descriptions, we ran across an issue where the length determinant would not be part of the payload if an octet string had an effective zero length.

This wasn't initially an issue, because the string we were parsing was located at the end of the buffer, and with zero initialized memory (on the C-parser-side), it found the correct length determinant of zero by accident. While building round-trip unit-tests for our payloads, this blew up - hence the amount of test code for the single line fix... :-)

I've tried to write some good-case/bad-case tests to not break any other functionality, and kept all other tests that I wrote while investigating the issue.

Happy new year! 🎉

@XAMPPRocky
Copy link
Collaborator

Thank you for your PR! The code looks good but it is failing tests.

@decryphe
Copy link
Contributor Author

I tried running cargo test, cargo test --all-targets, cargo clippy --all-targets. I'm on cargo 1.84.0-beta.5 (66221abde 2024-11-19) - what did I miss?

@XAMPPRocky
Copy link
Collaborator

Have tried with --workspace?

@decryphe
Copy link
Contributor Author

decryphe commented Jan 2, 2025

I tried both cargo test and cargo clippy with --all-targets --workspace, anything else I should try?

@XAMPPRocky
Copy link
Collaborator

Looking at the failed targets, they seem to be 32 bit, can you try compiling the workspace for one of the failed targets?

@decryphe decryphe force-pushed the topic/fix-uper-encode-empty-octet-string branch from d0c3be1 to d083c89 Compare January 2, 2025 23:16
@decryphe
Copy link
Contributor Author

decryphe commented Jan 2, 2025

Yep, finding it by running cross test --target i686-unknown-linux-gnu --workspace, it was easy to see that I had done an indexed access into a usize.to_be_bytes() array. Fixed and amended.

@XAMPPRocky XAMPPRocky merged commit 1c7521e into librasn:main Jan 3, 2025
65 checks passed
@decryphe decryphe deleted the topic/fix-uper-encode-empty-octet-string branch January 6, 2025 10:05
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