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

Bug 5092: improve https_port ssl-bump documentation #1981

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

Conversation

kinkie
Copy link
Contributor

@kinkie kinkie commented Jan 8, 2025

No description provided.

@kinkie kinkie added the backport-to-v6 maintainer has approved these changes for v6 backporting label Jan 8, 2025
@rousskov rousskov self-requested a review January 9, 2025 14:50
Copy link
Contributor

@rousskov rousskov left a comment

Choose a reason for hiding this comment

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

PR title promises to improve https_port documentation. PR code changes http_port documentation instead.

Comment on lines +2307 to 2308
The ssl_bump configuration option is required to fully enable
bumping of CONNECT requests.
Copy link
Contributor

@rousskov rousskov Jan 13, 2025

Choose a reason for hiding this comment

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

This paragraph is still very misleading IMO. I suggest using one of the following complete replacements (shown here without formatting due to cf.data.pre formatting complications):

  • "This option is essentially ignored (with a WARNING) if configuration has no ssl_bump directives."
  • "This option is essentially ignored (with a WARNING) unless configuration has ssl_bump directive(s)."

Comment on lines +2309 to +2310
Enabling this option requires also specifying either
tproxy or intercept
Copy link
Contributor

@rousskov rousskov Jan 13, 2025

Choose a reason for hiding this comment

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

This new statement is incorrect: http_port supports SslBump in forward proxy mode as well. That mode is the default.

Suggested change
Enabling this option requires also specifying either
tproxy or intercept

Accepting the above suggestion is not enough to address Bug 5092 though.

@rousskov rousskov added the S-waiting-for-author author action is expected (and usually required) label Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-v6 maintainer has approved these changes for v6 backporting S-waiting-for-author author action is expected (and usually required)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants