Skip to content

Commit 89f0987

Browse files
committed
revocation: de-duplicate redundant check work
Since a covering filter is queried with every SCT timestamp, consulting it again for a later SCT can't produce a different answer. Remember which filter files have been seen & processed and avoid re-doing that work.
1 parent 0447cb0 commit 89f0987

1 file changed

Lines changed: 77 additions & 1 deletion

File tree

upki/src/revocation/index.rs

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,13 @@ impl Index {
190190
///
191191
/// Uses binary search over the log-ID directory to find relevant entries, then seeks into
192192
/// the index file to read only those entries. Loads matching filter files and queries
193-
/// them for the certificate's revocation status.
193+
/// them for the certificate's revocation status. Each distinct filter file is read and
194+
/// parsed at most once per check.
194195
pub fn check(&mut self, input: &RevocationCheckInput) -> Result<RevocationStatus, Error> {
195196
let key = input.key();
196197
let dir_data = &self.tables[self.logs_offset..];
197198
let mut maybe_good = false;
199+
let mut seen = [false; 256];
198200

199201
'timestamp: for sct in &input.sct_timestamps {
200202
// Binary search the sorted log_id directory (stride LOG_DIR_ENTRY_SIZE)
@@ -237,6 +239,13 @@ impl Index {
237239
continue;
238240
}
239241

242+
// A filter is queried with every SCT timestamp, so consulting it
243+
// again for a later SCT cannot produce a different answer.
244+
if seen[filter_index] {
245+
continue;
246+
}
247+
seen[filter_index] = true;
248+
240249
let filename = self.filename(filter_index)?;
241250
let path = self.cache_dir.join(filename);
242251
let bytes = match fs::read(&path) {
@@ -804,6 +813,73 @@ mod tests {
804813
Ok(())
805814
}
806815

816+
// One filter covers both SCT logs but does not enroll our issuer. Its second
817+
// encounter (for the second SCT) is skipped as already-queried, and that skip
818+
// must not prevent the second log's other covering filter from being consulted
819+
// and revoking.
820+
#[test]
821+
fn check_skips_queried_filter_but_not_later_filters() -> Result<(), Box<dyn StdError>> {
822+
let dir = tempfile::tempdir()?;
823+
let config = test_config(dir.path());
824+
825+
let (log_a, log_b) = ([0xb1; 32], [0xb2; 32]);
826+
// f0 covers both logs but enrolls a different issuer -> NotEnrolled for us.
827+
let f0 = build_filter(
828+
[0xcc; 32],
829+
&[&[7, 7]],
830+
&[(log_a, 0, 2000), (log_b, 0, 2000)],
831+
);
832+
// f1 covers log_b and revokes our serial.
833+
let f1 = build_filter([0xaa; 32], &[&[1, 2, 3]], &[(log_b, 0, 2000)]);
834+
write_file(dir.path(), "f0.filter", &f0);
835+
write_file(dir.path(), "f1.filter", &f1);
836+
write_file(
837+
dir.path(),
838+
INDEX_BIN,
839+
&build_index(&[
840+
("f0.filter", &[(log_a, 0, 2000), (log_b, 0, 2000)]),
841+
("f1.filter", &[(log_b, 0, 2000)]),
842+
]),
843+
);
844+
845+
assert_eq!(
846+
Index::from_cache(&config)?.check(&multi_sct_input(&[(log_a, 1000), (log_b, 1000)]))?,
847+
RevocationStatus::CertainlyRevoked,
848+
);
849+
850+
Ok(())
851+
}
852+
853+
// A single filter covers both SCT logs and answers conclusively "not revoked".
854+
// The verdict from its first (and only) query must survive the deduplicated
855+
// second encounter.
856+
#[test]
857+
fn check_single_filter_covering_multiple_scts_not_revoked() -> Result<(), Box<dyn StdError>> {
858+
let dir = tempfile::tempdir()?;
859+
let config = test_config(dir.path());
860+
861+
let (log_a, log_b) = ([0xb1; 32], [0xb2; 32]);
862+
// f0 enrolls our issuer but revokes a different serial -> not revoked.
863+
let f0 = build_filter(
864+
[0xaa; 32],
865+
&[&[9, 9, 9]],
866+
&[(log_a, 0, 2000), (log_b, 0, 2000)],
867+
);
868+
write_file(dir.path(), "f0.filter", &f0);
869+
write_file(
870+
dir.path(),
871+
INDEX_BIN,
872+
&build_index(&[("f0.filter", &[(log_a, 0, 2000), (log_b, 0, 2000)])]),
873+
);
874+
875+
assert_eq!(
876+
Index::from_cache(&config)?.check(&multi_sct_input(&[(log_a, 1000), (log_b, 1000)]))?,
877+
RevocationStatus::NotRevoked,
878+
);
879+
880+
Ok(())
881+
}
882+
807883
#[expect(clippy::type_complexity)]
808884
fn build_index(filters: &[(&str, &[([u8; 32], u64, u64)])]) -> Vec<u8> {
809885
// Aggregate by log_id using BTreeMap for sorted order

0 commit comments

Comments
 (0)