Skip to content

[CALCITE-7605] Add threat model#5115

Open
vlsi wants to merge 2 commits into
apache:mainfrom
vlsi:CALCITE-7605-threat-model
Open

[CALCITE-7605] Add threat model#5115
vlsi wants to merge 2 commits into
apache:mainfrom
vlsi:CALCITE-7605-threat-model

Conversation

@vlsi

@vlsi vlsi commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Jira Link

[CALCITE-7605] Add threat model

Why

Calcite has no written statement of what it treats as a security vulnerability, so reporters and committers triage each report from scratch. This PR adds one: an attacker model, the properties Calcite promises and disclaims (P1–P4), and a triage rule for the class-loading sinks behind most of the recent CVEs.

This builds directly on the draft in #5020 by @rubenada, which in turn started from an earlier draft of mine. The normative content here (P1–P4, "always / not a vulnerability", the surprising-vs-unsurprising class-loading rule, the DoS goals) is the same material, reorganized. Credit for getting this moving goes to that PR.

Why a separate PR instead of comments on #5020

The changes I'm proposing are structural — drop two sections, split one out into its own file, restore some content — and that reads much better as a diff you can view whole than as a long review thread reconstructing the same edits in prose. Treat this as an alternative draft for the same Jira, not a competing effort: the PMC can merge whichever framing it prefers, or cherry-pick between them. I'm happy to fold this into #5020 instead if that's easier for the reviewers.

What

  • site/_docs/security_threat_model.md — the normative model: attacker and trust boundary, assets, P1–P4, "always / not a vulnerability", surprising-vs-unsurprising class loading with the triage rule, and denial of service as a hardening goal. Stable content; it should not need editing per CVE.
  • site/_docs/security_known_class_loading_sinks.md — a living, non-normative inventory of the concrete sinks the triage rule currently applies to (AvaticaUtils.instantiatePlugin, RelJson.typeNameToClass, JdbcSchema.create, the ClassNameFilter denylist, paths to audit), carrying a Last reviewed date. Keeping the class names out of the policy document means a new CVE updates this page, not the ratified model.
  • site/_data/docs.yml — a "Security" menu group so both pages appear in the sidebar.
  • SECURITY.md and AGENTS.md are carried over from [CALCITE-7605] Add threat model #5020; they add the private-reporting channel and are worth keeping regardless of which draft wins.

What I dropped from #5020, and why

  • The STRIDE table. STRIDE is a discovery technique for the authoring phase, not a publication format. Run against P1–P4 it surfaces no in-scope threat the properties don't already cover more precisely, and it forces near-empty rows for categories Calcite has no surface for (Spoofing and Repudiation both need identities and sessions Calcite doesn't have). One row also mischaracterized the mechanism: it described Janino "arbitrary code execution" as attacker SQL injected into bytecode generation, but Janino compiles Calcite-generated code, and the real primitive — loading a class by name — is already covered by P1 and the class-loading section. If the PMC wants a STRIDE-style completeness check, a six-line "STRIDE category → P1–P4 / out of scope" mapping does that without a low-fidelity threat table.
  • The ASCII data-flow diagram. It restates the trust boundary the prose already defines, adds a maintenance burden, and doesn't survive screen readers or narrow viewports. The "Attacker and trust boundary" and "Assets" sections carry the same information in text.

How to verify

  • cd site && bundle exec jekyll build (or the project's usual site build) renders both pages, and the "Security" group appears in the sidebar.
  • Read the two pages: every normative claim from [CALCITE-7605] Add threat model #5020 is present, the concrete sink list has moved to the living document, and the threat-model page links to it from the triage-rule section.

🤖 Generated with Claude Code

Document what Calcite treats as a security vulnerability: the attacker
model, the properties it provides and disclaims (P1-P4), the
surprising-vs-unsurprising class-loading rule, and denial of service as
a hardening goal. Add SECURITY.md with the private-reporting channel.

Keep the concrete class-loading sinks in a separate living document so a
new CVE updates that inventory, not the ratified model.

This is an alternative draft for the same Jira as apache#5020, reorganizing the
same normative content into a structure that is easier to read and to
maintain per-CVE.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

as sinks are found and fixed; a sink dropping off this list never narrows the
rule. Add a `Last reviewed` date whenever you touch an entry.

**Last reviewed:** June 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could be updated to "July 2026" before final merge.

… Triage dispositions

Fold in the three additive sections proposed in #3, corrected
against the code:

- Inputs: map every attacker-controlled input to its governing rule. Add the
  metaTableFactory/metaColumnFactory PLUGIN properties, place operator under the
  RelJson row (SqlOperator loads only via RelJson), and note that fun selects
  built-in libraries rather than loading a class.
- Downstream responsibilities: collect the host/operator duties in one place and
  drop the now-duplicated transport paragraph from the attacker section.
- Triage dispositions: a closed outcome set a human or agent can route against.
- Not a vulnerability: state that pushed-down SQL is by design, while a pushdown
  bug that reads beyond the configured schemas is P4.

Also record the ModelHandler.addFunctions raw Class.forName sink in the living
inventory.

Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vlsi
vlsi force-pushed the CALCITE-7605-threat-model branch from 9f00475 to 57a0bd9 Compare July 21, 2026 15:25
@rubenada

Copy link
Copy Markdown
Contributor

Thanks for moving this forward @vlsi , IMO looks good.
What would be the next step? Do we need a vote within PMC, or just a normal review + merge process here?

@vlsi

vlsi commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Regular flow could work. As this is a documentation page on a website, a brief review of a native speaker would be great as well.

@zabetak zabetak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The thread model LGTM, I am find merging it as is and fine-tune later once new vulnerabilities are reported.
On the other hand I have some reservations for security_known_class_loading_sinks.md thus I would leave it aside for now.

Comment on lines +1 to +5
---
layout: docs
title: Known class-loading sinks
permalink: /docs/security_known_class_loading_sinks.html
---

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would remove this section and the references to it. I don't think we should list publicly security sinks that we are in the process of fixing.

contradicts this model is a feature request or a documentation gap, not a
vulnerability.

**Status:** draft for PMC discussion; not yet ratified.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since we are about to merge this PR the Draft status should be removed. Anyways the model will necessarily evolve as more vulnerabilities are reported in the future.

@zabetak

zabetak commented Jul 23, 2026

Copy link
Copy Markdown
Member

Process wise I don't think we need a vote. It's nothing more than a documentation improvement that can be enhanced incrementally if needed.

There are already 3 PMC that reviewed the documentation here so we can conclude this rather fast if we agree among us. Feedback from others is of course much appreciated but if this lands before then we can review the suggestions in future PRs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants