Skip to content

Commit d1bd14a

Browse files
committed
feat(stack-encrypt)!: let an inverted Between range through, as SQL does
A `Between` whose low bound is above its high bound was refused with `TermError::InvertedRange` before either term was derived. SQL's own `BETWEEN` refuses nothing: `x BETWEEN 20 AND 10` is a valid query that matches no row, and an encrypted range now behaves the same way. The empty result leaks nothing and corrupts nothing, so a refusal bought little. It would also cost more than it buys once text and floats are ordered. Judging their ranges correctly means reproducing, before encryption, the order the index compares in: text by its fold, floats by their canonical encoding. That order belongs to the ORE crates, not to this one. So `TermError::InvertedRange` (code `stack_encrypt::inverted_range`) goes, with the typed path's `lo > hi` check (and the `PartialOrd` bound `op::Ordered` carried for it) and the data path's `integer_order`. The variant is under [Unreleased] and never shipped.
1 parent d90a172 commit d1bd14a

12 files changed

Lines changed: 100 additions & 202 deletions

File tree

‎languages/golang/encrypt/guest/src/abi.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -465,8 +465,9 @@ pub unsafe extern "C" fn se_plan_check(plan_ptr: *const u8, plan_len: u32) -> u6
465465
/// term's codec-encoded object out. An operator the field does not answer
466466
/// is `STATUS_ENCODING` naming the field, as a mistyped value is, before
467467
/// the cipher is consulted; the refusals the typed path makes before a term
468-
/// exists (a range the wrong way round, match text that cuts no token or
469-
/// does not normalize) are `STATUS_TERM`, also before the cipher. The input
468+
/// exists (match text that cuts no token or does not normalize) are
469+
/// `STATUS_TERM`, also before the cipher. A range the wrong way round is
470+
/// not refused: it matches no row, as SQL's `BETWEEN` does. The input
470471
/// buffer is wiped in place.
471472
///
472473
/// # Safety

