Skip to content

Conversation

@ZPascal
Copy link
Contributor

@ZPascal ZPascal commented Aug 18, 2025

PR Summary: Enable “.” in path validation to support IETF well-known ACME paths

Summary

This PR updates path validation to allow the dot character, enabling standardized well-known endpoints such as /.well-known/acme-challenge/. It also adds tests that:

  • Accept ACME HTTP-01 challenge paths under /.well-known/acme-challenge/ for both Prefix and Exact path types.
  • Explicitly reject paths containing $, #, and newline characters.

Motivation

  • IETF standards reserve /.well-known/ for well-known resources (RFC 5785), which ACME relies on for HTTP-01 challenges.
  • ACME’s HTTP-01 challenge requires serving under /.well-known/acme-challenge/... (RFC 8555). Prior validation disallowed the dot, preventing compliant paths.
  • We continue to disallow characters that are risky or ambiguous in reverse-proxy and config contexts.

What Changed

  • Validation now permits “.” in the allowed character set for path segments, in addition to alphanumeric, _, -, and /.
  • Tests:
    • Added valid cases for /.well-known/acme-challenge/ with both Prefix and Exact types.
    • Added invalid cases for paths containing $, #, and newline.

IETF RFC Alignment

  • RFC 5785 & RFC 8615 (Well-Known URIs): This PR enables the /.well-known/... namespace, allowing standardized discovery endpoints and ACME challenges.
  • RFC 8555 (ACME): Allows the HTTP-01 challenge path under /.well-known/acme-challenge/..., aligning with the ACME challenge location requirements.
  • RFC 3986 (URI Syntax): Partially aligns with the unreserved character set in path segments by including “.”. The implementation remains intentionally conservative (see below).

Missing RFC Features and Intentional Restrictions

While this change improves standards alignment, it does not implement the full breadth of URI semantics defined by the IETF. Notable gaps and deliberate restrictions include:

  • Full RFC 3986 path character set not supported:
    • Still disallows many reserved/sub-delimiter characters allowed by RFC 3986 path segments (e.g., ! $ & ' ( ) * + , ; = : @). Some of these are intentionally rejected to avoid misconfiguration or ambiguity.
    • Percent-encoding is not interpreted; only raw characters from the permitted subset are allowed.
  • Fragment delimiter “#” is outright rejected (appropriate in practice, as fragments are client-side only, but stricter than generic RFC parsing behavior).
  • Case sensitivity: The validator is case-insensitive, whereas RFC 3986 treats path segments as case-sensitive (only scheme and host are case-insensitive).
  • RFC 5785 specificities:
    • The registry mandates the literal lowercase segment “.well-known”. The current case-insensitive matching does not enforce lowercase.
  • RFC 8555 (ACME) specifics not enforced:
    • Token formatting: ACME expects a token segment with base64url characters (no padding), without additional path separators.
    • Trailing slash behavior and exact token file semantics are not validated; the validator only checks character classes, not per-endpoint structure.
  • Normalization and dot-segment handling (e.g., . and ..) are not interpreted or canonicalized; they are just allowed as literal characters within the approved set.

These limitations are intentional to keep validation simple, predictable, and safe for this controller’s context. Further RFC compliance can be considered if needed, preferably with targeted, endpoint-specific validation (e.g., stricter checks only for /.well-known/acme-challenge).

Security and Compatibility

  • Security: Continues to reject potentially hazardous characters ($, #, newline) that can cause parsing ambiguities or unsafe interpretations in downstream components.
  • Backward compatibility: Existing valid paths remain valid. This loosens validation to include “.”, enabling well-known use cases without breaking prior configurations.

Testing

  • Adds positive tests for well-known ACME challenge paths for both Prefix and Exact types.
  • Adds negative tests for paths containing $, #, and newline characters to ensure they remain disallowed.

Documentation

  • Consider updating user-facing docs to clarify:
    • The allowed path character subset.
    • Support for /.well-known/acme-challenge/ and common ACME integrations.
    • Known deviations from RFC semantics (case-insensitivity, restricted character set), and rationale.

What this PR does / why we need it:

The PR adds support for the IETF RFCs 5785, 8555, and 3986. It also solves #11176.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • CVE Report (Scanner found CVE and adding report)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation only

Which issue/s this PR fixes

fixes #11176

How Has This Been Tested?

I've executed the corresponding unit tests.

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have added unit and/or e2e tests to cover my changes.
  • All new and existing tests passed.

@netlify
Copy link

netlify bot commented Aug 18, 2025

Deploy Preview for kubernetes-ingress-nginx canceled.

Name Link
🔨 Latest commit 2799837
🔍 Latest deploy log https://app.netlify.com/projects/kubernetes-ingress-nginx/deploys/68a3a94cefd5bf0008d28085

@k8s-ci-robot k8s-ci-robot requested review from Gacko and cpanato August 18, 2025 22:13
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Aug 18, 2025
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 18, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

@k8s-ci-robot
Copy link
Contributor

Welcome @ZPascal!

It looks like this is your first PR to kubernetes/ingress-nginx 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/ingress-nginx has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority labels Aug 18, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @ZPascal. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 18, 2025
@ZPascal ZPascal force-pushed the add-dot-to-the-valid-path-type branch from 5ff3e06 to 7e04818 Compare August 18, 2025 22:15
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 18, 2025
Copy link
Member

@Gacko Gacko left a comment

Choose a reason for hiding this comment

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

/retitle Ingresses: Allow . in Exact and Prefix paths.
/triage accepted
/kind bug
/priority backlog
/lgtm

@k8s-ci-robot k8s-ci-robot changed the title Enable “.” in path validation to support IETF well-known ACME paths Ingresses: Allow . in Exact and Prefix paths. Aug 19, 2025
@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 19, 2025
@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-priority labels Aug 19, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Gacko, ZPascal

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 19, 2025
@Gacko
Copy link
Member

Gacko commented Aug 19, 2025

/cherry-pick release-1.13

@Gacko
Copy link
Member

Gacko commented Aug 19, 2025

/cherry-pick release-1.12

@k8s-infra-cherrypick-robot
Copy link
Contributor

@Gacko: once the present PR merges, I will cherry-pick it on top of release-1.13 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-infra-cherrypick-robot
Copy link
Contributor

@Gacko: once the present PR merges, I will cherry-pick it on top of release-1.12 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot merged commit 618aae1 into kubernetes:main Aug 19, 2025
27 checks passed
@k8s-infra-cherrypick-robot
Copy link
Contributor

@Gacko: new pull request created: #13799

In response to this:

/cherry-pick release-1.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-infra-cherrypick-robot
Copy link
Contributor

@Gacko: new pull request created: #13800

In response to this:

/cherry-pick release-1.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@ZPascal ZPascal deleted the add-dot-to-the-valid-path-type branch August 19, 2025 05:39
@ZPascal
Copy link
Contributor Author

ZPascal commented Aug 19, 2025

Thank you @Gacko, for the fast review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. priority/backlog Higher priority than priority/awaiting-more-evidence. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

strict-validate-path-type does not allow period/dot/. in Exact or Prefix path

4 participants