Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pruntime: Implement dcap key handover #1524

Merged
merged 8 commits into from
Feb 26, 2024
Merged

pruntime: Implement dcap key handover #1524

merged 8 commits into from
Feb 26, 2024

Conversation

kvinwang
Copy link
Collaborator

This PR implements the key handover feature according to the design from @shelvenzhou.

the RTP time is got from at least two of the white listed servers as show below:

/// Ref: https://github.com/jauderho/nts-servers/
const TRUSTED_NTS_SERVERS: &[&str] = &[
    "time.cloudflare.com",
    "gps.ntp.br",
    "a.st1.ntp.br",
    "paris.time.system76.com",
    "ntp3.fau.de",
    "ptbtime1.ptb.de",
    "ntppool1.time.nl",
    "nts.netnod.se",
];

A few points to note:

  • We use cfnts as nts client with much refactor here to make it meet our requirements.
  • Because our phala-nts introduce dep of latest rustls, some of the crates in the deps tree conflicts. So we have to patch them.
  • SGX Local attestation is used instead of the RA in the design, I think it meet the requirement.
  • Since RTP use trusted server from foreign countries and over UDP, it is hard to success get time from those servers in China mainland.
  • dev_mode is not fully supported in this version (Won't success in dev_mode). (Maybe added in the future).

@kvinwang kvinwang requested a review from shelvenzhou January 30, 2024 10:58
shelvenzhou
shelvenzhou reviewed Feb 6, 2024
View reviewed changes
crates/phactory/src/nts.rs Outdated
}

fn validate_results(results: Vec<u64>) -> Result<u64> {
if results.len() < 2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

make these thresholds as constants so we can easily notice and update them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

crates/phactory/src/nts.rs Outdated
.map(|r| (*r as i64 - average as i64).unsigned_abs())
.max()
.unwrap_or_default();
if max_diff > 60 {
Copy link
Contributor

Choose a reason for hiding this comment

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

as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

crates/phactory/src/prpc_service.rs Outdated
// 4. verify challenge block height and report timestamp
// only challenge within 150 blocks (30 minutes) is accepted
let challenge_height = challenge.block_number;
if !(challenge_height <= block_number && block_number - challenge_height <= 150) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems superceded by the challenge.ntp_time_secs check above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this one.

crates/phactory/src/prpc_service.rs
.map_err(|_| from_display("Invalid client sgx target info"))?
};
// the report data does not matter since we only care about the origin
let report = sgx_api_lite::report(its_target_info, &report_data)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean an IAS pRuntime can also get key from DCAP pRuntime?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, IAS pruntime validates RA to each other during handover, while dcap pruntime server won't generate RA report. So, an IAS pruntime won't accept an handover from DCAP pruntime.

@shelvenzhou
Copy link
Contributor

Double check @kvinwang : is NTP Authentication enabled in phala-nts?

@shelvenzhou
Copy link
Contributor

shelvenzhou commented Feb 6, 2024 •
edited
Loading

I would also like to highlight some constants for future reference:

  1. The handover is only allowed on the same physical machine, remote handover is not allowed;
  2. The key sender must be a DCAP pRuntime, whether the receiver must be a DCAP is under checking;
  3. Now we use NTP server for timestamp. We have 8 time servers, and will request all of them; at least 2 responses need to be received within 5s timeout, and their difference need to be within 60s. The average is used as final timestamp;
  4. For sender worker: its latest block must be within one week of the challenge timestamp;
  5. For receiver worker: it must reply to the challenge within 1min.

@nanometerzhu
Copy link
Contributor

Regarding to the NTP servers, there are 5 servers are in north Europe, and 2 are in Brazil. Are they too close?

@kvinwang
Copy link
Collaborator Author

kvinwang commented Feb 9, 2024

is NTP Authentication enabled in phala-nts?

Yes, it's enabled.

@kvinwang
Copy link
Collaborator Author

kvinwang commented Feb 9, 2024

Regarding to the NTP servers, there are 5 servers are in north Europe, and 2 are in Brazil. Are they too close?

@shelvenzhou do you think this is a problem?

@kvinwang kvinwang requested a review from shelvenzhou February 19, 2024 05:40
@shelvenzhou
Copy link
Contributor

Regarding to the NTP servers, there are 5 servers are in north Europe, and 2 are in Brazil. Are they too close?

@shelvenzhou do you think this is a problem?

Can we make this configurable with on-chain governance?

@kvinwang
Copy link
Collaborator Author

Can we make this configurable with on-chain governance?

Good idea.

@kvinwang
Copy link
Collaborator Author

Can we make this configurable with on-chain governance?

Added on-chain governance.

shelvenzhou
shelvenzhou approved these changes Feb 26, 2024
View reviewed changes
Copy link
Contributor

@shelvenzhou shelvenzhou left a comment

Choose a reason for hiding this comment

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

LGTM

@kvinwang kvinwang merged commit dfa3605 into master Feb 26, 2024
8 checks passed
@kvinwang kvinwang deleted the dcap-handover branch February 26, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shelvenzhou shelvenzhou shelvenzhou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kvinwang @shelvenzhou @nanometerzhu