Remove implicit truncation behaviour in ossl_i2c_ASN1_BIT_STRING

and make ASN1_BIT_STRING_set_bit compute the unused bits of the
BIT_STRING.

The implicit trunction behaviour allows you to set a value without
keeping the unused bits consistent, using ASN1_STRING_set, and then
have it magically "fixed" to account for the unused bits in the last
octet on output.

As it turns out, after much searching, nothing is using this behavior,

As we now have the new ASN1_BIT_STRING_set1 to set the entire value
and keep the unused bits correct, we make ASN1_BIT_STRING_set_bit
also do the same. Now that both the setters change the object
correctly we remove the implicit trunctation in ossl_i2x_ASN1_BIT_STRING
and make the provided BIT_STRING argument const.

See discussion in https://github.com/openssl/openssl/issues/29185
and in https://github.com/openssl/openssl/issues/29117

For https://github.com/openssl/openssl/issues/29117

Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
MergeDate: Tue Feb 24 19:11:26 2026
(Merged from https://github.com/openssl/openssl/pull/29711)

diff --git a/CHANGES.md b/CHANGES.md
index 0ac57fedf07d3c056466d18719fe406d8b33b054..8d154ba26cc165b23837be54aed96a0b5649bda0 100644 (file)
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -212,7 +212,9 @@ OpenSSL 4.0
    *Beat Bolli*
 
  * Added `ASN1_BIT_STRING_set1()` to set a bit string to a value including
-   the length in bytes and the number of unused bits.
+   the length in bytes and the number of unused bits. Internally,
+   'ASN1_BIT_STRING_set_bit()' has also been modified to keep the number of
+   unused bits correct when changing an ASN1_BIT_STRING.
 
    * Bob Beck *
 

diff --git a/crypto/asn1/a_bitstr.c b/crypto/asn1/a_bitstr.c
index 29b0a03b005150c87d8d135645ea990346fc4a4e..3c090044cf86704554ccb9ae91920d25fb4b603d 100644 (file)
--- a/crypto/asn1/a_bitstr.c
+++ b/crypto/asn1/a_bitstr.c
@@ -39,55 +39,24 @@ int ASN1_BIT_STRING_set(ASN1_BIT_STRING *x, unsigned char *d, int len)
     return ASN1_STRING_set(x, d, len);
 }
 
-int ossl_i2c_ASN1_BIT_STRING(ASN1_BIT_STRING *a, unsigned char **pp)
+int ossl_i2c_ASN1_BIT_STRING(const ASN1_BIT_STRING *a, unsigned char **pp)
 {
-    int ret, j, bits, len;
+    int ret = 0, bits = 0, len;
     unsigned char *p, *d;
 
     if (a == NULL)
-        return 0;
+        goto err;
 
     len = a->length;
 
-    if (len > 0) {
-        if (a->flags & ASN1_STRING_FLAG_BITS_LEFT) {
-            bits = (int)a->flags & 0x07;
-        } else {
-            for (; len > 0; len--) {
-                if (a->data[len - 1])
-                    break;
-            }
-
-            if (len == 0) {
-                bits = 0;
-            } else {
-                j = a->data[len - 1];
-                if (j & 0x01)
-                    bits = 0;
-                else if (j & 0x02)
-                    bits = 1;
-                else if (j & 0x04)
-                    bits = 2;
-                else if (j & 0x08)
-                    bits = 3;
-                else if (j & 0x10)
-                    bits = 4;
-                else if (j & 0x20)
-                    bits = 5;
-                else if (j & 0x40)
-                    bits = 6;
-                else if (j & 0x80)
-                    bits = 7;
-                else
-                    bits = 0; /* should not happen */
-            }
-        }
-    } else
-        bits = 0;
+    if (len > INT_MAX - 1)
+        goto err;
+
+    if ((len > 0) && (a->flags & ASN1_STRING_FLAG_BITS_LEFT))
+        bits = (int)a->flags & 0x07;
 
-    ret = 1 + len;
     if (pp == NULL)
-        return ret;
+        goto done;
 
     p = *pp;
 
@@ -99,6 +68,11 @@ int ossl_i2c_ASN1_BIT_STRING(ASN1_BIT_STRING *a, unsigned char **pp)
         p[-1] &= (0xff << bits);
     }
     *pp = p;
+
+done:
+    ret = len + 1;
+
+err:
     return ret;
 }
 
@@ -197,8 +171,26 @@ int ASN1_BIT_STRING_set_bit(ASN1_BIT_STRING *a, int n, int value)
         a->length = w + 1;
     }
     a->data[w] = ((a->data[w]) & iv) | v;
+
     while ((a->length > 0) && (a->data[a->length - 1] == 0))
         a->length--;
+
+    if (a->length > 0) {
+        uint8_t u8 = a->data[a->length - 1];
+        uint8_t unused_bits = 7;
+
+        /* Only keep least significant bit; count trailing zeroes. */
+        u8 &= 0x100 - u8;
+        if ((u8 & 0x0f) != 0)
+            unused_bits -= 4;
+        if ((u8 & 0x33) != 0)
+            unused_bits -= 2;
+        if ((u8 & 0x55) != 0)
+            unused_bits -= 1;
+
+        if (!asn1_bit_string_set_unused_bits(a, unused_bits))
+            return 0;
+    }
     return 1;
 }
 

diff --git a/crypto/asn1/asn1_local.h b/crypto/asn1/asn1_local.h
index 7796603dd44a09dee6c65a9fed7944af4c183f5f..b131ca7be9d25d9b87439ad678909f7c1b49018d 100644 (file)
--- a/crypto/asn1/asn1_local.h
+++ b/crypto/asn1/asn1_local.h
@@ -78,7 +78,7 @@ void ossl_asn1_template_free(ASN1_VALUE **pval, const ASN1_TEMPLATE *tt);
 
 ASN1_OBJECT *ossl_c2i_ASN1_OBJECT(ASN1_OBJECT **a, const unsigned char **pp,
     long length);
-int ossl_i2c_ASN1_BIT_STRING(ASN1_BIT_STRING *a, unsigned char **pp);
+int ossl_i2c_ASN1_BIT_STRING(const ASN1_BIT_STRING *a, unsigned char **pp);
 ASN1_BIT_STRING *ossl_c2i_ASN1_BIT_STRING(ASN1_BIT_STRING **a,
     const unsigned char **pp, long length);
 int ossl_i2c_ASN1_INTEGER(ASN1_INTEGER *a, unsigned char **pp);

diff --git a/crypto/asn1/tasn_enc.c b/crypto/asn1/tasn_enc.c
index 8dea9f36539268cb3d4d90c9141ff37ba63373d2..fb3d7802ed0a67b305290ecda118b72b0f34696b 100644 (file)
--- a/crypto/asn1/tasn_enc.c
+++ b/crypto/asn1/tasn_enc.c
@@ -590,7 +590,7 @@ static int asn1_ex_i2c(const ASN1_VALUE **pval, unsigned char *cout, int *putype
         break;
 
     case V_ASN1_BIT_STRING:
-        return ossl_i2c_ASN1_BIT_STRING((ASN1_BIT_STRING *)*pval,
+        return ossl_i2c_ASN1_BIT_STRING((const ASN1_BIT_STRING *)*pval,
             cout ? &cout : NULL);
 
     case V_ASN1_INTEGER: