KerberosFlags field has invalid size after decoding or encoding

[sk3w]

The BitString decoder and encoder both appear to truncate null bytes, resulting in invalid values for some KerberosFlags fields (which must be at least 32 bits). For example, my Windows 10 test machine generates a TGS-REQ with a KDCOptions field 0x40810000 (0x03050040810000 including the tag and length). That field decodes to a 16-bit BitString value of 0x4081. Similarly, the encoder will produce 0x0303004081 from the 32-bit BitString value, which gets rejected by the KDC. Removing the truncation on both encode_bit_string() and decode_bit_string() fixes my issue. I would try to put together a PR but I'm not sure the preferred way to handle this (ex. parameterize BitString with a min length, add a new type, etc.)

fn test_dec() {
    let input = b"\x03\x05\x00\x40\x81\x00\x00";
    let mut decoder = ber::de::Decoder::new(input, ber::de::DecoderOptions::ber());
    let output = decoder
        .decode_bit_string(Tag::new(Class::Universal, 3))
        .unwrap();
    let expected = KerberosFlags::from_vec([0x40, 0x81, 0x00, 0x00].to_vec());
    assert_eq!(output, expected)
}

fn test_enc() {
    let bitstring = KerberosFlags::from_vec([0x40, 0x81, 0x00, 0x00].to_vec());
    let mut encoder = ber::enc::Encoder::new(ber::enc::EncoderOptions::ber());
    encoder
        .encode_bit_string(Tag::new(Class::Universal, 3), &bitstring)
        .unwrap();
    assert_eq!(encoder.output(), vec![0x03, 0x05, 0x00, 0x40, 0x81, 0x00, 0x00])
}

[XAMPPRocky]

Thank you for your issue! Contributions are always welcome and appreciated 🙂 I have added support for constraints in the per branch, but I haven't changed the current behaviour of the BER decoder and encoder or data types in the other crates. It's not quite ready to be merged to main (you're still welcome to create a PR against that branch), we can also update the ber encoder to take advantage of it.

[sk3w]

Is there still a need to add a size constraint to KerberosFlags? I looked into it after the latest release, but wasn't sure about the changes needed for the BER encoder / decoder.

[XAMPPRocky]

@sk3w Yes there is, we need to add a size restriction to KerberosFlags. You shouldn't need to change the BER encoder / decoder (that's a separate thing you can do, but don't need to see #139), what I would do is add a MinSizedOctetString<const MIN: usize> and then copy FixedSizeOctetString except use a Vec instead.