From 478d999b5ce2690c5cada177ef0a5d3b2e036e54 Mon Sep 17 00:00:00 2001 From: dastansam Date: Mon, 22 Apr 2024 23:27:19 +0200 Subject: [PATCH 1/3] Introduce certificate revocation list --- domains/pallets/auto-id/src/lib.rs | 87 +++++++++++++++------------- domains/pallets/auto-id/src/tests.rs | 30 +++++++--- 2 files changed, 69 insertions(+), 48 deletions(-) diff --git a/domains/pallets/auto-id/src/lib.rs b/domains/pallets/auto-id/src/lib.rs index 717a1cefd5..8f6d26ee9a 100644 --- a/domains/pallets/auto-id/src/lib.rs +++ b/domains/pallets/auto-id/src/lib.rs @@ -42,6 +42,9 @@ use subspace_runtime_primitives::Moment; /// Unique AutoId identifier. pub type Identifier = H256; +/// Serial issued by the subject. +pub type Serial = U256; + /// X509 certificate. #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] pub struct X509Certificate { @@ -59,9 +62,7 @@ pub struct X509Certificate { pub raw: DerVec, /// A list of all certificate serials issues by the subject. /// Serial of root certificate is included as well. - pub issued_serials: BTreeSet, - /// Signifies if the certificate is revoked. - pub revoked: bool, + pub issued_serials: BTreeSet, /// Certificate action nonce. pub nonce: U256, } @@ -108,15 +109,37 @@ impl Certificate { } } - fn revoke(&mut self) { + fn revoke(&self) { match self { - Certificate::X509(cert) => cert.revoked = true, + Certificate::X509(cert) => { + if let Some(issuer_id) = cert.issuer_id { + CertificateRevocationList::::insert(issuer_id, cert.serial); + } else { + // Root certificate + CertificateRevocationList::::insert(self.derive_identifier(), cert.serial); + } + } } } - fn is_revoked(&self) -> bool { - match self { - Certificate::X509(cert) => cert.revoked, + /// Deterministically derives an identifier from the certificate. + /// + /// The identifier is derived by hashing the subject common name of the certificate. + /// If the certificate is a leaf certificate, the issuer identifier is combined with the subject common name. + fn derive_identifier(&self) -> Identifier { + match &self { + Certificate::X509(cert) => { + if let Some(issuer_id) = cert.issuer_id { + let mut data = issuer_id.to_fixed_bytes().to_vec(); + + data.extend_from_slice(cert.subject_common_name.as_ref()); + + blake2_256(&data).into() + } else { + // Root certificate + blake2_256(cert.subject_common_name.as_ref()).into() + } + } } } @@ -146,29 +169,6 @@ pub struct AutoId { pub certificate: Certificate, } -impl AutoId { - /// Deterministically derives an identifier for the `AutoId`. - /// - /// The identifier is derived by hashing the subject common name of the certificate. - /// If the certificate is a leaf certificate, the issuer identifier is combined with the subject common name. - fn derive_identifier(&self) -> Identifier { - match &self.certificate { - Certificate::X509(cert) => { - if let Some(issuer_id) = cert.issuer_id { - let mut data = issuer_id.to_fixed_bytes().to_vec(); - - data.extend_from_slice(cert.subject_common_name.as_ref()); - - blake2_256(&data).into() - } else { - // Root certificate - blake2_256(cert.subject_common_name.as_ref()).into() - } - } - } - } -} - /// Type holds X509 certificate details used to register an AutoId. #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] pub enum RegisterAutoIdX509 { @@ -215,7 +215,7 @@ pub struct CertificateAction { #[frame_support::pallet] mod pallet { - use crate::{AutoId, Identifier, RegisterAutoId, Signature}; + use crate::{AutoId, Identifier, RegisterAutoId, Serial, Signature}; use frame_support::pallet_prelude::*; use frame_support::traits::Time; use frame_system::pallet_prelude::*; @@ -234,6 +234,14 @@ mod pallet { #[pallet::storage] pub(super) type AutoIds = StorageMap<_, Identity, Identifier, AutoId, OptionQuery>; + /// Stores list of revoked certificates. + /// + /// It maps the unique identifier to the serial number of the certificate. Before accepting + /// the certificate, external entities should check if the certificate or its issuer has been revoked. + #[pallet::storage] + pub(super) type CertificateRevocationList = + StorageMap<_, Identity, Identifier, Serial, OptionQuery>; + #[pallet::error] pub enum Error { /// Issuer auto id does not exist. @@ -248,6 +256,8 @@ mod pallet { ExpiredCertificate, /// Certificate revoked. CertificateRevoked, + /// Certificate already revoked. + CertificateAlreadyRevoked, /// Nonce overflow. NonceOverflow, /// Identifier already exists. @@ -340,7 +350,6 @@ impl Pallet { validity: tbs_certificate.validity, raw: certificate, issued_serials: BTreeSet::from([tbs_certificate.serial]), - revoked: false, nonce: U256::zero(), }) } @@ -361,7 +370,7 @@ impl Pallet { ); ensure!( - !issuer_auto_id.certificate.is_revoked(), + !CertificateRevocationList::::contains_key(issuer_id), Error::::CertificateRevoked ); @@ -394,15 +403,14 @@ impl Pallet { validity: tbs_certificate.validity, raw: certificate, issued_serials: BTreeSet::from([tbs_certificate.serial]), - revoked: false, nonce: U256::zero(), }) } }, }; + let auto_id_identifier = certificate.derive_identifier(); let auto_id = AutoId { certificate }; - let auto_id_identifier = auto_id.derive_identifier(); ensure!( !AutoIds::::contains_key(auto_id_identifier), @@ -449,12 +457,13 @@ impl Pallet { }, signature, )?; + ensure!( - !auto_id.certificate.is_revoked(), - Error::::CertificateRevoked + !CertificateRevocationList::::contains_key(auto_id_identifier), + Error::::CertificateAlreadyRevoked ); - auto_id.certificate.revoke(); + auto_id.certificate.revoke::(); auto_id.certificate.inc_nonce::()?; // TODO: revoke all the issued leaf certificates if this is an issuer certificate. diff --git a/domains/pallets/auto-id/src/tests.rs b/domains/pallets/auto-id/src/tests.rs index cc47b1b0ff..8383eec288 100644 --- a/domains/pallets/auto-id/src/tests.rs +++ b/domains/pallets/auto-id/src/tests.rs @@ -1,7 +1,8 @@ use crate::pallet::AutoIds; use crate::{ - self as pallet_auto_id, Certificate, CertificateAction, CertificateActionType, Error, - Identifier, Pallet, RegisterAutoId, RegisterAutoIdX509, Signature, X509Certificate, + self as pallet_auto_id, Certificate, CertificateAction, CertificateActionType, + CertificateRevocationList, Error, Identifier, Pallet, RegisterAutoId, RegisterAutoIdX509, + Signature, X509Certificate, }; use alloc::collections::BTreeSet; use codec::Encode; @@ -281,7 +282,9 @@ fn test_revoke_certificate() { new_test_ext().execute_with(|| { let auto_id_identifier = register_issuer_auto_id(); let auto_id = AutoIds::::get(auto_id_identifier).unwrap(); - assert!(!auto_id.certificate.is_revoked()); + assert!(!CertificateRevocationList::::contains_key( + auto_id_identifier + )); let signing_data = CertificateAction { id: auto_id_identifier, nonce: auto_id.certificate.nonce(), @@ -295,7 +298,9 @@ fn test_revoke_certificate() { ) .unwrap(); let auto_id = AutoIds::::get(auto_id_identifier).unwrap(); - assert!(auto_id.certificate.is_revoked()); + assert!(CertificateRevocationList::::contains_key( + auto_id_identifier + )); assert_eq!(auto_id.certificate.nonce(), U256::one()); }) } @@ -334,7 +339,6 @@ fn test_auto_id_identifier_is_deterministic() { }, subject_public_key_info: DerVec::from(vec![1, 2, 3, 4]), nonce: U256::zero(), - revoked: false, serial: U256::zero(), raw: vec![1, 2, 3, 4].into(), issued_serials: BTreeSet::new(), @@ -344,13 +348,16 @@ fn test_auto_id_identifier_is_deterministic() { let expected_auto_id_identifier = "0x3170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314"; assert_eq!( - to_hex(&auto_id.derive_identifier().to_fixed_bytes(), true), + to_hex( + &auto_id.certificate.derive_identifier().to_fixed_bytes(), + true + ), expected_auto_id_identifier ); let auto_id_child = crate::AutoId { certificate: Certificate::X509(X509Certificate { - issuer_id: Some(auto_id.derive_identifier()), + issuer_id: Some(auto_id.certificate.derive_identifier()), subject_common_name: vec![0].into(), validity: Validity { not_before: 0, @@ -358,7 +365,6 @@ fn test_auto_id_identifier_is_deterministic() { }, subject_public_key_info: DerVec::from(vec![1, 2, 3, 4]), nonce: U256::zero(), - revoked: false, serial: U256::zero(), raw: vec![1, 2, 3, 4].into(), issued_serials: BTreeSet::new(), @@ -368,7 +374,13 @@ fn test_auto_id_identifier_is_deterministic() { let expected_auto_id_child_identifier = "0x1f6c133e7bca8c7714c5c9df36562e5cd51304530cc85e583351167bb75e072f"; assert_eq!( - to_hex(&auto_id_child.derive_identifier().to_fixed_bytes(), true), + to_hex( + &auto_id_child + .certificate + .derive_identifier() + .to_fixed_bytes(), + true + ), expected_auto_id_child_identifier ); }) From f2d0318bb8b219c76cd49224bbb8b28911f612c9 Mon Sep 17 00:00:00 2001 From: dastansam Date: Fri, 26 Apr 2024 12:45:03 +0200 Subject: [PATCH 2/3] Store list of revoked serials, only issuer can revoke --- domains/pallets/auto-id/res/issuer.cert.der | Bin 981 -> 1041 bytes domains/pallets/auto-id/res/leaf.cert.der | Bin 1009 -> 967 bytes domains/pallets/auto-id/res/private.leaf.pem | 28 ++++++ domains/pallets/auto-id/src/lib.rs | 83 +++++++++------- domains/pallets/auto-id/src/tests.rs | 96 +++++++++++++++++-- 5 files changed, 165 insertions(+), 42 deletions(-) create mode 100644 domains/pallets/auto-id/res/private.leaf.pem diff --git a/domains/pallets/auto-id/res/issuer.cert.der b/domains/pallets/auto-id/res/issuer.cert.der index c10a4217f5ae0c9b4bf12c334e4f3f6fa3e692c6..a74a6bbe3881c403944d71251351da0dd2ee7157 100644 GIT binary patch delta 430 zcmcc0K9NJ&poxXopo!`00%j&gCMFT(`?7xy@459N&}5I(J?H<=-DdDEoT$7~!ocr02hNBrhOBmntwM#7et2vWl;`QiVyxaIHjyyi{-saxH5?AKUW^WFp`8n*~ zKGA-n9&gL^OhK*gtIPSec|YxQZn@cZ>VNkaol9Gv%vFn=ST^V5%`?|>%8$A|%{z8B zrr^x}d2^mPbguD=>)>9n(f#mcjjQ!vk1Fl|cu19tUu@ze2}@zsKC!^{$ww9hF4JHV z-MeD8mrqEI?w+|mznt{XrARN^WqfG4NgB7_Nw1WHWrq_@`-OdF1B(j9%)U9ZHYps- zX1|#6X2W}l$o1@E-S0Il<}1ONa4 delta 385 zcmV-{0e=3G2-OE8FoFZoFoFWPpaTK{0s<5U?TY!TGWT6HDeLAw-5^PdB7ZZHBdHiO zGB7bSGcq(ZS{Ds6H83+WFflVTGBh-?9R>qc9S#H*1QbUTHD?pFcGPds z>h0axjwTEfSP?J{1_>&LNQUaG7xj338XZ;yWFKL{_u zFoP>0MNk9vnW<7fOwLF@?Z14yh{D~qf$Ks!#+E!}6MwVD3Q&(Nlm7D2Kf!5`qN<#K ze$NHfQn#&%E5Y6h4s->!m%rT^Dc@gsJ!|3V>u{?_sHkr6OLFOHF|k#LNvVv)=8#*hlM}q2ZdO%f0#x33?W>jHMdBPYHJ)a9buPlQnlc%Q z7S3$p^c08&_*ZrSf9*G})$HW7USFs<2gc)UWar^7tF-OEDPrjLl-E)LuUN*>`TB$B f&P$?V<<>dA(B(nYg>(O(CxXw-GYhwaM}t=edlatP diff --git a/domains/pallets/auto-id/res/leaf.cert.der b/domains/pallets/auto-id/res/leaf.cert.der index 934e3bba9eaffd9693cc83f0e2fad27862b29779..a7e7a97137a10cd0368f284a1c18e19061946b8b 100644 GIT binary patch delta 737 zcmV<70v`SG2ge5^FoFZaFoFWBpaTK{0s<6S_DToqs|WIC{&fQQ_4sAnDXi_0BdLEh zGBz&b98fa zb1@Mx4+aBO1Op5RbY*jNE@N+PFoKhH0Uv*@FhTLNAk#8gIWEC%DP#|X@S($^iBmiX zNQk#2^B?r1a$ljZ8?m?Dn}>-75BL!iXgQ94H|!DMjpse@iNzcB09EqFfI1tLn?YRP z`(paX)@nAkBwZboZ_dB+dJPPht5U*Wu2-9%9QKAxcx!=S1SiGC7m#z{rya`$Chvdm z7{Atx#HYAd`D(<;1`O3V4jIg*@H^MXfLlJq5&xGWk$ikL?Ij4Xn{+9a~euu3AXQIOjeMsFDj+49sHGes@vos9!oQG~Y8|Z9VwhvWv`7A#upo^esO`CsU9*5qt zJq{J?R>x_Ba?cG3srzl$dd>b8*vU(I28pbx7N;FJA3co@27#ft^S~)7Z1`&yZcP%eQA0j1)?-t&jgnu{r5B{McO9lCS zvvoP1A3Cg0&Ks_NoGv$3*WtlCI;eB93tej;WG!I(CnvioB(JjWhEOY4q8nZH1Vv2% z9JC5HQKpTPXpohmA=rp5!@Alhk3EtYRke1nl<11>1+uh%_PbEvadQtE$1A9_1C=!> Tf_lu@h(ADm=(%{&PXgd7cN{pb delta 754 zcmV&LNQUOU8jN$^-|1^Q}s8EIZFYYH}s)1OUvJr{$nr)gUZ zkBX{HNU3r&H>#rp`y6V2TJrCb-E~P#A5^5k*4q5mL_)6D0NXk&BW}9=@q@n))?ty; z`|L#;j@Lu|e}2k@0G8@Obaxn4g>VEkNu2iA=DZ4sV3;t+{Bm}a2++Qf1d!N-?PMb6 z-vGzSek;wJ5QIYzcvHSrN$=QYc8(`bHDctrMK}t5UDPN_gVe7o*fRnH0RRD`LNGuu z9R>qc9S#H*1QgP`zrr0!5u1p2UX1qc2Kg=~z2tAA1K(8`$9tz-PTlQ@+(zl=`J|3CSYHnc{UXPH0U{0OqmWq-h zu6K>>sitIZt54Xj-TypjuX9_ZSj?-IEvZ{KqSUNCkNuGJbRrgQCQmCFr%ieS%@qVA z3fBSv5cYqTb+a^9IUzzHt{(2_K0u_3mQJHPS3rn~T6-P=Fp&a{>=B9)>ykWsN!Zgm zqr6kW?A^3Liu*}#>>K-j_M6qppU_kUSQY0+K}STR2n0Buo{uiA>(;YmXQ_W*>}}aY kU@ir-QpDoXBIe2%Oce?|9Det*jAR~-RDeZjN!J3FO-<@Hi2wiq diff --git a/domains/pallets/auto-id/res/private.leaf.pem b/domains/pallets/auto-id/res/private.leaf.pem new file mode 100644 index 0000000000..b21addc9af --- /dev/null +++ b/domains/pallets/auto-id/res/private.leaf.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEugIBADANBgkqhkiG9w0BAQEFAASCBKQwggSgAgEAAoIBAQCtMEHxsyDTMlk5 +LsFtKWQPhPChw6KJUzwISIi3JPMf9KNyX6GvG7G33ZuHiQUP+BETaDmOfjfsEeCN +5z3vicUb9QBV8saAOhuUm0Fc3/ti+sfWaja2JF0dk2/Ov/J6DQyXq1LCX65Xm58c +9oZMeGuBYQQnxcUXkHPgpx3LBSbv7xi/1ozEp7hX+WrEyQYM1TcOGcym8DvXx4Be +IvUJh38W1wsFIimyVs6rjKzOn2zpYbW7I82cza8ppWjch+sVduLPhOSfyMKWfzmn +gCX+lvsYfoetAWeiwwp9SN1Qaup44ky5Hyupw7ZMola4J0SsdqrYR+KABVdUSFEZ +nIrYZMNxAgMBAAECgf9fvpxEihFNW494BloThnH8sKWQVsk2aTg/6ktx2B8fBGAe +/nhra5Xbl1nPDRSb1Mh/eULKYGCYYdf0m003pzKfc9omVJg+IMVJTVK//oV0j1qH +qAXeSLwxsvRAfCR7uKzhEBXwpnTfXaJLEo06qzunUizyz/h0QFJ6PJQKMA76MeC9 +duVHz13TiuC5IYfHbvGuSlDVNAYFv2taglun2JY6CM57VOflIYioShEOCjMWje1q +t+7d8WFcJnuBQc3cd4mw3CR3g/2HSoUmb1Bn0pHQa0IvgschPCcFGXAwJQ6Cf/P/ +U5BHTVkeeconjMYvDdyy+YvhBXo0UuRxxZbDBgECgYEA31kCvtvDFk0uCKZ2fB2H +qlwlA6V2Jki8edFsVHRD0DulKLLXnsy3t1UZA9TwfU20RYvDoVphbuaOHaONCtpz +ACkEZMtnOaorZSSnICbhnzuU7BPX/RNWa/34khbjy8YVCQ9h+Jo2l9GVAweZ8iBv +SO6M0hqGclSe/KnTnTkJAfECgYEAxoH+awk7FysqlKMN/2ugDQQitzJ7ZjJAcO/T +b+YKnjGmF3Onv/+IG2Nu++kpOaOoyZ+cb++kVGpT0rRmy+sjHpvvMJrD5GP0eiAl +nHckGCLf7gMx0rcyBmVVZOHzaHsHHRRoH9PzsSN91zKGhIU0XpdFjF1vLSUAblSO +br4OWYECgYBb+IBb7YzxIwkAwONripFyAo2vabQ0YaFTHHzabiH6noUNNE/78Vr5 +oI4zeL0rLBM+zCXbzKbwjvoYlF+hB4FxoHJRuzyfj0ZdWPGFGN2xv0w8xpMbgJoG +0EdKiSh2ofPJjk8Omxo9/Cy7Wab4AIky5CCS6B9S9yuc6aXdST4/UQKBgA7MpkEo +oQUrLLOELIj8ZyRRSJ1L4DNQT8mbt7HB/syoeu+IqdsAnA8erKmPSomHkA/oHGuj +/CZm/vTYikltsGKZ0Y1YHH6sjQ+F0ggGQeSixPsjtdU13z7m0yUAS3tgoLkkSlcF +IEf2k2010R2UKMFcmczLMny1I4EWQMA03zEBAoGAWSzHaWtK5mbzM90B2CxFcFHH +tdCPxKspe0bnGcZkFSHYHVUn6Mgbe3szA2QN9uftBLmXP+FXvl7+woUExZaYZZzW +njyKn1VP9dxmme2NSx3PoIN1N8QCKD4xXqlI+U1nnhBaYuM7ieIzr0hID9rrZZ10 +nCVF4arGqOL8Crkkqmg= +-----END PRIVATE KEY----- diff --git a/domains/pallets/auto-id/src/lib.rs b/domains/pallets/auto-id/src/lib.rs index 8f6d26ee9a..c0fabb9cf2 100644 --- a/domains/pallets/auto-id/src/lib.rs +++ b/domains/pallets/auto-id/src/lib.rs @@ -102,23 +102,22 @@ impl Certificate { } } - /// Checks if the certificate is valid at this time. - pub(crate) fn is_valid_at(&self, time: Moment) -> bool { + fn issuer_id(&self) -> Option { match self { - Certificate::X509(cert) => cert.validity.is_valid_at(time), + Certificate::X509(cert) => cert.issuer_id, } } - fn revoke(&self) { + fn serial(&self) -> U256 { match self { - Certificate::X509(cert) => { - if let Some(issuer_id) = cert.issuer_id { - CertificateRevocationList::::insert(issuer_id, cert.serial); - } else { - // Root certificate - CertificateRevocationList::::insert(self.derive_identifier(), cert.serial); - } - } + Certificate::X509(cert) => cert.serial, + } + } + + /// Checks if the certificate is valid at this time. + pub(crate) fn is_valid_at(&self, time: Moment) -> bool { + match self { + Certificate::X509(cert) => cert.validity.is_valid_at(time), } } @@ -208,8 +207,11 @@ pub enum CertificateActionType { /// Signing data used to verify the certificate action. #[derive(Debug, Decode, Encode, TypeInfo, PartialEq, Eq, Clone)] pub struct CertificateAction { + /// On which AutoId the action is taken. pub id: Identifier, + /// Current nonce of the certificate. pub nonce: U256, + /// Type of action taken. pub action_type: CertificateActionType, } @@ -219,6 +221,7 @@ mod pallet { use frame_support::pallet_prelude::*; use frame_support::traits::Time; use frame_system::pallet_prelude::*; + use scale_info::prelude::collections::BTreeSet; #[pallet::config] pub trait Config: frame_system::Config { @@ -236,16 +239,18 @@ mod pallet { /// Stores list of revoked certificates. /// - /// It maps the unique identifier to the serial number of the certificate. Before accepting + /// It maps the issuer's identifier to the list of revoked serial numbers of certificates. Before accepting /// the certificate, external entities should check if the certificate or its issuer has been revoked. #[pallet::storage] pub(super) type CertificateRevocationList = - StorageMap<_, Identity, Identifier, Serial, OptionQuery>; + StorageMap<_, Identity, Identifier, BTreeSet, OptionQuery>; #[pallet::error] pub enum Error { /// Issuer auto id does not exist. UnknownIssuer, + /// Unknown AutoId identifier. + UnknownAutoId, /// Certificate is invalid, InvalidCertificate, /// Invalid signature. @@ -288,8 +293,10 @@ mod pallet { } /// Revokes a certificate associated with given AutoId. - #[pallet::call_index(1)] + /// + /// The signature is verified against the issuer's public key. // TODO: benchmark + #[pallet::call_index(1)] #[pallet::weight({10_000})] pub fn revoke_certificate( origin: OriginFor, @@ -369,11 +376,6 @@ impl Pallet { Error::::ExpiredCertificate ); - ensure!( - !CertificateRevocationList::::contains_key(issuer_id), - Error::::CertificateRevoked - ); - let tbs_certificate = decode_tbs_certificate(certificate.clone()) .ok_or(Error::::InvalidCertificate)?; @@ -447,27 +449,44 @@ impl Pallet { auto_id_identifier: Identifier, signature: Signature, ) -> DispatchResult { - let mut auto_id = AutoIds::::get(auto_id_identifier).ok_or(Error::::UnknownIssuer)?; + let auto_id = AutoIds::::get(auto_id_identifier).ok_or(Error::::UnknownAutoId)?; + + let issuer_id = match auto_id.certificate.issuer_id() { + Some(issuer_id) => issuer_id, + // self revoke + None => auto_id_identifier, + }; + + let mut issuer_auto_id = AutoIds::::get(issuer_id).ok_or(Error::::UnknownIssuer)?; + + ensure!( + !CertificateRevocationList::::get(issuer_id).map_or(false, |serials| serials + .iter() + .filter(|serial| *serial == &auto_id.certificate.serial() + || *serial == &issuer_auto_id.certificate.serial()) + .count() + > 0), + Error::::CertificateAlreadyRevoked + ); + Self::do_verify_signature( - &auto_id, + &issuer_auto_id, CertificateAction { id: auto_id_identifier, - nonce: auto_id.certificate.nonce(), + nonce: issuer_auto_id.certificate.nonce(), action_type: CertificateActionType::RevokeCertificate, }, signature, )?; - ensure!( - !CertificateRevocationList::::contains_key(auto_id_identifier), - Error::::CertificateAlreadyRevoked - ); + CertificateRevocationList::::mutate(issuer_id, |serials| { + serials + .get_or_insert_with(BTreeSet::new) + .insert(auto_id.certificate.serial()); + }); - auto_id.certificate.revoke::(); - auto_id.certificate.inc_nonce::()?; - - // TODO: revoke all the issued leaf certificates if this is an issuer certificate. - AutoIds::::insert(auto_id_identifier, auto_id); + issuer_auto_id.certificate.inc_nonce::()?; + AutoIds::::insert(issuer_id, issuer_auto_id); Self::deposit_event(Event::::CertificateRevoked(auto_id_identifier)); Ok(()) diff --git a/domains/pallets/auto-id/src/tests.rs b/domains/pallets/auto-id/src/tests.rs index 8383eec288..182c34ec6b 100644 --- a/domains/pallets/auto-id/src/tests.rs +++ b/domains/pallets/auto-id/src/tests.rs @@ -37,8 +37,8 @@ impl Time for MockTime { type Moment = Moment; fn now() -> Self::Moment { - // valid block time for testing certs - 1_711_367_658_200 + // July 1, 2024, in milliseconds since Epoch + 1_719_792_000_000 } } @@ -215,8 +215,12 @@ fn register_leaf_auto_id(issuer_auto_id: Identifier) -> Identifier { auto_id_identifier } -fn sign_preimage(data: Vec) -> Signature { - let priv_key_pem = include_str!("../res/private.issuer.pem"); +fn sign_preimage(data: Vec, issuer: bool) -> Signature { + let priv_key_pem = if issuer { + include_str!("../res/private.issuer.pem") + } else { + include_str!("../res/private.leaf.pem") + }; let priv_key_der = parse(priv_key_pem).unwrap().contents().to_vec(); let rsa_key_pair = RsaKeyPair::from_pkcs8(&priv_key_der).unwrap(); let mut signature = vec![0; rsa_key_pair.public().modulus_len()]; @@ -278,7 +282,7 @@ fn test_register_issuer_auto_id_duplicate() { } #[test] -fn test_revoke_certificate() { +fn test_self_revoke_certificate() { new_test_ext().execute_with(|| { let auto_id_identifier = register_issuer_auto_id(); let auto_id = AutoIds::::get(auto_id_identifier).unwrap(); @@ -290,7 +294,7 @@ fn test_revoke_certificate() { nonce: auto_id.certificate.nonce(), action_type: CertificateActionType::RevokeCertificate, }; - let signature = sign_preimage(signing_data.encode()); + let signature = sign_preimage(signing_data.encode(), true); Pallet::::revoke_certificate( RawOrigin::Signed(1).into(), auto_id_identifier, @@ -298,10 +302,82 @@ fn test_revoke_certificate() { ) .unwrap(); let auto_id = AutoIds::::get(auto_id_identifier).unwrap(); - assert!(CertificateRevocationList::::contains_key( - auto_id_identifier - )); + assert!(CertificateRevocationList::::get(auto_id_identifier) + .unwrap() + .contains(&auto_id.certificate.serial())); + assert_eq!(auto_id.certificate.nonce(), U256::one()); + + // try revoking leaf certificate when issuer is revoked + let leaf_id = register_leaf_auto_id(auto_id_identifier); + let leaf_auto_id = AutoIds::::get(leaf_id).unwrap(); + let signing_data = CertificateAction { + id: leaf_id, + nonce: leaf_auto_id.certificate.nonce(), + action_type: CertificateActionType::RevokeCertificate, + }; + let signature = sign_preimage(signing_data.encode(), false); + + assert_noop!( + Pallet::::revoke_certificate(RawOrigin::Signed(1).into(), leaf_id, signature), + Error::::CertificateAlreadyRevoked + ); + }) +} + +#[test] +fn test_revoke_leaf_certificate() { + new_test_ext().execute_with(|| { + let issuer_id = register_issuer_auto_id(); + let leaf_id = register_leaf_auto_id(issuer_id); + let issuer_auto_id = AutoIds::::get(issuer_id).unwrap(); + let leaf_auto_id = AutoIds::::get(leaf_id).unwrap(); + + assert!(!CertificateRevocationList::::contains_key(issuer_id)); + + // leaf tries to revoke itself + let signing_data = CertificateAction { + id: leaf_id, + nonce: issuer_auto_id.certificate.nonce(), + action_type: CertificateActionType::RevokeCertificate, + }; + + // sign with leaf's private key + let signature = sign_preimage(signing_data.encode(), false); + + // leaf tries to revoke itself + assert_noop!( + Pallet::::revoke_certificate(RawOrigin::Signed(1).into(), leaf_id, signature), + Error::::InvalidSignature + ); + + // now issuer revokes leaf + let signing_data = CertificateAction { + id: leaf_id, + nonce: issuer_auto_id.certificate.nonce(), + action_type: CertificateActionType::RevokeCertificate, + }; + let signature = sign_preimage(signing_data.encode(), true); + + Pallet::::revoke_certificate(RawOrigin::Signed(1).into(), leaf_id, signature) + .unwrap(); + + assert!(CertificateRevocationList::::get(issuer_id) + .unwrap() + .contains(&leaf_auto_id.certificate.serial())); + + // revoking the same certificate again should fail + let signing_data = CertificateAction { + id: leaf_id, + nonce: issuer_auto_id.certificate.nonce(), + action_type: CertificateActionType::RevokeCertificate, + }; + let signature = sign_preimage(signing_data.encode(), true); + + assert_noop!( + Pallet::::revoke_certificate(RawOrigin::Signed(1).into(), leaf_id, signature), + Error::::CertificateAlreadyRevoked + ); }) } @@ -315,7 +391,7 @@ fn test_deactivate_auto_id() { nonce: auto_id.certificate.nonce(), action_type: CertificateActionType::DeactivateAutoId, }; - let signature = sign_preimage(signing_data.encode()); + let signature = sign_preimage(signing_data.encode(), true); Pallet::::deactivate_auto_id( RawOrigin::Signed(1).into(), auto_id_identifier, From 76c3a5f2842432793d4912c2ced7047774d43d8a Mon Sep 17 00:00:00 2001 From: dastansam Date: Wed, 1 May 2024 00:00:52 +0200 Subject: [PATCH 3/3] Resolve comments --- domains/pallets/auto-id/src/lib.rs | 36 ++++++++++++++++++---------- domains/pallets/auto-id/src/tests.rs | 25 ++++++++++--------- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/domains/pallets/auto-id/src/lib.rs b/domains/pallets/auto-id/src/lib.rs index c0fabb9cf2..220f1dc14e 100644 --- a/domains/pallets/auto-id/src/lib.rs +++ b/domains/pallets/auto-id/src/lib.rs @@ -108,7 +108,7 @@ impl Certificate { } } - fn serial(&self) -> U256 { + fn serial(&self) -> Serial { match self { Certificate::X509(cert) => cert.serial, } @@ -217,11 +217,11 @@ pub struct CertificateAction { #[frame_support::pallet] mod pallet { + use super::*; use crate::{AutoId, Identifier, RegisterAutoId, Serial, Signature}; use frame_support::pallet_prelude::*; use frame_support::traits::Time; use frame_system::pallet_prelude::*; - use scale_info::prelude::collections::BTreeSet; #[pallet::config] pub trait Config: frame_system::Config { @@ -391,6 +391,16 @@ impl Pallet { Error::::ExpiredCertificate ); + ensure!( + !CertificateRevocationList::::get(issuer_id).map_or(false, |serials| { + serials.iter().any(|s| { + *s == issuer_auto_id.certificate.serial() + || *s == tbs_certificate.serial + }) + }), + Error::::CertificateRevoked + ); + issuer_auto_id .certificate .issue_certificate_serial::(tbs_certificate.serial)?; @@ -451,21 +461,21 @@ impl Pallet { ) -> DispatchResult { let auto_id = AutoIds::::get(auto_id_identifier).ok_or(Error::::UnknownAutoId)?; - let issuer_id = match auto_id.certificate.issuer_id() { - Some(issuer_id) => issuer_id, + let (issuer_id, mut issuer_auto_id) = match auto_id.certificate.issuer_id() { + Some(issuer_id) => ( + issuer_id, + AutoIds::::get(issuer_id).ok_or(Error::::UnknownIssuer)?, + ), // self revoke - None => auto_id_identifier, + None => (auto_id_identifier, auto_id.clone()), }; - let mut issuer_auto_id = AutoIds::::get(issuer_id).ok_or(Error::::UnknownIssuer)?; - ensure!( - !CertificateRevocationList::::get(issuer_id).map_or(false, |serials| serials - .iter() - .filter(|serial| *serial == &auto_id.certificate.serial() - || *serial == &issuer_auto_id.certificate.serial()) - .count() - > 0), + !CertificateRevocationList::::get(issuer_id).map_or(false, |serials| { + serials.iter().any(|s| { + *s == auto_id.certificate.serial() || *s == issuer_auto_id.certificate.serial() + }) + }), Error::::CertificateAlreadyRevoked ); diff --git a/domains/pallets/auto-id/src/tests.rs b/domains/pallets/auto-id/src/tests.rs index 182c34ec6b..3b5be186f2 100644 --- a/domains/pallets/auto-id/src/tests.rs +++ b/domains/pallets/auto-id/src/tests.rs @@ -308,19 +308,22 @@ fn test_self_revoke_certificate() { assert_eq!(auto_id.certificate.nonce(), U256::one()); - // try revoking leaf certificate when issuer is revoked - let leaf_id = register_leaf_auto_id(auto_id_identifier); - let leaf_auto_id = AutoIds::::get(leaf_id).unwrap(); - let signing_data = CertificateAction { - id: leaf_id, - nonce: leaf_auto_id.certificate.nonce(), - action_type: CertificateActionType::RevokeCertificate, - }; - let signature = sign_preimage(signing_data.encode(), false); + // try issuing leaf certificate when issuer is revoked + let cert = include_bytes!("../res/leaf.cert.der").to_vec(); + let (_, cert) = x509_parser::certificate::X509Certificate::from_der(&cert).unwrap(); + let _ = identifier_from_x509_cert(Some(auto_id_identifier), &cert); assert_noop!( - Pallet::::revoke_certificate(RawOrigin::Signed(1).into(), leaf_id, signature), - Error::::CertificateAlreadyRevoked + Pallet::::register_auto_id( + RawOrigin::Signed(1).into(), + RegisterAutoId::X509(RegisterAutoIdX509::Leaf { + issuer_id: auto_id_identifier, + certificate: cert.tbs_certificate.as_ref().to_vec().into(), + signature_algorithm: algorithm_to_der(cert.signature_algorithm.clone()), + signature: cert.signature_value.as_ref().to_vec(), + }), + ), + Error::::CertificateRevoked, ); }) }