‎languages/golang/encrypt/guest/tests/native_ops.rs‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,13 +1556,6 @@ fn query_term_validation_refuses_what_the_query_op_refuses() {
15561556
"between",
15571557
STATUS_ENCODING,
15581558
),
1559-
(
1560-
"inverted range",
1561-
Tree::Array(vec![Tree::UInt32(65), Tree::UInt32(18)]),
1562-
"age",
1563-
"between",
1564-
STATUS_TERM,
1565-
),
15661559
("no token", s("al"), "name", "matches", STATUS_TERM),
15671560
] {
15681561
let input = encode(input);

‎languages/golang/encrypt/query.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,9 @@ func (l Limit) FetchOr(page uint64) uint64 {
108108
// that answers the operator, and derives that index's term under the
109109
// field's context; for a field that names an EQL type it runs the type's
110110
// query. An operator the field does not answer, or one the engine does not
111-
// know, is [ErrEncoding] naming the field, as a mistyped value is; a range
112-
// the wrong way round, and match text that cuts no token, are [ErrTerm].
111+
// know, is [ErrEncoding] naming the field, as a mistyped value is; match
112+
// text that cuts no token is [ErrTerm]. A range the wrong way round is not
113+
// refused: it matches no row, as SQL's BETWEEN does.
113114
// No data key is minted. For generated code.
114115
func (cph *Cipher) QueryTerm(ctx context.Context, plan *record.Plan, field, operator string, input any) (QueryTerm, error) {
115116
p, err := cph.queryPlan(plan, field)

‎languages/golang/encrypt/query_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,8 +317,9 @@ func TestAQueryTheFieldDoesNotAnswerIsRefused(t *testing.T) {
317317
t.Fatalf("%s on %s: %v, want field %q and reason %q", tc.operator, tc.field, err, tc.field, tc.reason)
318318
}
319319
}
320-
if _, err := cipher.QueryTerm(ctx, queryPlan, "age", "between", []any{uint32(65), uint32(18)}); !errors.Is(err, encrypt.ErrTerm) {
321-
t.Fatalf("an inverted range: %v, want ErrTerm", err)
320+
// An inverted range is not refused, as SQL's BETWEEN is not.
321+
if _, err := cipher.QueryTerm(ctx, queryPlan, "age", "between", []any{uint32(65), uint32(18)}); err != nil {
322+
t.Fatalf("an inverted range: %v", err)
322323
}
323324
if _, err := cipher.QueryTerm(ctx, queryPlan, "name", "matches", "al"); !errors.Is(err, encrypt.ErrTerm) {
324325
t.Fatalf("text that cuts no token: %v, want ErrTerm", err)
@@ -499,9 +500,9 @@ func TestGeneratedQueriesAreTheRustFixtures(t *testing.T) {
499500
t.Errorf("%s.Value() = %T %v", label, got, err)
500501
}
501502
}
502-
// An inverted range is the typed path's refusal, from the engine.
503-
if _, err := fields.Age.Between(ctx, cipher, 65, 18); !errors.Is(err, encrypt.ErrTerm) {
504-
t.Errorf("an inverted range: %v, want ErrTerm", err)
503+
// An inverted range is not refused, as SQL's BETWEEN is not.
504+
if _, err := fields.Age.Between(ctx, cipher, 65, 18); err != nil {
505+
t.Errorf("an inverted range: %v", err)
505506
}
506507
}
507508

‎packages/stack-encrypt/CHANGELOG.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
110110
for the value (`OreTerm`, `OpeTerm`); a range's is one `sem::RangeQuery`
111111
carrying both bounds' terms, read with `lo()` and `hi()`, so no statement
112112
can pair the low bound of one query with the high bound of another. A
113-
range whose low bound is above its high bound is refused when the query
114-
is awaited, before either term is derived (`TermError::InvertedRange`).
113+
range whose low bound is above its high bound is not refused: as SQL's
114+
`BETWEEN`, it matches nothing.
115115
What a query takes is the operator's `op::Arity` (`op::One` or
116116
`op::Pair`), and `op::Input` relates the value given to the plaintext the
117117
plan is checked against (ADR-0009, decision 7).

‎packages/stack-encrypt/docs/adr/0007-bindings-enter-through-a-plan-never-a-second-executor.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,9 +228,10 @@ orderings through an ORE index over an integer, or OPE when the field
228228
declares no ORE. A field that does not answer the operator is refused with
229229
the same shape of error as a mistyped field: an input error naming the field,
230230
with the reason `operator_not_answered` (or `unknown_operator`,
231-
`not_a_range`). The refusals the typed path makes before a term exists, a
232-
range the wrong way round and match text that cuts no token, are refused the
233-
same way here, before the cipher is consulted.
231+
`not_a_range`). The refusal the typed path makes before a term exists,
232+
match text that cuts no token, is made the same way here, before the cipher
233+
is consulted. A range the wrong way round is not refused on either path: it
234+
matches no row, as SQL's `BETWEEN` does.
234235

235236
**The term comes back with its query domain.** The guest's
236237
`se_query_term(input, plan, field, operator, options)` returns one object:

‎packages/stack-encrypt/docs/adr/0009-operators-select-the-index-and-a-query-term-is-not-the-stored-term.md‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,8 @@ and `Matches` take the plaintext the index is declared over
178178
so the plan is checked against `S` either way, and the chain derives one
179179
term per plaintext under the one label. `check` is where a query is refused
180180
before any term exists: a match index whose declared case disagrees with its
181-
`downcase` option, a `Between` whose bounds are the wrong way round, a match
182-
query with no tokens. `AtPath<Op>` takes a path and `Op`'s input, and gains
181+
`downcase` option, a match query with no tokens. A `Between` whose bounds
182+
are the wrong way round is not refused: it matches no row, as SQL's does. `AtPath<Op>` takes a path and `Op`'s input, and gains
183183
its shape with the JSON index.
184184

185185
The bound tightens per operator, through `TermFor<Op>`: any `QueryTerm`
@@ -572,9 +572,12 @@ the ranked loop is worth packaging; it is deferred, not refused.
572572

573573
`cipher.query(op::Between, &(lo, hi)).on(&plan)` produces one term,
574574
`sem::RangeQuery`, carrying the index's term for each bound (the ORE left
575-
ciphertexts, once block ORE lands), inclusive at both ends, and the client
576-
refuses `lo > hi` (`TermError::InvertedRange`) before either term is
577-
derived; `(v, v)` is one value. One call derives both bounds, under one
575+
ciphertexts, once block ORE lands), inclusive at both ends; `(v, v)` is one
576+
value. The client does not refuse `lo > hi`: as SQL's `BETWEEN`, such a
577+
range matches nothing. Judging it would mean reproducing, before
578+
encryption, the order the index compares in (text by its fold, floats by
579+
their canonical encoding), which belongs to the ORE crates. One call
580+
derives both bounds, under one
578581
context, so a caller cannot pair a bound from one column with a bound from
579582
another. The ordering operators are answered over the integer types and
580583
text (`op::Ordered`), by the ORE index by default and by OPE through

