docs: add trust consequences and disposition rules to THREAT_MODEL.md - #22236
Conversation
|
Hi @PushkarJ. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
6bb9fa6 to
92dc0c2
Compare
|
It looks really good!!! It should be able to reduce etcd SRC's burden. Thanks @PushkarJ |
|
/ok-to-test |
|
+1 to what @ahrtr, this is really good. Not sure if we need review from someone more familiar with security, but I found the contents really good (not a security expert). |
| Everything else — robustness defects behind trusted boundaries, hardening of best-effort components, build tooling, and non-default features — is welcome through normal issues and pull requests. | ||
| The project routinely accepts and backports such fixes; "not a vulnerability" never means "do not report". |
There was a problem hiding this comment.
"not a vulnerability" never means "do not report"
Good!
FYI. @PushkarJ is a security expert. He is an active contributor in sig-security, I recall he is approvers of several sub-projects under sig-security. I think this is mostly up to sig-etcd leads and etcd SRC. We also clarified that |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted filessee 24 files with indirect coverage changes @@ Coverage Diff @@
## main #22236 +/- ##
==========================================
- Coverage 69.75% 69.73% -0.03%
==========================================
Files 449 449
Lines 38207 38207
==========================================
- Hits 26652 26643 -9
- Misses 10127 10134 +7
- Partials 1428 1430 +2 Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
@PushkarJ please mark this PR ready to review when you feel comfortable. Thanks. |
b397bb4 to
9fd301d
Compare
The threat model defines trust boundaries but not the dispositions that follow from them. As a result, automated scanners and security researchers repeatedly report findings that maintainers must rebut by hand-deriving the same conclusions: malformed-input crashes behind authenticated peer/client transport are robustness defects, grpc-proxy and contrib are best-effort components, CI and release scripts are not product attack surface, diagnostic endpoints are non-default operator opt-ins, and watch permission revocation is eventually consistent by design. Make these dispositions explicit so reports can be evaluated against the document directly: * State that traffic which has passed client or peer mTLS is trusted input, and what a report against those boundaries must demonstrate. * Add boundaries for authentication/authorization semantics and for build, release, and CI tooling. * Add sections scoping best-effort components and non-default or test-only configuration. * Add a reporting section clarifying that robustness and hardening findings remain welcome as normal issues and pull requests. AI assisted: drafting the document restructure, commit message, and PR description Signed-off-by: "Pushkar Joglekar 3390906+PushkarJ@users.noreply.github.com" Signed-off-by: Pushkar Joglekar <pushkar.joglekar@broadcom.com>
| Except for `/debug/vars` and `--enable-pprof`, these facilities are **disabled by default**. Enabling them is an explicit operator decision that knowingly expands the attack surface. | ||
|
|
||
| The `/debug/vars` endpoint is protected by mTLS like other client APIs, and does not expose sensitive information. The `--enable-pprof` flag is also protected by mTLS and is expected to be used in trusted network. |
There was a problem hiding this comment.
This isn't accurate. --enable-pprof is disabled by default. Only /debug/vars is an exception (enabled by default)
Please follow #22236 (comment)
There was a problem hiding this comment.
Apologies. I misunderstood the earlier comment thread: #22236 (comment)
There was a problem hiding this comment.
Copy pasted the suggestion from here to resolve it: #22236 (comment)
Co-authored-by: Benjamin Wang <benjamin.wang@broadcom.com> Signed-off-by: Pushkar Joglekar <pushkar.joglekar@broadcom.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahrtr, PushkarJ, serathius The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
4 participants
What this PR does
Extends
THREAT_MODEL.mdwith explicit trust consequences and finding-dispositionrules, while keeping the existing boundary structure and assumptions unchanged:
trusted input, and that a vulnerability report must show unauthenticated
reachability or privilege escalation — crashes and resource exhaustion from
authenticated senders are robustness defects.
optional secondary control; watch permission revocation is eventually
consistent by design) and for build/release/CI tooling (
scripts/,tools/,hack/,.github/are not product attack surface).grpc-proxy, gateway,cache,contrib/) and non-default or test-only configuration (pprof,expvar, debug logging, tracing,
unix(s)://listeners).with an explicit note that robustness and hardening findings remain welcome
through normal issues and PRs — "not a vulnerability" never means
"do not report".
Why
Codifying the disposition rules lets researchers and automated tooling self-filter these
classes before reporting, while steering real robustness fixes to the normal
contribution flow instead of the security-disclosure process.
No behavioral or code changes; documentation only.
AI assisted: drafting the document restructure, commit message, and PR description