A mirror of https://github.com/libressl/openbsd.git https://git.lua4.win/openbsd/atom?h=OPENBSD_7_9_BASE 2026-02-08T17:17:03+00:00 2026-02-08T17:17:03+00:00 tb 2026-02-08T17:17:03+00:00 urn:sha1:c1d87b1072b9f7d2783c4658953f787fb56f26a1 2026-02-08T10:27:00+00:00 tb 2026-02-08T10:27:00+00:00 urn:sha1:50933fb9bc6bf2281489d17ee48416a43163d847 Instead of relying on i2c_ASN1_BIT_STRING() to determine the "unused" bits on encoding, set them explicitly in abs->flags via a call to asn1_abs_set_unused_bits(). This means ASN1_STRING_FLAGS_BITS_LEFT is now set on a bit string, which was previously explicitly cleared. This also means that the encoding of a non-zero ASN1_BIT_STRING populated by setting the bits individually will now go through the if (a->flags & ASN1_STRING_FLAG_BITS_LEFT) path in i2c_ASN1_BIT_STRING(). The most prominent usage of this function is in X.509 for the keyUsage extension or the CRL reason codes. There's also the NS cert type, TS PKIFailureInfo and general BITLIST config strings. The reason for the truncation logic comes from the DER for NamedBitLists X.690, 11.2.2 below: X.680, 22.7: When a "NamedBitList" is used in defining a bitstring type ASN.1 encoding rules are free to add (or remove) arbitrarily any trailing 0 bits to (or from) values that are being encoded or decoded. Application designers should therefore ensure that different semantics are not associated with such values which differ only in the number of trailing 0 bits. X.690, 11.2.2 Where ITU-T Rec. X.680 | ISO/IEC 8824-1, 22.7, applies, the bitstring shall have all trailing 0 bits removed before it is encoded. Note 1 - In the case where a size constraint has been applied, the abstract value delivered by a decoder to the application will be one of those satisfying the size constraint and differing from the transmitted value only in the number of trailing zero bits. Note 2 - If a bitstring value has no 1 bits, then an encoder shall encode the value with a length of 1 and an initial octet set to 0. ok kenjiro (on an earlier version) jsing 2026-01-04T09:54:23+00:00 tb 2026-01-04T09:54:23+00:00 urn:sha1:98b725232ef3d7792add084ade8884054ebc5094 no binary change 2026-01-04T09:51:42+00:00 tb 2026-01-04T09:51:42+00:00 urn:sha1:f65bd896414ed2e25bad00527fa4f1c5fabdad09 A nasty quirk in the bit string handling is that the serialization produced by i2d_ASN1_BIT_STRING() depends on whether the the magic ASN1_STRING_FLAG_BITS_LEFT is set. If ASN1_STRING_FLAG_BITS_LEFT is set, the number of unused bits is carried in a->flags & 0x07 and the remainder of the bit string is in a->data. This is terrible and undocumented but handled correctly. If ASN1_STRING_FLAG_BITS_LEFT is not set, all trailing zero bits are (intended to be) chopped off with all sorts of hilarious side effects. I broke this quite thoroughly when I incorrectly ported an overflow check from BoringSSL in: https://github.com/openbsd/src/commit/f81cc285d2aed8b36615119a306533696f3eb66c The result is that we currently return ret = a->length + 1 for both NULL and non-NULL pp. The calls to asn1_ex_i2c() in asn1_i2d_ex_primitive() thus report consistent lengths back, making it succeed. asn1_i2d_ex_primitive() therefore skips a->length + 1 bytes, while i2c_ASN1_BIT_STRING() only overwrites len + 1 bytes, which are possibly fewer. So a caller passing in an output buffer containing garbage (malloc) will get some of that garbage back in the encoding. Further, i2c_ASN1_BIT_STRING() also advances that pointer by the possibly reduced len + 1, but that fortunately doesn't matter since that's an effect local to asn1_ex_i2c(), the only caller of i2c_ASN1_BIT_STRING(). The last bit is that the current behavior may set bogus unused bits coming from the scanning backward madness. I added such an example in the parent commit. The fix is simple: use len after the truncation effect was established, not the original a->length, turning this commit into what my backport should have been. This fixes the two currently failing regress tests, so remove expected failure marker again. ok jsing kenjiro 2026-01-04T09:30:57+00:00 tb 2026-01-04T09:30:57+00:00 urn:sha1:efbc823179eebc9ec3fa32135df0af814522aef6 The /* should not happen */ happens if a->data is all zeroes (where there used to be an OOB access a few years back), which is a legitimate BIT STRING, so this is just nonsense. ok jsing kenjiro 2026-01-04T09:29:29+00:00 tb 2026-01-04T09:29:29+00:00 urn:sha1:e3fc5ce7fcb410fc9163db2d569ddec064139b9b 2025-05-10T05:54:39+00:00 tb 2025-05-10T05:54:39+00:00 urn:sha1:41e8f99dd1625a9f0c80ce9d4383e95b18e85709 ok jsing 2024-07-08T14:52:31+00:00 beck 2024-07-08T14:52:31+00:00 urn:sha1:3e4afb686e802ed188471899b25a031f4c4164af ok tb@ 2023-12-25T22:02:59+00:00 tb 2023-12-25T22:02:59+00:00 urn:sha1:a42068ce674b48e2f26710269f43fa6c18e895dc If a negative n is passed, these functions would underrun the bitstring's data array. So add checks for that and drop spades of unnecessary parens. These functions are quite broken anyway. The setter attempts to zap the unnecessary trailing zero octets, but fails to do so if the bit being cleared isn't already set. Worse is the getter where you can't tell an error (like attempting an out-of-bounds read) from the bit being unset. ok joshua 2023-07-28T10:33:13+00:00 tb 2023-07-28T10:33:13+00:00 urn:sha1:b8d30e719ace66cc62ab262ddf8f89df4046d5e8 This removes ASN1_BIT_STRING_name_print(), ASN1_BIT_STRING_{num,set}_asc(). Before trust was properly handled using OIDs, there was a period where it used bit strings. The actual interfaces used in openssl x509 were removed, but the functions they wrapped remained unused for the next 24 years. ok jsing