Skip to content

Commit

Permalink
Only allow identities from approved originating canisters (#6060)
Browse files Browse the repository at this point in the history
  • Loading branch information
hpeebles authored Jul 18, 2024
1 parent a44cb88 commit ee21557
Show file tree
Hide file tree
Showing 14 changed files with 51 additions and 13 deletions.
4 changes: 4 additions & 0 deletions backend/canisters/identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## [unreleased]

### Changed

- Only allow identities from approved originating canisters ([#6060](https://github.com/open-chat-labs/open-chat/pull/6060))

## [[2.0.1239](https://github.com/open-chat-labs/open-chat/releases/tag/v2.0.1239-identity)] - 2024-07-17

### Added
Expand Down
1 change: 1 addition & 0 deletions backend/canisters/identity/api/can.did
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type CreateIdentityResponse = variant {
Success : PrepareDelegationSuccess;
AlreadyRegistered;
PublicKeyInvalid : text;
OriginatingCanisterInvalid : principal;
ChallengeRequired;
ChallengeFailed;
};
Expand Down
1 change: 1 addition & 0 deletions backend/canisters/identity/api/src/lifecycle/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub struct Args {
pub governance_principals: Vec<Principal>,
pub user_index_canister_id: CanisterId,
pub cycles_dispenser_canister_id: CanisterId,
pub originating_canisters: Vec<CanisterId>,
pub skip_captcha_whitelist: Vec<CanisterId>,
pub ic_root_key: Vec<u8>,
pub wasm_version: BuildVersion,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::ChallengeAttempt;
use candid::{CandidType, Deserialize};
use serde::Serialize;
use types::Nanoseconds;
use types::{CanisterId, Nanoseconds};

#[derive(CandidType, Serialize, Deserialize, Debug)]
pub struct Args {
Expand All @@ -18,6 +18,7 @@ pub enum Response {
Success(SuccessResult),
AlreadyRegistered,
PublicKeyInvalid(String),
OriginatingCanisterInvalid(CanisterId),
ChallengeRequired,
ChallengeFailed,
}
Expand Down
4 changes: 4 additions & 0 deletions backend/canisters/identity/impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ struct Data {
governance_principals: HashSet<Principal>,
user_index_canister_id: CanisterId,
cycles_dispenser_canister_id: CanisterId,
#[serde(default)]
originating_canisters: HashSet<CanisterId>,
skip_captcha_whitelist: HashSet<CanisterId>,
user_principals: UserPrincipals,
#[serde(default)]
Expand All @@ -111,6 +113,7 @@ impl Data {
governance_principals: HashSet<Principal>,
user_index_canister_id: CanisterId,
cycles_dispenser_canister_id: CanisterId,
originating_canisters: Vec<CanisterId>,
skip_captcha_whitelist: Vec<CanisterId>,
ic_root_key: Vec<u8>,
test_mode: bool,
Expand All @@ -119,6 +122,7 @@ impl Data {
governance_principals,
user_index_canister_id,
cycles_dispenser_canister_id,
originating_canisters: originating_canisters.into_iter().collect(),
skip_captcha_whitelist: skip_captcha_whitelist.into_iter().collect(),
user_principals: UserPrincipals::default(),
identity_link_requests: IdentityLinkRequests::default(),
Expand Down
1 change: 1 addition & 0 deletions backend/canisters/identity/impl/src/lifecycle/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ fn init(args: Args) {
args.governance_principals.into_iter().collect(),
args.user_index_canister_id,
args.cycles_dispenser_canister_id,
args.originating_canisters,
args.skip_captcha_whitelist,
args.ic_root_key,
args.test_mode,
Expand Down
28 changes: 18 additions & 10 deletions backend/canisters/identity/impl/src/lifecycle/post_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use canister_tracing_macros::trace;
use ic_cdk::post_upgrade;
use identity_canister::post_upgrade::Args;
use stable_memory::get_reader;
use std::collections::HashSet;
use tracing::info;
use types::CanisterId;
use utils::cycles::init_cycles_dispenser_client;

#[post_upgrade]
Expand All @@ -27,17 +27,25 @@ fn post_upgrade(args: Args) {
info!(version = %args.wasm_version, "Post-upgrade complete");

mutate_state(|state| {
// Remove ETH and SOL canisters from skip_captcha_whitelist
let canisters_to_remove = HashSet::from([
"2notu-qyaaa-aaaar-qaeha-cai".to_string(),
"2kpva-5aaaa-aaaar-qaehq-cai".to_string(),
"4s357-zaaaa-aaaaf-bjz7q-cai".to_string(),
"lix6w-ciaaa-aaaaf-bj2aa-cai".to_string(),
]);
let originating_canisters = if state.data.test_mode {
[
"rdmx6-jaaaa-aaaaa-aaadq-cai", // II
"rubs2-eaaaa-aaaaf-bijfq-cai", // Email
"4s357-zaaaa-aaaaf-bjz7q-cai", // ETH
"lix6w-ciaaa-aaaaf-bj2aa-cai", // SOL
]
} else {
[
"rdmx6-jaaaa-aaaaa-aaadq-cai", // II
"zi2i7-nqaaa-aaaar-qaemq-cai", // Email
"2notu-qyaaa-aaaar-qaeha-cai", // ETH
"2kpva-5aaaa-aaaar-qaehq-cai", // SOL
]
};

state
.data
.skip_captcha_whitelist
.retain(|e| !canisters_to_remove.contains(&e.to_string()));
.originating_canisters
.extend(originating_canisters.iter().map(|s| CanisterId::from_text(s).unwrap()));
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ fn create_identity_impl(args: Args, state: &mut RuntimeState) -> Response {
Err(error) => return PublicKeyInvalid(error),
};

if !state.data.originating_canisters.contains(&originating_canister) {
return OriginatingCanisterInvalid(originating_canister);
}

if state.data.requires_captcha(&originating_canister) {
let Some(attempt) = args.challenge_attempt else {
return ChallengeRequired;
Expand Down
1 change: 1 addition & 0 deletions backend/integration_tests/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ fn install_canisters(env: &mut PocketIc, controller: Principal) -> CanisterIds {
governance_principals: vec![controller],
user_index_canister_id,
cycles_dispenser_canister_id,
originating_canisters: vec![NNS_INTERNET_IDENTITY_CANISTER_ID, sign_in_with_email_canister_id],
skip_captcha_whitelist: vec![NNS_INTERNET_IDENTITY_CANISTER_ID, sign_in_with_email_canister_id],
ic_root_key: env.root_key().unwrap(),
wasm_version: BuildVersion::min(),
Expand Down
6 changes: 6 additions & 0 deletions backend/tools/canister_installer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ async fn install_service_canisters_impl(
governance_principals: vec![principal],
user_index_canister_id: canister_ids.user_index,
cycles_dispenser_canister_id: canister_ids.cycles_dispenser,
originating_canisters: vec![
canister_ids.nns_internet_identity,
canister_ids.sign_in_with_email,
canister_ids.sign_in_with_ethereum,
canister_ids.sign_in_with_solana,
],
skip_captcha_whitelist: vec![canister_ids.nns_internet_identity, canister_ids.sign_in_with_email],
ic_root_key: agent.read_root_key(),
wasm_version: version,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const idlFactory = ({ IDL }) => {
});
const CreateIdentityResponse = IDL.Variant({
'AlreadyRegistered' : IDL.Null,
'OriginatingCanisterInvalid' : IDL.Principal,
'Success' : PrepareDelegationSuccess,
'ChallengeFailed' : IDL.Null,
'ChallengeRequired' : IDL.Null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface CreateIdentityArgs {
'challenge_attempt' : [] | [{ 'key' : number, 'chars' : string }],
}
export type CreateIdentityResponse = { 'AlreadyRegistered' : null } |
{ 'OriginatingCanisterInvalid' : Principal } |
{ 'Success' : PrepareDelegationSuccess } |
{ 'ChallengeFailed' : null } |
{ 'ChallengeRequired' : null } |
Expand Down
3 changes: 3 additions & 0 deletions frontend/openchat-agent/src/services/identity/mappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ export function createIdentityResponse(candid: ApiCreateIdentityResponse): Creat
if ("PublicKeyInvalid" in candid) {
return { kind: "public_key_invalid" };
}
if ("OriginatingCanisterInvalid" in candid) {
return { kind: "originating_canister_invalid" };
}
if ("ChallengeFailed" in candid) {
return { kind: "challenge_failed" };
}
Expand Down
6 changes: 4 additions & 2 deletions frontend/openchat-shared/src/domain/identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export type CreateIdentityResponse =
| { kind: "already_registered" }
| { kind: "challenge_failed" }
| { kind: "challenge_required" }
| { kind: "public_key_invalid" };
| { kind: "public_key_invalid" }
| { kind: "originating_canister_invalid" };

export type CheckAuthPrincipalResponse = { kind: "success" } | { kind: "not_found" };

Expand Down Expand Up @@ -65,7 +66,8 @@ export type CreateOpenChatIdentityError =
| "already_registered"
| "challenge_failed"
| "challenge_required"
| "public_key_invalid";
| "public_key_invalid"
| "originating_canister_invalid";

export type GenerateChallengeResponse =
| { kind: "throttled" }
Expand Down

0 comments on commit ee21557

Please sign in to comment.