‎packages/stack-encrypt/src/codes.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ mod tests {
8181
TermError::Ore(_) => TermError::Ore(cllw_ore::Error),
8282
TermError::InvalidOptions(_) => TermError::InvalidOptions("k out of range"),
8383
TermError::EmptyTermText => TermError::EmptyTermText,
84-
TermError::InvertedRange => TermError::InvertedRange,
8584
TermError::NoEqualityTerm => TermError::NoEqualityTerm,
8685
TermError::NoOrderTerm => TermError::NoOrderTerm,
8786
TermError::Normalize(_) => {
@@ -396,7 +395,6 @@ mod tests {
396395
json!({ "len": 3 }),
397396
),
398397
(Box::new(TermError::EmptyTermText), json!({})),
399-
(Box::new(TermError::InvertedRange), json!({})),
400398
(Box::new(TermError::NoEqualityTerm), json!({})),
401399
(Box::new(TermError::NoOrderTerm), json!({})),
402400
(

‎packages/stack-encrypt/src/dynamic/query.rs‎

Lines changed: 36 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
use serde::{Serialize, Serializer};
7171

7272
use super::WireBytes;
73-
use std::cmp::Ordering;
7473
use std::fmt;
7574

7675
use stack_kms::DataKeySource;
@@ -585,19 +584,15 @@ fn not_a_range(field: &str) -> Error {
585584
}
586585

587586
/// The refusals the typed path makes when a query is awaited, before any
588-
/// term exists ([`Supports::check`](crate::op::Supports::check)): a range
589-
/// whose low bound is above its high bound, and match text the index
590-
/// cannot normalize or that cuts no token. The match index's own
591-
/// derivation refuses the last two again; asking here lets a binding
592-
/// refuse at its boundary.
587+
/// term exists ([`Supports::check`](crate::op::Supports::check)): match
588+
/// text the index cannot normalize or that cuts no token, and equality
589+
/// text the scheme cannot normalize. Each index's own derivation refuses
590+
/// these again; asking here lets a binding refuse at its boundary. A range
591+
/// whose low bound is above its high bound is not refused: as SQL's
592+
/// `BETWEEN`, it matches nothing.
593593
fn check_input(index: &IndexSpec, input: &Input) -> Result<(), Error> {
594594
let refused = |error: TermError| Error::Cipher(error.into());
595595
match (index, input) {
596-
(_, Input::Pair(lo, hi)) => {
597-
if integer_order(lo, hi) == Some(Ordering::Greater) {
598-
return Err(refused(TermError::InvertedRange));
599-
}
600-
}
601596
(IndexSpec::Match(options), Input::One(value)) => {
602597
query_tokens(value, options).map(drop)?;
603598
}
@@ -612,33 +607,6 @@ fn check_input(index: &IndexSpec, input: &Input) -> Result<(), Error> {
612607
Ok(())
613608
}
614609

615-
/// The order of two integers of one kind, or `None` for anything else.
616-
fn integer_order(lo: &ValueFrame, hi: &ValueFrame) -> Option<Ordering> {
617-
let (lo_kind, lo) = integer_of(lo)?;
618-
let (hi_kind, hi) = integer_of(hi)?;
619-
(lo_kind == hi_kind).then(|| lo.cmp(&hi))
620-
}
621-
622-
/// An integer scalar's kind and value, or `None` for anything else. The
623-
/// value is its sign and 128-bit two's-complement word (vitaminc's
624-
/// [`Int::sign_and_word`](vitaminc_aead::plaintext::Int::sign_and_word)),
625-
/// ordered negatives first, so one comparison covers every width from
626-
/// `i128::MIN` to `u128::MAX`.
627-
fn integer_of(value: &ValueFrame) -> Option<(ValueKind, (bool, u128))> {
628-
let kind = value.kind()?;
629-
let (negative, word) = value
630-
.visit(OneScalar::new(
631-
|ty: Option<Logical>, value: Physical<'_>| match (ty, value) {
632-
(None, Physical::Int(int)) => Ok(int.sign_and_word()),
633-
_ => Err(None),
634-
},
635-
))
636-
.ok()?;
637-
// `!negative` so a negative sorts below every non-negative; within a
638-
// sign the two's-complement words order as the values do.
639-
Some((kind, (!negative, word)))
640-
}
641-
642610
/// The tokens a match query of `value` keeps, under the index's options.
643611
///
644612
/// # Errors
@@ -730,10 +698,11 @@ pub fn check_query(
730698
/// field does not answer ([`Reason::OperatorNotAnswered`]), an input that
731699
/// cannot be read as the field's kind ([`Reason::FieldType`]) and a
732700
/// `between` input that is not two values ([`Reason::NotARange`]).
733-
/// [`Error::Cipher`] for the query refusals the typed path makes: a range
734-
/// whose low bound is above its high ([`TermError::InvertedRange`]), and
735-
/// match text that cuts no token ([`TermError::EmptyTermText`]) or that
736-
/// the index cannot normalize ([`TermError::Normalize`]).
701+
/// [`Error::Cipher`] for the query refusals the typed path makes: match
702+
/// text that cuts no token ([`TermError::EmptyTermText`]), and text the
703+
/// index cannot normalize ([`TermError::Normalize`]). A range whose low
704+
/// bound is above its high bound is not refused: as SQL's `BETWEEN`, it
705+
/// matches nothing.
737706
/// [`Error::Target`] for what the resolver refuses.
738707
pub fn query_term<'a, K>(
739708
cipher: &'a KeysetCipher<'_, K>,
@@ -1417,9 +1386,7 @@ mod tests {
14171386
async fn the_typed_paths_query_refusals_are_made_before_any_term() {
14181387
let cipher = cipher().await;
14191388
let keyset = cipher.default_keyset();
1420-
let pair = |lo, hi| Tree::Array(vec![Tree::UInt32(lo), Tree::UInt32(hi)]);
14211389
let cases = [
1422-
("age", OpSpec::Between, pair(65, 18), "inverted"),
14231390
("name", OpSpec::Matches, text("al"), "empty"),
14241391
("name", OpSpec::Matches, text("\u{0378}abc"), "normalize"),
14251392
("email", OpSpec::Eq, text("a\u{0378}bc"), "normalize"),
@@ -1431,7 +1398,6 @@ mod tests {
14311398
] {
14321399
let term = term_error(&error);
14331400
let expected = match what {
1434-
"inverted" => matches!(term, Some(TermError::InvertedRange)),
14351401
"empty" => matches!(term, Some(TermError::EmptyTermText)),
14361402
_ => matches!(term, Some(TermError::Normalize(_))),
14371403
};
@@ -1912,8 +1878,8 @@ mod tests {
19121878
])
19131879
.unwrap();
19141880
assert!(
1915-
ask("huge", OpSpec::Between, inverted).await.is_err(),
1916-
"an inverted 128-bit range is refused"
1881+
ask("huge", OpSpec::Between, inverted).await.is_ok(),
1882+
"an inverted 128-bit range is not refused"
19171883
);
19181884

19191885
let stored = |value: &ValueFrame, name: &'static str| {
@@ -1990,53 +1956,30 @@ mod tests {
19901956
);
19911957
}
19921958

1993-
/// A range is refused the wrong way round for every integer kind a
1994-
/// field can declare, and two values of different kinds have no order
1995-
/// here: the input was read as one kind first.
1996-
#[test]
1997-
fn a_range_is_ordered_within_each_integer_kind() {
1998-
let cases = [
1999-
(Tree::Int32(-1), Tree::Int32(1)),
2000-
(Tree::Int64(-1), Tree::Int64(1)),
2001-
(Tree::UInt32(1), Tree::UInt32(2)),
2002-
(Tree::UInt64(1), Tree::UInt64(2)),
2003-
];
2004-
for (lo, hi) in cases {
2005-
let (lo, hi) = (vf(&lo), vf(&hi));
2006-
assert_eq!(integer_order(&lo, &hi), Some(Ordering::Less));
2007-
assert_eq!(integer_order(&hi, &lo), Some(Ordering::Greater));
2008-
assert_eq!(integer_order(&lo, &lo), Some(Ordering::Equal));
2009-
}
2010-
assert_eq!(
2011-
integer_order(&vf(&Tree::UInt32(1)), &vf(&Tree::UInt64(2))),
2012-
None
2013-
);
2014-
assert_eq!(integer_order(&vf(&text("a")), &vf(&text("b"))), None);
2015-
// Every width, the 128-bit edges included: a `u128` above
2016-
// `i128::MAX` and a negative `i128` order as the numbers do.
2017-
let int = |int: vitaminc_aead::plaintext::Int| {
2018-
ValueFrame::of(vitaminc_aead::plaintext::Raw(
2019-
None,
2020-
vitaminc_aead::plaintext::Physical::Int(int),
2021-
))
2022-
.expect("a value")
1959+
/// A range whose low bound is above its high bound is not refused, as
1960+
/// SQL's `BETWEEN` is not: it is checked, and its terms derived, like
1961+
/// any other range, and matches nothing.
1962+
#[tokio::test]
1963+
async fn an_inverted_range_is_a_query_like_any_other() {
1964+
let cipher = cipher().await;
1965+
let keyset = cipher.default_keyset();
1966+
// `score` declares OPE alone, whose terms compare as bytes.
1967+
let inverted = Tree::Array(vec![Tree::Int64(65), Tree::Int64(18)]);
1968+
assert!(check_query(
1969+
&users(),
1970+
"score",
1971+
OpSpec::Between,
1972+
vf(&inverted),
1973+
&NoTargets
1974+
)
1975+
.is_ok());
1976+
let term = query(&keyset, "score", OpSpec::Between, inverted)
1977+
.await
1978+
.expect("derived");
1979+
let Operand::Range { lo, hi } = term.operand() else {
1980+
panic!("a range");
20231981
};
2024-
use vitaminc_aead::plaintext::Int;
2025-
for (lo, hi) in [
2026-
(Int::I8(-1), Int::I8(1)),
2027-
(Int::I16(-1), Int::I16(1)),
2028-
(Int::I128(i128::MIN), Int::I128(i128::MAX)),
2029-
(Int::I128(-1), Int::I128(0)),
2030-
(Int::U8(1), Int::U8(2)),
2031-
(Int::U16(1), Int::U16(2)),
2032-
(Int::U128(i128::MAX as u128), Int::U128(u128::MAX)),
2033-
] {
2034-
let (lo, hi) = (int(lo), int(hi));
2035-
assert_eq!(integer_order(&lo, &hi), Some(Ordering::Less));
2036-
assert_eq!(integer_order(&hi, &lo), Some(Ordering::Greater));
2037-
assert_eq!(integer_order(&lo, &lo), Some(Ordering::Equal));
2038-
}
2039-
assert_eq!(integer_order(&int(Int::I8(1)), &int(Int::I16(2))), None);
1982+
assert!(lo > hi, "the bounds as given, and the terms in their order");
20401983
}
20411984

20421985
#[test]

0 commit comments

Comments
 (0)