Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Revert "Don't copy CA certificates to client certificate data" #1527

Merged
merged 1 commit into from
Jan 20, 2020

Conversation

eu-siemann
Copy link
Contributor

This reverts commit c6c3543,
which seems to break things when curl is compiled with GnuTLS backend.

This reverts commit c6c3543,
which seems to break things when curl is compiled with GnuTLS backend.

Signed-off-by: Eugene Smirnov <evgenii.smirnov@here.com>
@@ -297,6 +297,7 @@ bool Crypto::parseP12(BIO *p12_bio, const std::string &p12_password, std::string
for (int i = 0; i < sk_X509_num(ca_certs.get()); i++) { // NOLINT
ca_cert = sk_X509_value(ca_certs.get(), i); // NOLINT
PEM_write_bio_X509(ca_sink.get(), ca_cert);
PEM_write_bio_X509(cert_sink.get(), ca_cert);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a comment here to note that it should not be necessary, works with OpenSSL but not gnutls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a quick revert, I'd like to find the root cause before adding any comments. I'll definitely follow up with other PR.

@codecov-io
Copy link

codecov-io commented Jan 17, 2020

Codecov Report

Merging #1527 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1527      +/-   ##
==========================================
+ Coverage   81.91%   81.91%   +<.01%     
==========================================
  Files         186      186              
  Lines       11361    11362       +1     
==========================================
+ Hits         9306     9307       +1     
  Misses       2055     2055
Impacted Files Coverage Δ
src/libaktualizr/crypto/crypto.cc 86.11% <100%> (+0.05%) ⬆️
src/libaktualizr/package_manager/ostreemanager.cc 79.77% <0%> (-0.38%) ⬇️
src/libaktualizr/storage/sqlstorage.cc 77.05% <0%> (-0.22%) ⬇️
src/libaktualizr/storage/sqlstorage_base.cc 78.37% <0%> (+2.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 14f7c6e...6786e5f. Read the comment docs.

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I too would love to know more, but for now, let's merge this and I'll get going on the release.

@pattivacek pattivacek merged commit ebddc2e into master Jan 20, 2020
@pattivacek pattivacek deleted the fix/cert branch January 20, 2020 07:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants