Add Nitriding key sync support #364

DJAndries · 2024-07-19T23:09:19Z

No description provided.

github-actions · 2024-07-19T23:12:59Z

src/instance.rs

+            .instance_names
+            .iter()
+            .position(|name| name == instance_name)
+            .unwrap();


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

github-actions · 2024-07-19T23:12:59Z

src/instance.rs

+        // Get base time for calculating curren epochs
+        let now = time::OffsetDateTime::now_utc()
+            .replace_millisecond(0)
+            .expect("failed to remove millisecond component from OffsetDateTime");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

github-actions · 2024-07-19T23:12:59Z

src/instance.rs

+            for epoch in config.first_epoch..current_epoch {
+                server
+                    .puncture(epoch)
+                    .expect("Failed to puncture obsolete epoch");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

github-actions · 2024-07-19T23:12:59Z

src/state.rs

+        if !self.is_leader.initialized() {
+            self.is_leader
+                .set(false)
+                .expect("failed to set leader status");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

github-actions · 2024-07-19T23:12:59Z

src/state.rs

+                        None => {
+                            let new_instance =
+                                OPRFInstance::new(&self.config, &instance_name, false)
+                                    .expect("Could not initialize PPOPRF state");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

github-actions · 2024-07-19T23:12:59Z

src/state.rs

+
+                    instance_guard
+                        .as_mut()
+                        .unwrap()


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

github-actions · 2024-07-19T23:12:59Z

src/state.rs

+        if !self.is_leader.initialized() {
+            self.is_leader
+                .set(true)
+                .expect("failed to set leader status");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

github-actions · 2024-07-19T23:12:59Z

src/state.rs

+        }
+        let mut private_keys = OPRFKeysRef::default();
+        for (instance_name, instance) in &mut server_guards {
+            let instance = instance.as_ref().unwrap();


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon @bcaller

claucece

LGTM!

github-actions · 2024-08-31T03:02:03Z

[puLL-Merge] - brave/star-randsrv@364

Description

This PR introduces significant changes to the STAR randomness webservice, primarily focusing on enabling key synchronization between leader and worker enclaves. The changes include modifications to the server structure, handling of PPOPRF instances, and the addition of new endpoints for key management. The PR also updates dependencies and improves error handling.

Possible Issues

The new key synchronization feature might introduce potential race conditions or consistency issues if not properly managed across multiple instances.
The changes to the epoch rotation logic could potentially affect the randomness guarantees if not implemented correctly.
The new dependency on reqwest for HTTP requests to nitriding might introduce new failure modes that need to be handled carefully.

Security Hotspots

Key Management: The new methods for setting and getting private keys (set_ppoprf_private_key and get_ppoprf_private_key) are critical security points. Ensure that these endpoints are properly secured and only accessible within the trusted enclave environment.
Leader/Worker Designation: The logic for determining whether a server is a leader or worker (self.is_leader.set()) is a crucial security point. Ensure that this cannot be manipulated by an attacker to gain unauthorized access to key material.
Key Synchronization: The process of sending private keys to nitriding (send_private_keys_to_nitriding) is a potential point of key material exposure. Ensure that this communication channel is properly secured.

Changes

Changes

Cargo.toml and Cargo.lock:
- Updated version to 0.3.0
- Added new dependencies including reqwest and bincode
README.md:
- Added information about reproducible builds and Nitro Enclave image auditing
src/handler.rs:
- Modified to handle the new key synchronization features
- Added new endpoints for setting and getting PPOPRF private keys
src/instance.rs (new file):
- Introduced OPRFInstance struct to manage OPRF instances
src/main.rs:
- Updated Config struct to include new options for key synchronization
- Modified app function to conditionally add new routes for key management
src/result.rs (new file):
- Introduced custom Error type and Result alias
src/state.rs:
- Significant changes to OPRFServer struct and its methods to support key synchronization
- Added methods for setting and getting private keys
src/tests.rs:
- Added new tests for enclave leader and worker scenarios
- Modified existing tests to accommodate new functionality
src/util.rs:
- Added utility function for sending private keys to nitriding

These changes significantly alter the structure and behavior of the application, particularly in how it manages PPOPRF instances and synchronizes keys between leader and worker enclaves. The new functionality requires careful review to ensure it maintains the security and correctness guarantees of the original implementation.

github-actions · 2024-08-31T03:05:16Z

src/instance.rs

+            .instance_names
+            .iter()
+            .position(|name| name == instance_name)
+            .unwrap();


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

github-actions · 2024-08-31T03:05:16Z

src/instance.rs

+        // Get base time for calculating curren epochs
+        let now = time::OffsetDateTime::now_utc()
+            .replace_millisecond(0)
+            .expect("failed to remove millisecond component from OffsetDateTime");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

github-actions · 2024-08-31T03:05:16Z

src/instance.rs

+            for epoch in config.first_epoch..current_epoch {
+                server
+                    .puncture(epoch)
+                    .expect("Failed to puncture obsolete epoch");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

github-actions · 2024-08-31T03:05:16Z

src/state.rs

+        if !self.is_leader.initialized() {
+            self.is_leader
+                .set(false)
+                .expect("failed to set leader status");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

github-actions · 2024-08-31T03:05:16Z

src/state.rs

+                        None => {
+                            let new_instance =
+                                OPRFInstance::new(&self.config, &instance_name, false)
+                                    .expect("Could not initialize PPOPRF state");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

github-actions · 2024-08-31T03:05:16Z

src/state.rs

+
+                    instance_guard
+                        .as_mut()
+                        .unwrap()


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

github-actions · 2024-08-31T03:05:16Z

src/state.rs

+        if !self.is_leader.initialized() {
+            self.is_leader
+                .set(true)
+                .expect("failed to set leader status");


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

github-actions · 2024-08-31T03:05:16Z

src/state.rs

+        }
+        let mut private_keys = OPRFKeysRef::default();
+        for (instance_name, instance) in &mut server_guards {
+            let instance = instance.as_ref().unwrap();


_{reported by reviewdog 🐶}
[semgrep] expect or unwrap called in function returning a Result

Source: https://semgrep.dev/r/trailofbits.rs.panic-in-function-returning-result.panic-in-function-returning-result

Cc @thypon

DJAndries requested a review from a team as a code owner July 19, 2024 23:09

github-actions bot added the puLL-Merge label Jul 19, 2024

github-actions bot reviewed Jul 19, 2024

View reviewed changes

github-actions bot added the needs-security-review label Jul 19, 2024

github-actions bot assigned bcaller and thypon Jul 19, 2024

DJAndries force-pushed the key-sync branch from 8aa34a0 to 9922e14 Compare August 26, 2024 20:11

claucece approved these changes Aug 27, 2024

View reviewed changes

DJAndries force-pushed the key-sync branch from 9922e14 to 536f60a Compare August 31, 2024 02:59

DJAndries added 3 commits August 30, 2024 20:01

Add Nitriding key sync support

c7e070d

Amend README to mention custom-built enclave kernel

3a24951

Fix Cargo.lock

747aaf4

DJAndries force-pushed the key-sync branch from 536f60a to 747aaf4 Compare August 31, 2024 03:01

github-actions bot reviewed Aug 31, 2024

View reviewed changes

Add Nitriding key sync support #364

Are you sure you want to change the base?

Add Nitriding key sync support #364

Conversation

DJAndries commented Jul 19, 2024

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

claucece left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 31, 2024

Description

Possible Issues

Security Hotspots

Changes

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot Aug 31, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants