Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/23097 )
Change subject: IMPALA-13235: [Patch 2 of 5] - Add OpenSSL Utility Function to Validate PEM Bundles ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/23097/3/be/src/util/openssl-util.cc File be/src/util/openssl-util.cc: http://gerrit.cloudera.org:8080/#/c/23097/3/be/src/util/openssl-util.cc@180 PS3, Line 180: else if (not_before > 0) { > Any way to trigger this error in test? Good question. Did more digging, and the OpenSSL docs say PEM_read_bio_X509 returns NULL if there is an error: https://docs.openssl.org/1.1.1/man3/PEM_read_bio_PrivateKey/#return-values The X509_get0_notBefore and X509_get0_notAfter functions do not return NULL because they return data from a certificate that passed validation in the PEM_read_bio_X509 function: https://docs.openssl.org/1.1.1/man3/X509_get0_notBefore/#return-values:~:text=X509_get0_notBefore()%2C%20X509_get0_notAfter()%20and%20X509_CRL_get0_lastUpdate()%20return%20a%20pointer%20to%20an%20ASN1_TIME%20structure. The X509_cmp_current_time function returns 0 if there is an error and 1 or -1 otherwise. My research did not turn up a way to make this case happen unless a NULL is passed to this function. Since the output from functions that don't return NULL is passed to this function, it shouldn't be possible to get a 0 returned from these functions in this case. Still, I added checks just in case that happens. The docs say nothing about any of these functions setting an OpenSSL error http://gerrit.cloudera.org:8080/#/c/23097/3/be/src/util/openssl-util.cc@198 PS3, Line 198: > Why this is not called when returning early in L180? error handling has been reworked, and this is no longer applicable -- To view, visit http://gerrit.cloudera.org:8080/23097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e9f12e854b679ccf3e94a38c2688a7431460a7a Gerrit-Change-Number: 23097 Gerrit-PatchSet: 6 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: gaurav singh <[email protected]> Gerrit-Comment-Date: Fri, 27 Jun 2025 20:32:36 +0000 Gerrit-HasComments: Yes
