Conversation
The threat model covers confidentiality and integrity in detail but says nothing about availability, even though cert-manager is a single shared, leader-elected component whose exhaustion affects every tenant. Add a section covering the two ways a principal can get there: creating expensive workload, and pointing cert-manager at a server that returns hostile responses. Both reduce to the RBAC advice already given, so link back to it rather than repeating it, and point at the existing scaling and best practice pages for pod resource limits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JWqdR4ZapV8pjr5zJcA466 Signed-off-by: Richard Wall <richard@the-moon.net>
cert-manager-prow
Bot
added
dco-signoff: yes
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
✅ Deploy Preview for cert-manager ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The threat model page is thorough on confidentiality and integrity — reading
Secretresources you should not, forging identities, confused deputy, SSRF — but says nothing at all about availability.That gap matters more than it first appears, because cert-manager is not a per-tenant component. The controller and cainjector are leader-elected, so only one replica is ever active and they cannot be scaled horizontally (as best-practice.md already notes). One overloaded controller delays issuance and renewal for every tenant in the cluster, which makes resource exhaustion a cross-tenant concern rather than a local one. Someone reading the threat model to decide how to hand out RBAC currently gets no signal about that.
This adds an
Availability and Denial of Servicesection underSpecific Risks to Consider, covering the two ways a principal gets there:Certificateresources, or deliberately expensive ones.IssuerandClusterIssuerresources, and those responses are untrusted input even when the request was legitimate.Both reduce to the RBAC advice the page already gives, so the section links back to it rather than restating it. It also cites
GHSA-r4pg-vg54-wxx4(oversized PEM) andGHSA-gx3x-vq4p-mhhv(malformed DNS response) as worked examples of the second class.The closing point is the practical one: the Helm chart ships
resources: {}, so by default the pods runBestEffortand controller memory growth becomes node-level pressure on unrelated workloads. That advice already exists in Scalability and Restrict the use of large RSA keys; this just connects the threat model to it instead of duplicating it.Only
content/docsis touched — the versioned snapshots are left alone.Checks run locally:
cspellclean,markdown-link-checkexit 0,remark --frailexit 0.Possible follow-up, not in this PR: a sweep for unbounded
json.NewDecoder(resp.Body)/io.ReadAll(resp.Body)across the controller, so the second class is bounded by construction rather than found one advisory at a time.🤖 Generated with Claude Code
https://claude.ai/code/session_01JWqdR4ZapV8pjr5zJcA466