feat(policy): match undisclosed issues by title marker and restriction - #13
Merged
Conversation
A security issue that is not yet public is recognisable two ways, and a
policy could express neither: its title carries a marker the security team
removes at disclosure, and it is readable only through a Bugzilla group.
Add `summary_contains`, a case-insensitive substring search over the bug's
one-line summary, and `group_restricted`, which asks whether the bug is
readable only via at least one group. The latter deliberately does not name
groups: group vocabularies differ per instance and a rule keyed on names
breaks when one is renamed. Either signal alone suffices, so a bug that
loses one is still covered by the other. Both are deny rules in the shipped
example rather than restrict-with-summary, because for such a bug the title
is itself the sensitive part.
Make unreadable metadata undecidable rather than empty. `BugMeta` fields
become `Option`, where `None` means the bug object did not carry the field —
absent, null, wrongly typed, or a list with an element the parser could not
read. `Matcher::evaluate` returns `Yes`/`No`/`Unknown` in place of a bool,
and `classify` denies on `Unknown` for every action. A deny rule denies
because the bug may be what it looks for; a granting rule may neither grant
on unchecked data nor be skipped, since skipping would fall through to a
later rule or to an allowing `default_action`.
This closes three fail-open paths that a bool could not express:
* a `restrict` rule under `default_action = "deny"` is a grant, so
"unknown matches" handed out capabilities on unreadable metadata;
* a granting rule matching only on unknown data short-circuited the deny
rule that would have caught the bug had the field been readable;
* a group array whose elements were unreadable collapsed to an empty list,
i.e. to "world-readable" — the most permissive answer available.
A definitive `No` still wins over `Unknown`, so a rule another criterion
ruled out stays ruled out, and only the fields a rule actually consults can
make it undecidable.
The example policy gains the two rules, drops a group-based freshness rule
that first-match-wins had made unreachable, and states the unreadable-data
behaviour once instead of per criterion.
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
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.
What
Two new
matchcriteria, so a policy can recognise a security issue that isnot yet public:
summary_contains— case-insensitive substring search over the bug'sone-line summary. Security teams mark such issues in the title and remove
the marker at disclosure.
group_restricted—truematches a bug readable only through atleast one Bugzilla group,
falsea world-readable one. It names no groupson purpose: vocabularies differ per instance and a rule keyed on names
breaks the day one is renamed.
Either signal alone is enough, so a bug that loses one stays covered by the
other.
examples/policy.tomluses both asdenyrules rather thanrestrictwith asummarycapability — for an undisclosed issue the titleis itself the sensitive part, and a summary view would hand over exactly the
line that must not leak.
Fail-closed model
Implementing this surfaced that "field missing" was being treated as "field
empty".
BugMetafields are nowOption, whereNonemeans the bug objectdid not carry the field (absent, null, wrongly typed, or a list with an
element the parser could not read).
Matcher::evaluatereturnsYes/No/Unknowninstead of a bool, andclassifydenies onUnknownfor every action: a deny rule denies because the bug may be what it looks
for, and a granting rule may neither grant on unchecked data nor be skipped,
because skipping would fall through to a later rule or to an allowing
default_action.That closes three fail-open paths a bool could not express:
restrictrule underdefault_action = "deny"is a grant, so"unknown matches" handed out capabilities on unreadable metadata;
rule that would have caught the bug had the field been readable;
groupsarray with unreadable elements collapsed to an empty list —i.e. to "world-readable", the most permissive answer available.
A definitive
Nostill beatsUnknown, so a rule another criterion ruledout stays ruled out, and only the fields a rule actually consults can make it
undecidable.
""and[]are knowledge, not ignorance.Verification
Unknownon its own unreadable field, and classify-level tests for therestrict-under-deny-default and short-circuit cases above.
freshness window is denied by id and withheld from search (a query
returning two bugs upstream returns none through bugwarden), a disclosed
17-day-old security bug is served in full, and 19 of 20 ordinary bugs are
unaffected — the rules are not over-blocking.
examples/policy.tomlloads; a deliberately typo'd key is still rejectedat startup.
Notes
docs/DESIGN.mdI4 is widened to cover any undecidable rule, and itsMatcher/BugMetadeclarations are brought back in sync.table implied otherwise; both now describe the same behaviour.
fresh-security-group: with agroup_restricted = truerule ahead of it, first-match-wins made it unreachable.