Skip to content

Load all certs in a CaBuffer #1224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Load all certs in a CaBuffer #1224

wants to merge 5 commits into from

Conversation

afjoseph
Copy link

@afjoseph afjoseph commented May 23, 2025

Hey! Thanks for the great project.

I noticed that cpr::CaBuffer doesn't load all certificates in the buffer: just the first one. PEM_read_bio_X509 is meant to run in a loop (some examples) to add all certs in the buffer. This PR covers this.

It also fixes the inclusion of openssl/pemerr.h in BoringSSL, which doesn't exist. This closes #333 (comment)

There's also an unused error printer that I removed, which caused issues with -Werror

Cheers!

Copy link
Member

@COM8 COM8 left a comment

Choose a reason for hiding this comment

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

@afjoseph thanks for contributing and fixing this.
Only auto needs to be replaced and a few clang-tidy things.

@COM8 COM8 added the Bug 🐛 label May 26, 2025
@COM8 COM8 added this to the CPR 1.11.x milestone May 26, 2025
@afjoseph
Copy link
Author

@COM8 Changes modified accordingly

cpr/ssl_ctx.cpp Outdated
// Create a memory BIO using the data of cert_buf
// Note: It is assumed, that cert_buf is nul terminated and its length is determined by strlen
char* cert_buf = static_cast<char*>(raw_cert_buf);
bio_ptr bio = BIO_new_mem_buf(cert_buf, -1);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bio_ptr bio = BIO_new_mem_buf(cert_buf, -1);
BIO * bio = BIO_new_mem_buf(cert_buf, -1);

I guess here I suggested the wrong type. The CI fails.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NDK and CPR
2 participants