Skip to content

Unused bits in asn1 bit string #1610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
moshe-shahar opened this issue May 2, 2018 · 7 comments · Fixed by #2028
Closed

Unused bits in asn1 bit string #1610

moshe-shahar opened this issue May 2, 2018 · 7 comments · Fixed by #2028
Labels
bug component-crypto Crypto primitives and low-level interfaces

Comments

@moshe-shahar
Copy link
Contributor

Description

  • Type: Bug
  • Priority: Minor

mbed TLS build:
Version: 2.8.0 (latest)


Bugs

The unused bits in mbedtls_asn1_write_bitstring are not as expected and should contain also the trailing zeros.
Tested against Python cryptography library

From spec:

Named bit lists are BIT STRINGs where the values have been assigned
names. This specification makes use of named bit lists in the
definitions for the key usage, CRL distribution points, and freshest
CRL certificate extensions, as well as the freshest CRL and issuing
distribution point CRL extensions. When DER encoding a named bit
list, trailing zeros MUST be omitted.
That is, the encoded value
ends with the last named bit that is set to one.

@RonEld
Copy link
Contributor

RonEld commented May 3, 2018

For reference, the The quote is from RFC5280 Appendix B

@RonEld
Copy link
Contributor

RonEld commented May 3, 2018

@moshe-shahar Please specify what is the current behavior, what is the expected behavior and how to reproduce.

@moshe-shahar
Copy link
Contributor Author

Current behavior is to set the unused bits field in the DER to modulo 8 on bits argument.
Expected behavior is set the unused bits field to include the trailing zeros.
Example for 1000000 bit string with 7 bits len:
Current unused bits is 1.
Expected is 7 (1+6 trailing zeros).

@trianglee
Copy link

@sbutcher-arm This defect is now biting us - we cannot encode a specific field we need to encode in a CSR. Is there any forecast to when this could be looked at?

@ciarmcom
Copy link

ARM Internal Ref: IOTSSL-2526

@hanno-becker
Copy link

@moshe-shahar I think we need to distinguish between raw bitstrings, which may include trailing zero bits, and named bit lists, for which such are forbidden in DER. Changing mbedtls_asn1_write_bitstring() to remove trailing bits doesn't seem to be the right approach to me. Instead, I think we should add a new pair of API calls for parsing and writing named bit lists. See #2028 for more details. Apologies if I mixed up things, please let me know if you see the matter differently.

@moshe-shahar
Copy link
Contributor Author

@hanno-arm, I can't comment or suggest what is the correct fix since I'm not sure I'm familiar with all cases this library should support.

@RonEld RonEld added the component-crypto Crypto primitives and low-level interfaces label Feb 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-crypto Crypto primitives and low-level interfaces
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants