Fix #95 #107
Fix #95 #107
Conversation
|
Please note that the return value of |
src/include/uthenticode.h
Outdated
| private: | ||
| Certificate(X509 *cert); | ||
| explicit Certificate(X509 *cert); | ||
| Certificate(X509 *cert, unsigned long xn_flags); |
There was a problem hiding this comment.
I'd rather not expose a custom ctor here -- it effectively binds us to the OpenSSL ABI (via xn_flags, plus formalizing that we support OpenSSL's weird stringification, which wasn't intentional), and exposes a decision point that I suspect 99% of people using this library don't need.
If the goal is to change the default to XN_FLAG_RFC2253 + UTF-8 conversion, let's just do that and then I'll perform a major release to indicate potential breakage.
There was a problem hiding this comment.
Amended the original commit. Please have a look if this resolves the issue for you, @woodruffw.
hugmyndakassi
force-pushed
the
using-x509_name_print_ex-for-DNs
branch
from
cd8bb0a
to
4c02ad3
Compare
|
@woodruffw is there something you would like fixed in this PR prior to merging? I don't see any activity since my comment from last week (this could mean some message is stuck in draft state, invisible to me). |
|
Nope — I’ve just been traveling and haven’t had the time to review it. I’ll try and give it a review tonight.Sent from mobile. Please excuse my brevity.On Oct 15, 2024, at 8:59 AM, Þórhildur ***@***.***> wrote:
@woodruffw is there something you would like fixed in this PR prior to merging? I don't see any activity since my comment from last week.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
src/uthenticode.cpp
Outdated
| (void) name_to_string(issuer_, X509_get_issuer_name(cert), xn_flags); | ||
| (void) name_to_string(subject_, X509_get_subject_name(cert), xn_flags); |
There was a problem hiding this comment.
Just making sure I understand: you've put (void) here to tell the compiler to ignore the return type, right? If so assigning to std::ignore is probably a little more idiomatic, or just dropping the return type from name_to_string entirely.
|
NB: Per test failures, you'll need to update a few test strings to match the new string format: https://github.com/trailofbits/uthenticode/actions/runs/11233887711/job/31228529055?pr=107 |
hugmyndakassi
force-pushed
the
using-x509_name_print_ex-for-DNs
branch
from
4c02ad3
to
7e24241
Compare
- Certificate got another ctor which takes the flags to pass when formatting the X509_NAME values - The default formatting changed to XN_FLAG_RFC2253 but can be overridden from the outside by defining UTHENTICODE_DEFAULT_XN_FLAGS - This introduces an incompatibility _if_ the caller assumes that the issuer and subject can be compared in their string form
hugmyndakassi
force-pushed
the
using-x509_name_print_ex-for-DNs
branch
from
7e24241
to
8428b65
Compare
Should be resolved now. Not sure what about the mac errors, though. I have no mac to test. |
Don't worry about those -- that's been an issue for a while, and I haven't had time to triage it. We can merge with the mac builds failing. |
|
Thanks @hugmyndakassi! |
X509_NAMEvaluesXN_FLAG_RFC2253but can be overridden from the outside by definingUTHENTICODE_DEFAULT_XN_FLAGS