cmcfarlen commented on code in PR #13482:
URL: https://github.com/apache/trafficserver/pull/13482#discussion_r3707749131
##########
src/api/InkAPI.cc:
##########
@@ -8316,10 +8316,10 @@ TSSslServerCertUpdate(const char *cert_path, const char
*key_path)
}
// Extract common name
- int pos =
X509_NAME_get_index_by_NID(X509_get_subject_name(cert.get()), NID_commonName,
-1);
- X509_NAME_ENTRY *common_name =
X509_NAME_get_entry(X509_get_subject_name(cert.get()), pos);
- ASN1_STRING *common_name_asn1 = X509_NAME_ENTRY_get_data(common_name);
- char *common_name_str = reinterpret_cast<char *>(const_cast<unsigned char
*>(ASN1_STRING_get0_data(common_name_asn1)));
+ int pos =
X509_NAME_get_index_by_NID(X509_get_subject_name(cert.get()), NID_commonName,
-1);
+ auto *common_name =
X509_NAME_get_entry(X509_get_subject_name(cert.get()), pos);
+ auto *common_name_asn1 = X509_NAME_ENTRY_get_data(common_name);
+ char *common_name_str = reinterpret_cast<char *>(const_cast<unsigned char
*>(ASN1_STRING_get0_data(common_name_asn1)));
if (ASN1_STRING_length(common_name_asn1) !=
static_cast<int>(strlen(common_name_str))) {
Review Comment:
Not addressing this one here — it is a pre-existing bug, not something this
PR introduces. The `strlen` check and the `const_cast` on the
`ASN1_STRING_get0_data` result both predate this change; the only edit on these
lines was replacing the explicit `X509_NAME_ENTRY *` / `ASN1_STRING *`
declarations with `auto *` so they compile against both the const and non-const
OpenSSL signatures.
The concerns are real though, and worth their own issue:
- `pos == -1` when the cert has no CN, so `X509_NAME_get_entry(name, -1)`
returns null and `X509_NAME_ENTRY_get_data(nullptr)` dereferences it.
- `strlen` on a buffer OpenSSL does not guarantee is NUL-terminated. Note
the intent of that line is specifically to *detect* an embedded NUL by
comparing against `ASN1_STRING_length`, so a fix needs to preserve that check.
Keeping this PR scoped to OpenSSL 4.0 compatibility since it is wanted for
the upcoming release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
