Skip to content
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

Add mod_ssl-like Placeholder Support for Server and Client Certificates in Caddy #6780

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

Conversation

Ko496-glitch
Copy link

This pull request introduces functionality similar to Apache's mod_ssl by adding support for dynamic placeholders for server and client certificates in Caddy. The implementation addresses the following:

1)Dynamic Placeholder Population:

Populates placeholders for both server and client certificates during TLS connections, enabling detailed certificate information to be accessed in configurations (e.g., headers, logs).

2)Support for Default Values:

Ensures placeholders return meaningful default values (e.g., null or descriptive strings) when client certificates are not provided (e.g., in non-mTLS scenarios).

3)Centralized Logic:

Introduced a new function, handleMTLSEnabledWithExport, to handle placeholder population efficiently, reducing redundancy and improving maintainability.

@CLAassistant
Copy link

CLAassistant commented Jan 9, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Ko496-glitch
❌ keenu7ohlan
You have signed the CLA already but the status is still pending? Let us recheck it.

modules/caddyhttp/replacer.go Outdated Show resolved Hide resolved
Copy link
Author

@Ko496-glitch Ko496-glitch left a comment

Choose a reason for hiding this comment

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

Updated the lowercase placeholder and comments on the change.

@Ko496-glitch
Copy link
Author

@mohammed90 just checking in too see if everything for the PR looks good ?

@mohammed90
Copy link
Member

@mohammed90 just checking in too see if everything for the PR looks good ?

At first glance, it looks fine, but I haven't had the time to review it thoroughly. We'll get to it.

@mohammed90 mohammed90 added the under review 🧐 Review is pending before merging label Jan 13, 2025
@Ko496-glitch
Copy link
Author

#6713

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under review 🧐 Review is pending before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants