Skip to content

Commit 8f39258

Browse files
authored
feat: More validation for OpenID credential issuers in attribute sharing (dfinity#3606)
<!-- Make sure you talk to us before submitting changes. See CONTRIBUTING.md. --> # Motivation Fail early by pre-validating OpenID credential issuers used in attribute sharing scopes. # Changes * Adds validation function for OpenID credential issuer identifiers requiring HTTPS, prohibiting query/fragment parameters, and enforcing a maximum length of 1024 characters # Tests * Updates all test cases to use valid HTTPS URLs for issuer identifiers * Adds extensive test coverage for validation edge cases including IPv6 addresses, length limits, and invalid formats
1 parent 8880d4c commit 8f39258

7 files changed

Lines changed: 318 additions & 69 deletions

File tree

-3.54 KB
Binary file not shown.
-4.53 KB
Binary file not shown.

src/frontend/src/lib/legacy/flows/dappsExplorer/dapps.json

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -324,18 +324,6 @@
324324
"oneLiner": "Your AI-powered companion for simplifying and streamlining the Motoko coding experience.",
325325
"logo": "motokopilot_logo.png"
326326
},
327-
{
328-
"name": "NOBLEBLOCKS",
329-
"website": "https://www.nobleblocks.com",
330-
"logo": "nobleblocks_logo.webp",
331-
"oneLiner": "A Community-Driven DeSci Project for Scientific Publishing"
332-
},
333-
{
334-
"name": "John Dao",
335-
"website": "https://johndao.gg",
336-
"logo": "john-dao_logo.webp",
337-
"oneLiner": "A Twitter/X account controlled by a DAO on the Internet Computer"
338-
},
339327
{
340328
"name": "aVa",
341329
"website": "https://ksayv-myaaa-aaaan-qedxq-cai.icp0.io",

src/internet_identity/src/attributes.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::{
2-
openid::{OpenIdCredential, OpenIdCredentialKey, OPENID_SESSION_DURATION_NS},
2+
openid::{OpenIdCredential, OpenIdCredentialKey},
33
state,
44
storage::{account::Account, anchor::Anchor},
5-
update_root_hash,
5+
update_root_hash, MINUTE_NS,
66
};
77
use ic_canister_sig_creation::signature_map::{CanisterSigInputs, SignatureMap};
88
use ic_representation_independent_hash::{representation_independent_hash, Value};
@@ -15,8 +15,12 @@ use internet_identity_interface::internet_identity::types::{
1515
};
1616
use std::collections::{BTreeMap, BTreeSet};
1717

18+
/// Domain separator for attribute certification signatures. Clients need this to verify signatures.
1819
const ATTRIBUTES_CERTIFICATION_DOMAIN: &[u8] = b"ii-request-attribute";
19-
const ATTRIBUTES_CERTIFICATION_SESSION_DURATION_NS: u64 = OPENID_SESSION_DURATION_NS;
20+
21+
/// Duration for which attribute certifications are valid. Does not strictly need to be the same
22+
/// as `OPENID_SESSION_DURATION_NS`, but for simplicity we keep them aligned for now.
23+
const ATTRIBUTES_CERTIFICATION_SESSION_DURATION_NS: u64 = 30 * MINUTE_NS;
2024

2125
fn expiration_timestamp_ns(issued_at_timestamp_ns: Timestamp) -> Timestamp {
2226
issued_at_timestamp_ns.saturating_add(ATTRIBUTES_CERTIFICATION_SESSION_DURATION_NS)
@@ -84,7 +88,7 @@ impl Anchor {
8488
/// Processes `requested_attributes` for all attribute scopes, prepares signatures for
8589
/// the (attribute_key, attribute_value) pairs that can be fulfilled for this (anchor, account).
8690
///
87-
/// Returns the list of attribute keys certified with expiry `now_timestamp_ns + OPENID_SESSION_DURATION_NS`.
91+
/// Returns the list of attribute keys certified with expiry `now_timestamp_ns + ATTRIBUTES_CERTIFICATION_SESSION_DURATION_NS`.
8892
pub fn prepare_attributes(
8993
&self,
9094
requested_attributes: BTreeMap<Option<AttributeScope>, BTreeSet<AttributeName>>,
@@ -133,6 +137,12 @@ impl Anchor {
133137
issuer: openid_credential.iss.clone(),
134138
});
135139
// E.g., {`email`, `name`}
140+
//
141+
// Why we do not include `sub` / `aud` into the keys of this map:
142+
// --------------------------------------------------------------
143+
// Currently, an anchor can only have a single iss linked once. The storage layer
144+
// allows for duplicate iss, but the implementation has been restricted to enforce
145+
// the one-to-one relationship.
136146
let attribute_names = requested_attributes.remove(&scope)?;
137147

138148
let mut attribute_keys: BTreeMap<AttributeName, AttributeKey> = attribute_names
@@ -188,7 +198,8 @@ impl OpenIdCredential {
188198
attributes: &Vec<Attribute>,
189199
now_timestamp_ns: Timestamp,
190200
) {
191-
let expiration_timestamp_ns = now_timestamp_ns.saturating_add(OPENID_SESSION_DURATION_NS);
201+
let expiration_timestamp_ns =
202+
now_timestamp_ns.saturating_add(ATTRIBUTES_CERTIFICATION_SESSION_DURATION_NS);
192203

193204
let seed = account.calculate_seed();
194205

src/internet_identity/src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1308,7 +1308,8 @@ mod attribute_sharing {
13081308
}
13091309
}
13101310

1311-
/// API for the attribute sharing mvp
1311+
/// Legacy API for the original attribute sharing MVP (VC-based).
1312+
/// The current attribute sharing protocol endpoints live in `mod attribute_sharing`.
13121313
mod attribute_sharing_old_vc {
13131314
use super::*;
13141315

src/internet_identity/tests/integration/attributes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
//! Tests related to prepare_delegation, get_delegation and get_principal II canister calls.
1+
//! Tests related to prepare_attributes and get_attributes II canister calls.
22
33
use canister_tests::api::internet_identity as api;
44
use canister_tests::framework::*;

0 commit comments

Comments
 (0)