Skip to content

Commit

Permalink
Use P256KeyPair rather than just storing the secret key bytes (#6083)
Browse files Browse the repository at this point in the history
  • Loading branch information
hpeebles authored Jul 22, 2024
1 parent 5ff1261 commit 253a9b3
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 7 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions backend/canisters/local_user_index/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed

- Clear old data from the failed upgrades log ([#6062](https://github.com/open-chat-labs/open-chat/pull/6062))
- Use `P256KeyPair` rather than just storing the secret key bytes ([#6083](https://github.com/open-chat-labs/open-chat/pull/6083))

### Removed

Expand Down
1 change: 1 addition & 0 deletions backend/canisters/local_user_index/impl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jwt = { path = "../../../libraries/jwt" }
ledger_utils = { path = "../../../libraries/ledger_utils" }
local_user_index_canister = { path = "../api" }
msgpack = { path = "../../../libraries/msgpack" }
p256_key_pair = { path = "../../../libraries/p256_key_pair" }
proof_of_unique_personhood = { path = "../../../libraries/proof_of_unique_personhood" }
rand = { workspace = true }
serde = { workspace = true }
Expand Down
10 changes: 8 additions & 2 deletions backend/canisters/local_user_index/impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use event_store_utils::EventDeduper;
use local_user_index_canister::GlobalUser;
use model::global_user_map::GlobalUserMap;
use model::local_user_map::LocalUserMap;
use p256_key_pair::P256KeyPair;
use proof_of_unique_personhood::verify_proof_of_unique_personhood;
use serde::{Deserialize, Serialize};
use std::cell::RefCell;
Expand Down Expand Up @@ -231,7 +232,7 @@ impl RuntimeState {
event_relay: event_relay_canister_id,
internet_identity: self.data.internet_identity_canister_id,
},
oc_secret_key_initialized: self.data.oc_secret_key_der.is_some(),
oc_secret_key_initialized: self.data.oc_key_pair.is_initialised(),
canister_upgrades_failed: canister_upgrades_metrics.failed,
}
}
Expand Down Expand Up @@ -265,6 +266,8 @@ struct Data {
pub rng_seed: [u8; 32],
pub video_call_operators: Vec<Principal>,
pub oc_secret_key_der: Option<Vec<u8>>,
#[serde(default)]
pub oc_key_pair: P256KeyPair,
pub event_store_client: EventStoreClient<CdkRuntime>,
pub event_deduper: EventDeduper,
pub users_to_delete_queue: VecDeque<UserToDelete>,
Expand Down Expand Up @@ -337,7 +340,10 @@ impl Data {
referral_codes: ReferralCodes::default(),
rng_seed: [0; 32],
video_call_operators,
oc_secret_key_der,
oc_secret_key_der: None,
oc_key_pair: oc_secret_key_der
.map(|sk| P256KeyPair::from_secret_key_der(sk).unwrap())
.unwrap_or_default(),
event_store_client: EventStoreClientBuilder::new(event_relay_canister_id, CdkRuntime::default())
.with_flush_delay(Duration::from_millis(MINUTE_IN_MS))
.build(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use crate::lifecycle::{init_env, init_state};
use crate::memory::get_upgrades_memory;
use crate::Data;
use crate::{mutate_state, Data};
use canister_logger::LogEntry;
use canister_tracing_macros::trace;
use ic_cdk::post_upgrade;
use local_user_index_canister::post_upgrade::Args;
use p256_key_pair::P256KeyPair;
use stable_memory::get_reader;
use tracing::info;
use utils::cycles::init_cycles_dispenser_client;
Expand All @@ -23,5 +24,11 @@ fn post_upgrade(args: Args) {
init_cycles_dispenser_client(data.cycles_dispenser_canister_id, data.test_mode);
init_state(env, data, args.wasm_version);

mutate_state(|state| {
if let Some(sk_der) = state.data.oc_secret_key_der.clone() {
state.data.oc_key_pair = P256KeyPair::from_secret_key_der(sk_der).unwrap();
}
});

info!(version = %args.wasm_version, "Post-upgrade complete");
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn get_user(state: &RuntimeState) -> Option<(UserId, bool)> {
}

fn build_token<T: Serialize>(token_type: AccessTokenType, custom_claims: T, state: &mut RuntimeState) -> Response {
let Some(secret_key_der) = state.data.oc_secret_key_der.as_ref() else {
if !state.data.oc_key_pair.is_initialised() {
return InternalError("OC Secret not set".to_string());
};

Expand All @@ -93,7 +93,7 @@ fn build_token<T: Serialize>(token_type: AccessTokenType, custom_claims: T, stat
custom_claims,
);

match jwt::sign_and_encode_token(secret_key_der, claims, &mut rng) {
match jwt::sign_and_encode_token(state.data.oc_key_pair.secret_key_der(), claims, &mut rng) {
Ok(token) => Success(token),
Err(err) => InternalError(format!("{err:?}")),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use canister_api_macros::update;
use canister_tracing_macros::trace;
use local_user_index_canister::c2c_notify_user_index_events::{Response::*, *};
use local_user_index_canister::Event;
use p256_key_pair::P256KeyPair;
use std::cmp::min;
use tracing::info;
use user_canister::{
Expand Down Expand Up @@ -177,7 +178,9 @@ fn handle_event(event: Event, state: &mut RuntimeState) {
}
}
Event::SecretKeySet(sk_der) => {
state.data.oc_secret_key_der = Some(sk_der);
if let Ok(key_pair) = P256KeyPair::from_secret_key_der(sk_der) {
state.data.oc_key_pair = key_pair;
}
}
Event::NotifyUniquePersonProof(user_id, proof) => {
state.data.global_users.insert_unique_person_proof(user_id, proof);
Expand Down
15 changes: 14 additions & 1 deletion backend/libraries/p256_key_pair/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use p256::pkcs8::DecodePrivateKey;
use p256::{
ecdsa,
elliptic_curve::{rand_core::CryptoRngCore, subtle::CtOption, NonZeroScalar},
Expand All @@ -14,6 +15,14 @@ pub struct P256KeyPair {
}

impl P256KeyPair {
pub fn from_secret_key_der(sk_der: Vec<u8>) -> Result<P256KeyPair, Box<dyn Error>> {
let p256_sk = p256::SecretKey::from_pkcs8_der(&sk_der)?;
let sk = ecdsa::SigningKey::from_bytes(&p256_sk.to_bytes())?;
let pk_pem = Self::signing_key_to_pem(sk);

Ok(P256KeyPair { sk_der, pk_pem })
}

pub fn initialize(&mut self, rng: &mut impl CryptoRngCore) {
if self.is_initialised() {
return;
Expand All @@ -22,7 +31,7 @@ impl P256KeyPair {
let sk: ecdsa::SigningKey = ecdsa::SigningKey::random(rng);

self.sk_der = P256KeyPair::to_der(&sk).unwrap();
self.pk_pem = sk.verifying_key().to_public_key_pem(Default::default()).unwrap();
self.pk_pem = Self::signing_key_to_pem(sk);
}

pub fn secret_key_der(&self) -> &[u8] {
Expand All @@ -37,6 +46,10 @@ impl P256KeyPair {
!self.sk_der.is_empty()
}

fn signing_key_to_pem(sk: ecdsa::SigningKey) -> String {
sk.verifying_key().to_public_key_pem(Default::default()).unwrap()
}

fn to_der(p256_sk: &ecdsa::SigningKey) -> Result<Vec<u8>, Box<dyn Error>> {
let scalar: CtOption<NonZeroScalar<NistP256>> = NonZeroScalar::from_repr(p256_sk.to_bytes());
if bool::from(scalar.is_none()) {
Expand Down

0 comments on commit 253a9b3

Please sign in to comment.