You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2020/10/22 21:24:01 UTC
[kudu-CR] Fix order of clearing openssl error and printing it
Hello Alexey Serbin, Andrew Wong, Grant Henke,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/16631
to review the following change.
Change subject: Fix order of clearing openssl error and printing it
......................................................................
Fix order of clearing openssl error and printing it
When verifying the certificate chain fails with an error other than
self-signed certificate, we try to get the subject and issuer to print
in the error message. Unfortunately X509NameToString(), the method doing
the conversion, also checks that there are no leftover OpenSSL errors,
so it fails immediately on call. This commit changes the behavior to
clear the errors *before* calling X509NameToString().
Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
---
M src/kudu/security/tls_context.cc
1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16631/1
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
[kudu-CR] Fix order of clearing openssl error and printing it
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing openssl error and printing it
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16631/1//COMMIT_MSG
Commit Message:
PS1:
What was the error we saw here? Is there any way to test this so we don't regress it in the future?
http://gerrit.cloudera.org:8080/#/c/16631/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
http://gerrit.cloudera.org:8080/#/c/16631/1/src/kudu/security/tls_context.cc@248
PS1, Line 248: ERR_clear_error(); // in case it left anything on the queue.
nit: Can you also note that it's important to call this before X509NameToString() and why?
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 22 Oct 2020 21:45:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16631
to look at the new patch set (#3).
Change subject: Fix order of clearing and printing openssl error
......................................................................
Fix order of clearing and printing openssl error
When verifying the certificate chain fails with an error other than
self-signed certificate, we try to get the subject and issuer to print
in the error message. Unfortunately X509NameToString(), the method doing
the conversion, also checks that there are no leftover OpenSSL errors,
so it fails immediately on call. This commit changes the behavior to
clear the errors *before* calling X509NameToString().
I ran into this problem while debugging test failures on a host where
the OpenSSL was provided by CryptoComply SafeLogic:
F1020 12:06:13.327023 25579 openssl_util.h:210] Check failed: ERR_peek_error() == 0 (67567722 vs. 0) Expected no pending OpenSSL errors on std::string kudu::security::X509NameToString(X509_NAME*) entry, but had: error:0407006A:rsa routines:RSA_padding_check_PKCS1_type_1:block type is not 01:rsa_pk1.c:102 error:04067072:rsa routines:RSA_EAY_PUBLIC_DECRYPT:padding check failed:rsa_eay.c:786 error:0D0C5006:asn1 encoding routines:ASN1_item_verify:EVP lib:a_verify.c:218
Unfortunately, I couldn't reproduce it in other OpenSSL versions and
distributions, so I can't add a regression test, at least for now.
Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
---
A src/kudu/security/tls_context-test.cc
M src/kudu/security/tls_context.cc
2 files changed, 58 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16631/3
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16631
to look at the new patch set (#4).
Change subject: Fix order of clearing and printing openssl error
......................................................................
Fix order of clearing and printing openssl error
When verifying the certificate chain fails with an error other than
self-signed certificate, we try to get the subject and issuer to print
in the error message. Unfortunately X509NameToString(), the method doing
the conversion, also checks that there are no leftover OpenSSL errors,
so it fails immediately on call. This commit changes the behavior to
clear the errors *before* calling X509NameToString().
I ran into this problem while debugging test failures on a host where
the OpenSSL was provided by CryptoComply SafeLogic:
F1020 12:06:13.327023 25579 openssl_util.h:210] Check failed: ERR_peek_error() == 0 (67567722 vs. 0) Expected no pending OpenSSL errors on std::string kudu::security::X509NameToString(X509_NAME*) entry, but had: error:0407006A:rsa routines:RSA_padding_check_PKCS1_type_1:block type is not 01:rsa_pk1.c:102 error:04067072:rsa routines:RSA_EAY_PUBLIC_DECRYPT:padding check failed:rsa_eay.c:786 error:0D0C5006:asn1 encoding routines:ASN1_item_verify:EVP lib:a_verify.c:218
Unfortunately, I couldn't reproduce it in other OpenSSL versions and
distributions, so I can't add a regression test, at least for now.
Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
---
A src/kudu/security/tls_context-test.cc
M src/kudu/security/tls_context.cc
2 files changed, 60 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16631/4
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing and printing openssl error
......................................................................
Fix order of clearing and printing openssl error
When verifying the certificate chain fails with an error other than
self-signed certificate, we try to get the subject and issuer to print
in the error message. Unfortunately X509NameToString(), the method doing
the conversion, also checks that there are no leftover OpenSSL errors,
so it fails immediately on call. This commit changes the behavior to
clear the errors *before* calling X509NameToString().
I ran into this problem while debugging test failures on a host where
the OpenSSL was provided by CryptoComply SafeLogic:
F1020 12:06:13.327023 25579 openssl_util.h:210] Check failed: ERR_peek_error() == 0 (67567722 vs. 0) Expected no pending OpenSSL errors on std::string kudu::security::X509NameToString(X509_NAME*) entry, but had: error:0407006A:rsa routines:RSA_padding_check_PKCS1_type_1:block type is not 01:rsa_pk1.c:102 error:04067072:rsa routines:RSA_EAY_PUBLIC_DECRYPT:padding check failed:rsa_eay.c:786 error:0D0C5006:asn1 encoding routines:ASN1_item_verify:EVP lib:a_verify.c:218
Unfortunately, I couldn't reproduce it in other OpenSSL versions and
distributions, so I can't add a regression test, at least for now.
Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Reviewed-on: http://gerrit.cloudera.org:8080/16631
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/security/tls_context.cc
1 file changed, 7 insertions(+), 3 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing and printing openssl error
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 23 Oct 2020 16:42:38 +0000
Gerrit-HasComments: No
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing and printing openssl error
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 26 Oct 2020 13:35:42 +0000
Gerrit-HasComments: No
[kudu-CR] Fix order of clearing openssl error and printing it
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16631
to look at the new patch set (#2).
Change subject: Fix order of clearing openssl error and printing it
......................................................................
Fix order of clearing openssl error and printing it
When verifying the certificate chain fails with an error other than
self-signed certificate, we try to get the subject and issuer to print
in the error message. Unfortunately X509NameToString(), the method doing
the conversion, also checks that there are no leftover OpenSSL errors,
so it fails immediately on call. This commit changes the behavior to
clear the errors *before* calling X509NameToString().
I ran into this problem while debugging test failures on a host where
the OpenSSL was provided by CryptoComply SafeLogic:
F1020 12:06:13.327023 25579 openssl_util.h:210] Check failed: ERR_peek_error() == 0 (67567722 vs. 0) Expected no pending OpenSSL errors on std::string kudu::security::X509NameToString(X509_NAME*) entry, but had: error:0407006A:rsa routines:RSA_padding_check_PKCS1_type_1:block type is not 01:rsa_pk1.c:102 error:04067072:rsa routines:RSA_EAY_PUBLIC_DECRYPT:padding check failed:rsa_eay.c:786 error:0D0C5006:asn1 encoding routines:ASN1_item_verify:EVP lib:a_verify.c:218
Unfortunately, I couldn't reproduce it in other OpenSSL versions and
distributions, so I can't add a regression test, at least for now.
Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
---
A src/kudu/security/tls_context-test.cc
M src/kudu/security/tls_context.cc
2 files changed, 58 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16631/2
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing and printing openssl error
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16631/3/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
http://gerrit.cloudera.org:8080/#/c/16631/3/src/kudu/security/tls_context.cc@249
PS3, Line 249:
> Is this still necessary to add ERR_clear_error() after calling GetOpenSSLEr
Nope, you're right. I wasn't aware that GetOpenSSLErrors() also clears the errors.
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 23 Oct 2020 16:27:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing and printing openssl error
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16631/1//COMMIT_MSG
Commit Message:
PS1:
> +1
I tried to add a test, but I have no good way to trigger this consistently, it seems to have been triggered by a thread safety problem in SafeLogic's CryptoComply Server OpenSSL 1.0.2v-fips. I tried creating a malformed PEM file, but then the error returned on Cert::FromString() instead of TlsContext::VerifyCertChainUnlocked().
http://gerrit.cloudera.org:8080/#/c/16631/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
http://gerrit.cloudera.org:8080/#/c/16631/1/src/kudu/security/tls_context.cc@248
PS1, Line 248: int err = X509_STORE_CTX_get_error(store_ctx.get());
> Maybe, it's also worth outputting the collected error information along wit
Done
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 23 Oct 2020 12:27:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] Fix order of clearing openssl error and printing it
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing openssl error and printing it
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16631/1//COMMIT_MSG
Commit Message:
PS1:
> What was the error we saw here? Is there any way to test this so we don't r
+1
It would be great to add a test scenario which reproduces the issue. You can take a look at available cert-related cases in src/kudu/security/test/test_certs.cc, in particular comments for the CreateTestSSLCertSignedByChain() function. I guess it might be possible to generate a certificate chain that has similar issue.
http://gerrit.cloudera.org:8080/#/c/16631/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
http://gerrit.cloudera.org:8080/#/c/16631/1/src/kudu/security/tls_context.cc@248
PS1, Line 248: ERR_clear_error(); // in case it left anything on the queue.
> nit: Can you also note that it's important to call this before X509NameToSt
Maybe, it's also worth outputting the collected error information along with cert_details when returning RuntimeError at line 256?
It's about calling GetOpenSSLErrors() (which does extracts all errors from the error stack as well) after X509_verify_cert() storing the result in string, and the using the string when forming the RuntimeError message.
int rc = X509_verify_cert(store_ctx.get());
if (rc != 1) {
const auto verify_err_msg = GetOpenSSLErrors();
...
}
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 23 Oct 2020 00:29:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16631
to look at the new patch set (#5).
Change subject: Fix order of clearing and printing openssl error
......................................................................
Fix order of clearing and printing openssl error
When verifying the certificate chain fails with an error other than
self-signed certificate, we try to get the subject and issuer to print
in the error message. Unfortunately X509NameToString(), the method doing
the conversion, also checks that there are no leftover OpenSSL errors,
so it fails immediately on call. This commit changes the behavior to
clear the errors *before* calling X509NameToString().
I ran into this problem while debugging test failures on a host where
the OpenSSL was provided by CryptoComply SafeLogic:
F1020 12:06:13.327023 25579 openssl_util.h:210] Check failed: ERR_peek_error() == 0 (67567722 vs. 0) Expected no pending OpenSSL errors on std::string kudu::security::X509NameToString(X509_NAME*) entry, but had: error:0407006A:rsa routines:RSA_padding_check_PKCS1_type_1:block type is not 01:rsa_pk1.c:102 error:04067072:rsa routines:RSA_EAY_PUBLIC_DECRYPT:padding check failed:rsa_eay.c:786 error:0D0C5006:asn1 encoding routines:ASN1_item_verify:EVP lib:a_verify.c:218
Unfortunately, I couldn't reproduce it in other OpenSSL versions and
distributions, so I can't add a regression test, at least for now.
Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
---
M src/kudu/security/tls_context.cc
1 file changed, 7 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/16631/5
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] Fix order of clearing and printing openssl error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16631 )
Change subject: Fix order of clearing and printing openssl error
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16631/3/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:
http://gerrit.cloudera.org:8080/#/c/16631/3/src/kudu/security/tls_context.cc@249
PS3, Line 249: ERR_clear_error();
Is this still necessary to add ERR_clear_error() after calling GetOpenSSLError() above? I thought X509_verify_cert() put something into the error stack and X509NameToString() hit it with SCOPED_OPENSSL_NO_PENDING_ERRORS. As I can see, X509_STORE_CTX_get_error() is just an accessor to a field, so it wouldn't add anything new in there:
https://github.com/openssl/openssl/blob/d1fb6b481b1d70932a1435f83eae10cc68edbe36/crypto/x509/x509_vfy.c#L2177-L2180
Or X509_STORE_CTX_get_error() is something else in CaseLogic's implementation?
--
To view, visit http://gerrit.cloudera.org:8080/16631
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f78bdedce7a976a6e8117bb8683032dd917c626
Gerrit-Change-Number: 16631
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 23 Oct 2020 15:24:20 +0000
Gerrit-HasComments: Yes