[CALCITE-7674] Add Avatica threat model - #313
Conversation
potiuk
left a comment
There was a problem hiding this comment.
Reviewed this against the rubric we use for scanner-facing models. Short version: it passes comfortably, and it's stronger than most first drafts we see — including several we've written ourselves. Every section a scanning agent depends on is present and substantive: scope and the Calcite boundary split, attacker profiles, the P1–P5 properties, always/never-a-vulnerability lists, downstream responsibilities, and triage dispositions.
Three things I'd single out as unusually good, because they're the parts that actually change triage outcomes:
- The Inputs table maps every input to a governing rule and then closes the set — "A report that reaches a sink not covered here is a model gap." That catch-all is what stops the table going stale silently.
Model gapis a disposition with a feedback loop — escalate, rule, then update this document so the next report of its kind is no longer a gap. Most models stop at the escalation.- The DoS section is honest about missing controls rather than overclaiming, and pairs with a
Known limitationdisposition instead of pretending the bound exists.
Five things worth your judgement, roughly in order of value:
1. Does this model cover apache/calcite-avatica-go? (substantive — the one I'd most want answered)
The model is JVM-centric throughout: P1 says "client JVM", the assets list has "The Avatica client's JVM", and the class-loading sections are Class.forName / asSubclass. The Go client speaks the same wire protocol from a completely different codebase, where none of that applies. Right now a Go-client finding lands as Model gap by default. One sentence either way — "this model covers the Java server and JDBC driver; the Go client is not covered" or "…and applies to the Go client except where it names JVM mechanisms" — closes it before the first report rather than after.
2. "Always a vulnerability" #7 vs the DoS section.
Item 7 says single-frame resource exhaustion is a vulnerability; the DoS section says treat those gaps as known limitations not per-report vulnerabilities until controls land. Known limitation in the dispositions reconciles them, but a reporter reading item 7 alone will expect Valid and feel bait-and-switched. A clause on item 7 — "except where Denial of service records the bound as not yet landed" — makes the two agree on the page.
3. SECURITY.md link text points at a file that doesn't exist.
[THREAT_MODEL.md](site/_docs/security_threat_model.md) — the path is right, the label names a non-existent THREAT_MODEL.md. Cosmetic, but the label is what a human follows.
4. The CVE-2020-13955 entry doesn't state its rule.
The section says these are recorded "as ground truth for the rule they establish", and CVE-2022-36364 does exactly that — it grounds the surprising-class-loading rule. The 2020 entry currently reads "a Calcite Avatica issue in an earlier release", which establishes nothing. I've deliberately not guessed what it was; you'll know whether it grounds a rule already in the model or should just be dropped.
5. Status: draft for PMC discussion; not yet ratified — worth flipping when this merges, so the merged document doesn't describe itself as unratified.
None of these block anything. Happy to push any of them as commits on this branch if that's easier than doing it yourself — your call, it's your PR.
|
Thanks a lot for the review @potiuk !
|
|
Re-reviewed at
1 is the one still open, and I'd argue for one more commit rather Suggested wording, closing the opening paragraph:
Happy to push that as a commit on the branch if it's easier — your Otherwise this is good from my side, and agreed on wanting a second |
Sorry, @potiuk , I misunderstood your remark (I thought you just needed a confirmation about the scope of the model). I understand now that it makes sense to mention that explicitly on the model itself. I have just committed your suggestion. Thanks. |
CALCITE-7674 Add Avatica threat model