The comparison should notice differences in bit count.
Update-Note: ASN1_STRING_cmp no longer incorrectly treats BIT STRINGs
with different padding bits as equal.
Bug: 446
Change-Id: I22b3fcc5d369540d029ca234e9b3b02402cec4c3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49928
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
OpenSSL's BIT STRING representation has two modes, one where it
implicitly trims trailing zeros and the other where the number of unused
bits is explicitly set. This means logic in ASN1_item_verify, or
elsewhere in callers, that checks flags and ASN1_STRING_length is
inconsistent with i2c_ASN1_BIT_STRING.
Add ASN1_BIT_STRING_num_bytes for code that needs to deal with X.509
using BIT STRING for some fields instead of OCTET STRING. Switch
ASN1_item_verify to it. Some external code does this too, so export it
as public API.
This is mostly a theoretical issue. All parsed BIT STRINGS use explicit
byte strings, and there are no APIs (apart from not-yet-opaquified
structs) to specify the ASN1_STRING in X509, etc., structures. We
intentionally made X509_set1_signature_value, etc., internally construct
the ASN1_STRING. Still having an API is more consistent and helps nudge
callers towards rejecting excess bits when they want bytes.
It may also be worth a public API for consistently accessing the bit
count. I've left it alone for now because I've not seen callers that
need it, and it saves worrying about bytes-to-bits overflows.
This also fixes a bug in the original version of the truncating logic
when the entire string was all zeros, and const-corrects a few
parameters.
Change-Id: I9d29842a3d3264b0cde61ca8cfea07d02177dbc2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48225
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
At one point in the SSLeay days, all the ASN1_STRING typedefs were
separate structs (but only in debug builds) and the M_ASN1_* macros
included type casts to handle this.
This is long gone, but we still have the M_ASN1_* macros. Remove the
casts and switch code within the library to call the macros. Some
subtleties:
- The "MSTRING" types (what OpenSSL calls its built-in CHOICEs
containing some set of string types) are weird because the M_FOO_new()
macro and the tasn_new.c FOO_new() function behave differently. I've
split those into a separate CL.
- ASN1_STRING_type, etc., call into the macro, which accesses the field
directly. This CL inverts the dependency.
- ASN1_INTEGER_new and ASN1_INTEGER_free, etc., are generated via
IMPLEMENT_ASN1_STRING_FUNCTIONS in tasn_typ.c. I've pointed
M_ASN1_INTEGER_new and M_ASN1_INTEGER_free to these fields. (The free
function is a no-op, but consistent.)
- The other macros like M_ASN1_BIT_STRING_dup largely do not have
corresponding functions. I've aligned with OpenSSL in just using the
generic ASN1_STRING_dup function. But some others, like
M_ASN1_OCTET_STRING_dup have a corresponding ASN1_OCTET_STRING_dup
function. OpenSSL retained these, so I have too.
Update-Note: Some external code uses the M_ASN1_* macros. This should
remain compatible, but some type errors may have gotten through
unnoticed. This CL restores type-checking.
Change-Id: I8656abc7d0f179192e05a852c97483c021ad9b20
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44045
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Adam Langley