-
Notifications
You must be signed in to change notification settings - Fork 156
[tool-openssl] basic asn1parse support #2882
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2882 +/- ##
==========================================
- Coverage 78.26% 78.21% -0.05%
==========================================
Files 683 685 +2
Lines 117604 118083 +479
Branches 16515 16620 +105
==========================================
+ Hits 92042 92358 +316
- Misses 24675 24839 +164
+ Partials 887 886 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return 1; | ||
| } | ||
|
|
||
| static int asn1_parse_string_type(BIO *bp, const uint8_t *data, long len) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How paranoid should we be about the string. There are several different encodings that this could be, some of which could have an invalid length (e.g., multi-byte UTF8) or contain control characters.
Should we basically have this behave similar to asn1_parse_octet_string_type in relation to validating ASCII content?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function entry point is guarded by a check on the explicit ASN.1 types that are either numbers, digits, ascii, or subset of ascii, and UTF-8, which appears to be assumed that the output destination for the BIO, be that a terminal interface or a file supports UTF-8 encoding. I've checked OpenSSL 3+ and the behavior is the same there as well.
2683105
Issues:
Resolves P342458791
Description of changes:
Adds basic support for
openssl asn1parsewith the following arguments:The behavior of the command will default to OpenSSL's
-strictpemflag behavior which was an optional feature. We can revisit if this determined to be required in order to relax the input restriction for valid PEM blocks.Call-outs:
The original asn1parse function used
ASN1_dumprather thenASN1_parse, under the hood they are the same function, except thatASN1_dumpcan be given a flag to indicate that unknown data should be hex dumped out. As our BIO hexdump functions doesn't match OpenSSL's the output would be slightly different. For now I've opted to not support the-dumpflag for the CLI, and chose to only exposeASN1_parsein the library. That way we could come back and add-dumpandASN1_dumpwith proper output without worrying about backwards compatibility concerns.Testing:
ASN1_parselibrary function.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.