Skip to content

examples: improve OpenSSL certificate examples#20807

Closed
vszakats wants to merge 8 commits intocurl:masterfrom
vszakats:certexample
Closed

examples: improve OpenSSL certificate examples#20807
vszakats wants to merge 8 commits intocurl:masterfrom
vszakats:certexample

Conversation

@vszakats
Copy link
Copy Markdown
Member

@vszakats vszakats commented Mar 3, 2026

  • add/fix/synchronize error messages and comments.
  • consistently return error from the callback on failure.
  • fix potential leaks on OpenSSL API failures.
  • fix to not pass the nul-terminator to BIO read.
  • scope a variable.
  • sync code/formatting between the two examples.

https://github.com/curl/curl/pull/20807/files?w=1

- fix/syncronize/add error messages.
- return error from the callback as necessary.
- scope a variable.
- sync code/formatting between the two examples.
@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Mar 3, 2026

@aisle-analyzer augment review

@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot bot commented Mar 3, 2026

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

# Severity Title
1 🔵 Low Silent client-certificate authentication downgrade in OpenSSL SSL_CTX callback example (errors ignored)

See details in the comment below.


Analyzed PR: #20807 at commit 707b0ef

Last updated on: 2026-03-03T22:54:47Z

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes minor improvements to two OpenSSL in-memory certificate example files (cacertinmem.c and usercertinmem.c). The changes aim to improve error messages (naming the failing function directly), propagate errors from the SSL context callback to the caller, scope variables more narrowly, and synchronize the code style between the two examples.

Changes:

  • Error messages are updated to name the specific OpenSSL/curl function that failed (e.g., BIO_new_mem_buf() failed instead of generic messages).
  • A result variable is introduced in both files so the callback can return an error code instead of always returning CURLE_OK.
  • mypem/mykey string literals are moved to the top of the function and marked static, and variable declarations are reordered for consistency.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
docs/examples/usercertinmem.c Updates error messages, adds result variable, marks mypem/mykey as static, reorders declarations
docs/examples/cacertinmem.c Updates error messages, adds result variable, restructures error handling, changes mypem from array to pointer, scopes loop variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Mar 3, 2026

🤖 Augment PR Summary

Summary: Refines the OpenSSL “certificate/key in memory” examples to behave more consistently on errors and to reduce leak/cleanup risks.

Changes:

  • Refactors both sslctx_function() callbacks to use a single result return path with goto out cleanup.
  • Standardizes error messages to name the failing OpenSSL API call and adds missing early-abort checks.
  • Stores PEM blobs as static const char[] and passes explicit buffer lengths (sizeof(...) - 1) to BIO_new_mem_buf().
  • Ensures OpenSSL allocations (BIO/X509/EVP_PKEY/STACK) are consistently freed across all paths.

Technical Notes: Callbacks now return CURLE_ABORTED_BY_CALLBACK on setup failures, matching the PR’s stated intent.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 4 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

@vszakats vszakats changed the title examples: minor improvements in OpenSSL cert examples examples: improve OpenSSL certificate examples Mar 3, 2026
@testclutch

This comment was marked as outdated.

@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Mar 3, 2026

@aisle-analyzer augment review

@aisle-research-bot
Copy link
Copy Markdown

aisle-research-bot bot commented Mar 3, 2026

🔒 Aisle Security Analysis

✅ We scanned this PR and did not find any security vulnerabilities.

Aisle supplements but does not replace security review.


Analyzed PR: #20807 at commit 3b75349

Last updated on: 2026-03-03T23:29:37Z

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

@vszakats vszakats requested a review from Copilot March 3, 2026 23:37
@vszakats
Copy link
Copy Markdown
Member Author

vszakats commented Mar 3, 2026

augment review

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@vszakats vszakats closed this in 38ee353 Mar 4, 2026
@vszakats vszakats deleted the certexample branch March 4, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants