Skip to content

feat: make sure keygen generates homPRF key#560

Open
kc1212 wants to merge 5 commits intomainfrom
kc1212/feat/2980/prf-keygen-service
Open

feat: make sure keygen generates homPRF key#560
kc1212 wants to merge 5 commits intomainfrom
kc1212/feat/2980/prf-keygen-service

Conversation

@kc1212
Copy link
Copy Markdown
Contributor

@kc1212 kc1212 commented Apr 30, 2026

Description of changes

Integrate homomorphic prf keygen on the service level.

There is a subtle issue around resharing. Namely when we rehare a sk between two different sets, set2 does not know whether set1 has a hom. prf key or not, so it does not know whether to run the protocol. The workaround is to have set2 detect from the public keys whether resharing should be run on the hom. prf keys. This means set1 and set2 is setting the flag oprf_key_present from different sources, the former sets it using its own secret key share, the latter sets it using what it finds in the public key.

Issue ticket number and link

Closes zama-ai/kms-internal#2980

PR Checklist

I attest that all checked items are satisfied. Any deviation is clearly justified above.

  • Title follows conventional commits (e.g. chore: ...).
  • Tests added for every new pub item and test coverage has not decreased.
  • Public APIs and non-obvious logic documented; unfinished work marked as TODO(#issue).
  • unwrap/expect/panic only in tests or for invariant bugs (documented if present).
  • No dependency version changes OR (if changed) only minimal required fixes.
  • No architectural protocol changes OR linked spec PR/issue provided.
  • No breaking deployment config changes OR devops label + infra notified + infra-team reviewer assigned.
  • No breaking gRPC / serialized data changes OR commit marked with ! and affected teams notified.
  • No modifications to existing versionized structs OR backward compatibility tests updated.
  • No critical business logic / crypto changes OR ≥2 reviewers assigned.
  • No new sensitive data fields added OR Zeroize + ZeroizeOnDrop implemented.
  • No new public storage data OR data is verifiable (signature / digest).
  • No unsafe; if unavoidable: minimal, justified, documented, and test/fuzz covered.
  • Strongly typed boundaries: typed inputs validated at the edge; no untyped values or errors cross modules.
  • Self-review completed.

Dependency Update Questionnaire (only if deps changed or added)

Answer in the Cargo.toml next to the dependency (or here if updating):

  1. Ownership changes or suspicious concentration?
  2. Low popularity?
  3. Unusual version jump?
  4. Lacking documentation?
  5. Missing CI?
  6. No security / disclosure policy?
  7. Significant size increase?

More details and explanations for the checklist and dependency updates can be found in CONTRIBUTING.md

@kc1212 kc1212 self-assigned this Apr 30, 2026
@kc1212 kc1212 requested a review from a team as a code owner April 30, 2026 11:52
@cla-bot cla-bot Bot added the cla-signed The CLA has been signed. label Apr 30, 2026
@kc1212 kc1212 marked this pull request as draft April 30, 2026 11:52
@kc1212 kc1212 force-pushed the kc1212/feat/2980/prf-keygen-service branch from 97bc64b to e3f3ec7 Compare April 30, 2026 12:42
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 30, 2026

Consolidated Tests Results 2026-05-04 - 13:41:07

Test Results

passed 9 passed

Details

tests 9 tests
clock not captured
tool junit-to-ctrf
build build-and-test arrow-right test-reporter link #1867
pull-request feat: make sure keygen generates homPRF key link #560

test-reporter: Run #1867

Tests 📝 Passed ✅ Failed ❌ Skipped ⏭️ Pending ⏳ Other ❓ Flaky 🍂 Duration ⏱️
9 9 0 0 0 0 0 not captured

🎉 All tests passed!

Tests

View All Tests
Test Name Status Flaky Duration
k8s_test_crs_uniqueness 32.8s
k8s_test_insecure_keygen_encrypt_and_public_decrypt 2m 6s
k8s_test_insecure_keygen_encrypt_multiple_types 2m 17s
k8s_test_keygen_and_crs 2m 1s
k8s_test_keygen_uniqueness 5m 15s
k8s_test_centralized_insecure 54.4s
nightly_full_gen_tests_default_k8s_centralized_sequential_crs 1.8s
k8s_test_centralized_insecure 54.2s
nightly_full_gen_tests_default_k8s_centralized_sequential_crs 1.8s

🍂 No flaky tests in this run.

Github Test Reporter by CTRF 💚

🔄 This comment has been updated

@kc1212 kc1212 force-pushed the kc1212/feat/2980/prf-keygen-service branch 4 times, most recently from 447d725 to 11a6018 Compare May 4, 2026 10:32
@kc1212 kc1212 marked this pull request as ready for review May 4, 2026 10:38
@kc1212 kc1212 force-pushed the kc1212/feat/2980/prf-keygen-service branch 3 times, most recently from 5498923 to aae070f Compare May 4, 2026 12:45
@kc1212 kc1212 force-pushed the kc1212/feat/2980/prf-keygen-service branch from aae070f to 235a7a7 Compare May 4, 2026 12:45
titouantanguy
titouantanguy previously approved these changes May 4, 2026
Copy link
Copy Markdown
Contributor

@titouantanguy titouantanguy left a comment

Choose a reason for hiding this comment

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

LGTM ! I think it's fine to deduce whether there is a dedicated oprf from the public key during resahring as it's trusted too (cause we check a majority of the resharer give us the same)

}

impl VerifiedPublicMaterial {
// TODO: is there no better way to find if a particular key exists? we want to avoid the cloning
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.

Probably would need to ask tfhe-rs.
(more genrally, could be nice to have something that gives a ref to the inner keys instead of having to take onwership)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

improved slightly in 37b620a also asked tfhe-rs

/// negative check: a fresh OPRF server key built from a one-bit-flipped
/// version of the same LWE key must produce some mismatches against the
/// cleartext reference.
fn assert_oprf_correctness<const EXTENSION_DEGREE: usize>(
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.

part of this fn is duplicated with check_oprf_correctness from service/src/client/key_gen.rs I believe ?
Should we try and factor the duplicated part somewhere accessible to both here and there ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

refactored in 37b620a

num_triples_needed +=
// Raw triples necessary for the 2 BK
self.lwe_dimension().0 * (self.glwe_sk_num_bits() + self.glwe_sk_num_bits_sns());
num_triples_needed += self.lwe_dimension().0 * self.glwe_sk_num_bits();
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.

What about modifying the line above with

num_triples_needed +=
// Raw triples necessary for the regular BK, the OPRF BK and the SnS BK
self.lwe_dimension().0 * (2*self.glwe_sk_num_bits() + self.glwe_sk_num_bits_sns());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

refactored in 37b620a

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

Labels

cla-signed The CLA has been signed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants