BIT STRING hard-coded padding bits

[Brassinolide]

BIT STRING require an extra byte to indicate the number of padding bits

But the code here hard-codes 0 as the padding bits, this resulted in encoding errors in some cases.

The encoding logic of OCTET STRING and BIT STRING should be separated. For BIT STRING, the number of padding bits can be manually specified.

botan/src/lib/asn1/der_enc.cpp

Line 329 in feb9b3f

encoded.push_back(0);

[randombit]

Currently the only uses for BIT STRING are effectively cases that should have been octet strings, and full bytes are used. Do you have a case where the ASN.1 is actually encoding a bit string that is not a multiple of 8?

[Brassinolide]

Currently the only uses for BIT STRING are effectively cases that should have been octet strings, and full bytes are used. Do you have a case where the ASN.1 is actually encoding a bit string that is not a multiple of 8?

Yes, for example, in the RFC3161 timestamp protocol, most of the PKIFailureInfo enumeration values are not multiples of 8.

For example, the badDataFormat is 5, it needs to be padded with 2 zeros, the encoded value is 0x03 0x02 0x02 0x04

If the original code is used, it will be encoded as 0x03 0x02 0x00 0x04, resulting in an error

PKIFailureInfo ::= BIT STRING {
   badAlg               (0),
     -- unrecognized or unsupported Algorithm Identifier
   badRequest           (2),
     -- transaction not permitted or supported
   badDataFormat        (5),
     -- the data submitted has the wrong format
   timeNotAvailable    (14),
     -- the TSA's time source is not available
   unacceptedPolicy    (15),
     -- the requested TSA policy is not supported by the TSA
   unacceptedExtension (16),
     -- the requested extension is not supported by the TSA
    addInfoNotAvailable (17)
      -- the additional information requested could not be understood
      -- or is not available
    systemFailure       (25)
      -- the request cannot be handled due to system failure  }

Here is my temporary solution without modifying the library

void encode_bitstring(Botan::DER_Encoder& codec, std::span<const uint8_t> bitstring, uint8_t padding) {
    std::vector<uint8_t> encoded;
    encoded.push_back(padding);
    encoded.insert(encoded.end(), bitstring.begin(), bitstring.end());

    codec.add_object(Botan::ASN1_Type::BitString, Botan::ASN1_Class::Universal, encoded);
}

std::pair<std::vector<uint8_t>, uint8_t> bitpos_to_vec(size_t bit) {
    size_t byte_index = bit / 8;
    size_t bit_index = bit % 8;

    std::vector<uint8_t> result(byte_index + 1, 0);
    result[byte_index] |= (1 << (7 - bit_index));

    return { result,  (7 - bit_index) == 7 ? 0 : (7 - bit_index) };
}

Botan::DER_Encoder encoder;

const auto vec = bitpos_to_vec(5);
encode_bitstring(encoder, vec.first, vec.second);

// 0x03 0x02 0x02 0x04
encoder.get_contents();

[randombit]

Yes, for example, in the RFC3161 timestamp protocol, most of the PKIFailureInfo enumeration values are not multiples of 8.

Thanks for the context. I'll think about what we should do here.