Yan-Daojiang has posted comments on this change. ( http://gerrit.cloudera.org:8080/23429 )
Change subject: OpenSSL 3.x compatibility adaptation ...................................................................... Patch Set 8: (7 comments) http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/security/crypto.cc File src/kudu/security/crypto.cc: http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/security/crypto.cc@340 PS7, Line 340: f OPENSSL_VERSION_NUMBER >= 0x30000000L > Is this needed for OpenSSL 3.0 and newer? Thanks! This allocation is not needed for OpenSSL 3.0 and newer, it incurs unnecessary allocation/free. I’ll move the EVP_PKEY_new() call into the < 3.0 branch only and, for >= 3.0, adopt the generated key directly (e.g., ret->AdoptRawData(gen)), reducing overhead and making the code clearer. http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/jwt-util.cc File src/kudu/util/jwt-util.cc: http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/jwt-util.cc@146 PS7, Line 146: } // namespace security : : : // JWK Set (JSON Web Key Set) is JSON data structure that represents a set of JWKs. : // > Is it possible to reconcile this and SslTypeTraits<EVP_PKEY> from openssl_u Yes. I reconciled this by extending the generic SslTypeTraits<EVP_PKEY> (for OpenSSL 3.0+) to include kWritePemFunc and kWriteDerFunc pointing to PEM_write_bio_PUBKEY and i2d_PUBKEY_bio. With that in place, jwt-util.cc no longer needs a local EvpPublicKeyTraits; the calls were updated to ToString<EVP_PKEY> to use the default traits. This is a minimal change and does not affect the OpenSSL 1.x path. http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/jwt-util.cc@462 PS7, Line 462: auto mod = bn_from_bytes(str_n); : auto exp = bn_from_bytes(str_e); : : auto ctx = ssl_make_unique(EVP_PKEY_CTX_new_from_name(nullptr, "RSA", nullptr)); : OPENSSL_RET_IF_NULL(ctx.get(), "failed to create EVP_PKEY_CTX for RSA"); : int rc = EVP_PKEY_fromdata_init(ctx.get()); : if (rc <= 0) { : return Statu > This part looks exactly the same as at lines 494 -- 501. Is it possible to Done http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/jwt-util.cc@488 PS7, Line 488: au > Isn't it necessary to check 'rc' for an error? Done http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/jwt-util.cc@664 PS7, Line 664: // with coordinate lengths pre-validated via BN_bn2binpad against the : // expected 'coord_len'. > nit: please add a comment to explain why these shouldn't fail Done http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/jwt-util.cc@672 PS7, Line 672: > Isn't it necessary to check for an error here? Done http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/openssl_util.cc File src/kudu/util/openssl_util.cc: http://gerrit.cloudera.org:8080/#/c/23429/7/src/kudu/util/openssl_util.cc@251 PS7, Line 251: "OpenSSL for multi-threaded usage (setting thread callback " : "functions for OpenSSL of versions earlier than 1.1.0) and " : "then call kudu::client::DisableOpenSSLInitialization() " : "to avoid potential crashes due to conflicting initialization"); : // > If this is already under the 'else' part of the '#if OPENSSL_VERSION_NUMBER Thank you for your reminder. It was my negligence that caused the redundancy here. I have now modified this place. -- To view, visit http://gerrit.cloudera.org:8080/23429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic587a85e6b9088ffd353f9119b75431f1ec60b5c Gerrit-Change-Number: 23429 Gerrit-PatchSet: 8 Gerrit-Owner: Yan-Daojiang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yan-Daojiang <[email protected]> Gerrit-Comment-Date: Fri, 10 Oct 2025 14:30:12 +0000 Gerrit-HasComments: Yes
