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

DER encoded BIT STRING produces BitString with incorrect length #134

Closed
benmaddison opened this issue Aug 2, 2023 · 3 comments · Fixed by #135
Closed

DER encoded BIT STRING produces BitString with incorrect length #134

benmaddison opened this issue Aug 2, 2023 · 3 comments · Fixed by #135

Comments

@benmaddison
Copy link
Contributor

benmaddison commented Aug 2, 2023

Decoding a DER-encoded BIT STRING with a non-zero "unused bits" byte into a types::BitString results in a value padded with 0s to the nearest byte.

This makes it impossible to tell the difference between values with "real" trailing zeros and those without.

In particular, RFC3779 and other RPKI-related standards use a BIT STRING to encode IP prefixes, indicating the length of the prefix through the bit-length of the BIT STRING. This issue makes it impossible to correctly decode IP prefixes with lengths that are not a multiple of 8.

Note that I have only tried this with the DER implementation. I do not know whether the other codecs suffer from a similar issue.

The issue can be reproduced with the following minimal example:

use bitvec::{bitvec, order::Msb0};
use rasn::{der, types::BitString};

fn main() -> Result<(), Box<dyn std::error::Error>> {
    // DER-encoded byte-slice of BIT STRING B'0101'
    let encoded: &[u8] = &[
        0x03,        // tag = 3
        0x02,        // length = 2
        0x04,        // unused bits = 4
        0b0101_0000, // value
    ];
    let bit_string = der::decode::<BitString>(encoded)?;

    // Expected value
    let expected = bitvec![u8, Msb0; 0, 1, 0, 1];

    assert_eq!(bit_string, expected);

    Ok(())
}

I will take a look at the relevant code, and propose a fix if I can grok it easily enough.

@XAMPPRocky
Copy link
Collaborator

Thank you for your issue! Just to check. Have you tested with the latest version? I'm pretty sure I fixed this last week 39a9f47

@XAMPPRocky
Copy link
Collaborator

Ah nvm, it seems I didn't add back in the intentional bit string truncation. So it should be simple to just adjust the decode and encode to do that.

Note that I have only tried this with the DER implementation. I do not know whether the other codecs suffer from a similar issue.

This is only relevant for BER, PER bit strings don't have the concept of extra bits since it is a bit-level codec.

@benmaddison
Copy link
Contributor Author

@XAMPPRocky have attempted a fix in #135.
Let me know your thoughts when you have a sec.
I have tested on my project, and this fixes the bug that lead me here.

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 a pull request may close this issue.

2 participants