From 4a8abe8eef6e00ec34ed1113b2feb567610dcd90 Mon Sep 17 00:00:00 2001 From: oggy-dfin <89794951+oggy-dfin@users.noreply.github.com> Date: Mon, 11 Dec 2023 09:47:56 +0100 Subject: [PATCH] Limit delegations to depth 1 (#500) --- ic-agent/src/agent/agent_error.rs | 4 ++ ic-agent/src/agent/agent_test.rs | 54 +++++++++++++++++- .../src/agent/agent_test/with_subnet_key.bin | Bin 0 -> 12168 bytes ic-agent/src/agent/mod.rs | 8 ++- 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 ic-agent/src/agent/agent_test/with_subnet_key.bin diff --git a/ic-agent/src/agent/agent_error.rs b/ic-agent/src/agent/agent_error.rs index ad0740c1..70477f05 100644 --- a/ic-agent/src/agent/agent_error.rs +++ b/ic-agent/src/agent/agent_error.rs @@ -110,6 +110,10 @@ pub enum AgentError { #[error("Certificate is stale (over {0:?}). Is the computer's clock synchronized?")] CertificateOutdated(Duration), + /// The certificate contained more than one delegation. + #[error("The certificate contained more than one delegation")] + CertificateHasTooManyDelegations, + /// The query response did not contain any node signatures. #[error("Query response did not contain any node signatures")] MissingSignature, diff --git a/ic-agent/src/agent/agent_test.rs b/ic-agent/src/agent/agent_test.rs index ade6eaa9..d6939d37 100644 --- a/ic-agent/src/agent/agent_test.rs +++ b/ic-agent/src/agent/agent_test.rs @@ -5,10 +5,10 @@ use self::mock::{assert_mock, assert_single_mock, mock, mock_additional}; use crate::{ agent::{http_transport::ReqwestTransport, Status}, export::Principal, - Agent, AgentError, + Agent, AgentError, Certificate, }; use candid::{Encode, Nat}; -use ic_certification::Label; +use ic_certification::{Delegation, Label}; use ic_transport_types::{NodeSignature, QueryResponse, RejectCode, RejectResponse, ReplyResponse}; use std::{collections::BTreeMap, time::Duration}; #[cfg(all(target_family = "wasm", feature = "wasm-bindgen"))] @@ -488,6 +488,56 @@ async fn no_cert() { assert_mock(read_mock).await; } +const RESP_WITH_SUBNET_KEY: &[u8] = include_bytes!("agent_test/with_subnet_key.bin"); + +#[cfg_attr(not(target_family = "wasm"), tokio::test)] +#[cfg_attr(target_family = "wasm", wasm_bindgen_test)] +async fn too_many_delegations() { + // Use the certificate as its own delegation, and repeat the process the specified number of times + fn self_delegate_cert(subnet_id: Vec, cert: &Certificate, depth: u32) -> Certificate { + let mut current = cert.clone(); + for _ in 0..depth { + current = Certificate { + tree: current.tree.clone(), + signature: current.signature.clone(), + delegation: Some(Delegation { + subnet_id: subnet_id.clone(), + certificate: serde_cbor::to_vec(¤t).unwrap(), + }), + } + } + current + } + + let canister_id_str = "rdmx6-jaaaa-aaaaa-aaadq-cai"; + let canister_id = Principal::from_text(canister_id_str).unwrap(); + let subnet_id = Vec::from( + Principal::from_text("uzr34-akd3s-xrdag-3ql62-ocgoh-ld2ao-tamcv-54e7j-krwgb-2gm4z-oqe") + .unwrap() + .as_slice(), + ); + + let (_read_mock, url) = mock( + "POST", + format!("/api/v2/canister/{}/read_state", canister_id_str).as_str(), + 200, + RESP_WITH_SUBNET_KEY.into(), + Some("application/cbor"), + ) + .await; + let path_label = Label::from_bytes("subnet".as_bytes()); + let agent = make_untimed_agent(&url); + let cert = agent + .read_state_raw(vec![vec![path_label]], canister_id) + .await + .expect("read state failed"); + let new_cert = self_delegate_cert(subnet_id, &cert, 1); + assert!(matches!( + agent.verify(&new_cert, canister_id).unwrap_err(), + AgentError::CertificateHasTooManyDelegations + )); +} + #[cfg(not(target_family = "wasm"))] mod mock { diff --git a/ic-agent/src/agent/agent_test/with_subnet_key.bin b/ic-agent/src/agent/agent_test/with_subnet_key.bin new file mode 100644 index 0000000000000000000000000000000000000000..99e9889eca7f751b24542b24d76fa1b009b62e56 GIT binary patch literal 12168 zcmd6tbyU>d_Wx&Sq`RdX>F$ya3F&U6Q@XpmJEa7X21QZ{2`Ooi4(SpEevIBnp8H+* z;j*~@{FudBA6PT{oY#Kuz0cWa&gA52u7#noy`#B_xuL$Ju{wj(u|9!O6hd*zxA^_64R! z0M7j~!ZNEIJbU&PUk?Hu>Q;c&x|X;O`{uZv78vMr@`n1><_?a=_PX}^)~3b|e&A|& zH^ck-NdaztfS_OC??oGEX@sdEsR}n-W$m1>&Hj{g+L1hgaG=@ctC;! z!GKWkw|{~G!Sw;9%A%wNx>cMJBDUNZX-Py8mBZe^{k2YjsTH01331{{VY7;?ioB~S zgy$}DO#}%*fU|4NTz{z3#q|d615yb9#g@)pd?6cmvshJ^zuS3F7tN%XjG!RPlO{cuY<_;Au#ffoULs_gBv za8Ek6{i70v(iV1wt)AOF!=Mq>%pfI8Y=Oj|Y94*z{xvuq)*ikY?QP2!<7+1s&%Qmm zw8f4*bX4jhCuc|lO;}#7d=YjoevOd&$kVF);+4h#EX<(-kYs8E$aUf@Qp!*%FO#az z>fXqVFu*vET*`lXN(|{wejiq!Th{1@vj+67<-n4Miq9~n2z*~^3}+qO(b2|TUR#_kEyxGH?-7#)DdI1BfpWO5Jpal&5p#{u8oHh_qXTHizo88G- z{tOc@M+#<8+s;f=P((nz9PT^sFf2MCQj- z-Vq8hNSdNB_I4R=UDllVK87v&&S%T@vQTp-3GvG@&rLzP;&)Z32;dFGdoXG)4b$!# zcstZn`UKg}kTJjhaSu}9-XBun^}|WyV_qWfouwVszGKUa0vcdFW)LDkSyaTJ4_TU$ zA#!!ik}G%P&gLS$Fnr5n78kaGsd&E9-aimb>|jVCu`4@~wzl4;nU%cHyvbU?nh5p9 z_nk@eP)FsgOWy)ToLdCu?`)-e{#JbZz|7L$g}_pMxQ5?drYUmE5|2Y4`rz|Fu`=J( zV87or7@+ru_2%(45cuccRsSl@0Go%qAO>22=w(zjuF@m0mXC&ON4atFY*0xrPHSb5 z3%*p#uD-G$LSz+eTBUG5>I|98C4jK+h`=)-#)(c@UJaqm57Ut2F032~P51WGnberi zrJy$Rad>V+DXwUbwm!ixH~wxTsllmw!wofGm6GzV%q|cVYvbnUc=fY8rXV2y&9|V>4PkmFyw|mT6Y)3@< zGo9Vy1P^OtAB)c;&APM%x_>&qCCFL2ZA?+bLs9}f3K_ekGfPw`^z(DJD2}2t*RK(e zDB^!5@vrhs-%l;-j<{<{Mld+?tE5q# zeK>{T{V~QN(=_ddCmZI}KDxOc4z3AX{iLX0@-!#K#kbE6r9sLv;3i5yF(~#T#;nmu zCHFDQ=g!yBs>tnEXLFX`gA{lh+Q9!Ih6BvQbE67uh=ka9Q!YTyXJV_ z(zp%B>{49PoPU|6m#^L2>b%7A;zjvRec@R~Q52KOZi%hcIB7I;%^hX;p*?KA*G?tn z#C71Lj#VG3I3hbzAS%sd-RE~2Gpaqpbryk0L_0ejzg|5?2B?BREn?E^t#^@8zXvNI z=T_CqhpQU+$rlg465~e&J?nk|t_#sDuB{P0F*~J1D=cOArRnU4Hs##itVOQpI}`I|}?f3d6o;Slkxb-ylX^^%dq6osP+=#gAzQ_MMWU*QE`) znGagvb}}F9>duc(pQs*XOQ8rE7H|g&CgA0p?tSc|`fJe@G8$@#nE-n`D33zC0&yUX zgpe?W7_Uf7g2c^nA6mg%UF9FHYuE>0L3Ac(_r*|qWgh-zEmQMO@e6x5+;{dZ_^1+| zS}5j)2iCaH1GzEhqzaX@uW}feeK75)ufVLrHfItu9crH1 z&SGz6SmdL9A1YAva|G!sGE<4-GwpX-3^Ty}F^5}EuWSx&L)lVqy?<33MEalGfxjH*LOS*^q*`!U1 zBMu~&wE#9$sEWzqUMzyZHt-pCYDvCq<%&^agVY&nBAUu0w-NXfZmoZm$n2e!Bcm!=Iy1Ur_L5)=f1;^+I0H2p~rXIPm@8fT&Ay(18=OvTZ(u_ z*nC0EBNJ6@v-;eYk&j!P3uN7Xl1a3ZEa2)DZ9|L^N%YC6<}1spqm&emg_zfOTi?h! zjDK@@`QDvR7Q>GI!>01vbCSzFUla{7Y4=a*-&ubta5I7{QB|#&0>Sj5RA%}_O0ii0 zW(p7}6XRAF%%gnw2;+$h0*WL{!YxA?2o0lc-QKUbSCS9$=oE7iO}xBf`a$z|P0Rl9 zh^d6Cmc!esYtRK9U`tgQhu&21ocr>{-PWmEGCa3%uyvuULs8!!sWZSMO%gHsL_4GG zU|V6>yx#roeb?(1O}&qoiOY4ipBb|XT%}$#(wL6LWI;IgRNQU9R$ecCa`5UY(CpnH3@Qs+vE5pgr!z6LG|-l8z3UC8(dpHwbI0$t zZsuy*uFzTp*3(O^5$iN{T;kBys8W!0-F^n6AnseOs81|MBpj( zUR~+uYmN8oDOTUFABQI|zHf8P8c;e`E511o_&?44zvWg|D1vG8m*PF4xXv&>#dt8e zbjQYbX;1XT;BbVN6q^rz``=$OF`0|to$;;<_+V-%l?I~6h5~|q^ub4pcSv{J2dHH~ zJ09VeqI9QH!zB95nlwlm+E@N{C=~J`Q@NK=Ana)F*Ka z!??MY`SThC&0@aPdoU3H}!7=4qm3gvbj`LsLNZh*~#{wr!H0n1;}pGRDUqk2g@ zdp2h@+}`ch8;W4cDD0USQ8i6vKYYsNTLBvc*}HBDi~$vm_eQ92M`w8SvTfZr8)k+bo#)48HH| z7(dX*k+?=hff$BWRn)A-kIHM*_T<4B{hv>pIFaE>K&3ni-&?%fy5lgWTQ(1>&OCy& z30XzJQBUC)s;Q`)S=(z|1=q7|+20C`myQ~>D~e#IdCRNxo`=mqCN^=jY}SEhKbe=G$mSaVF8{RvO8Uar zNwzr3ov<3UFIj|AhId+!A=#%>GD7MS$FQXPEPJx6RlEA zx%#~CN&>FWrBUOcr2crdapiQ>d1J%k*FNpWJ`*25vOpgLZY^gj&N_rT=XXTAP)tv4 zpAEv%yY0g(bU*|N3bxdrB3UQs*GQCXJ+3z0er4ol5gf@+y{7kT_96taCBjcn`1#1N zNz*bUxRNMpCD}Ka=B8hc-ECb->eZpW^E0VwxI}>TWTPL{n~$o;$6t>>Okk+$OE3`r z<^8piA0NwHV!!UwOwQmXLwdedJmZk8aEt%mSzUSe2;k$*$?-xW&W{sV0GnR3o3Bhe zu@Jl86-Y-^4bt{+3WMgqjkW*Yem~hjoHtIFQ|HxuT2m#=PB{iGv&`Q9-PVz?KK914 z5Q>OrmL{%KhvLI{dUSq|e5A7&+WO;tNcZXQ*gqhunn`d>cMq(kjs4H^=k1d|Y zg0E9jV|KUwnRwgQ>9mMv;8EKsSxx;Ld%;&huB^V7#69>WojyyyX8&_#;TgRTS>Pt} zsgA0VQsX)LlM>EjQ>R^=%Z0nG8x@i&!Rjcd|ERMP9M4Q#Ux?o(lE|iy*+5k?2GN5y}p=m*vOIf0IiGEE<@J^$#OBBPf)*o z`V=eNqyV~LEaQ=^Da*UO0RfISlp1ocP8^t{v->nBwtsf!0Plx4CyBa>r@T+L)r&Z) z6IS5jWoyUhsMmN}&g49U3d)6=CTh8U_`1!7VkQw}j1iP|ss?m-I1_=gls;NhhC#7e7_*jNpovBRI??Co&!z8ACUtnC`@WFxm4 zCcPdHO;dIn@wDv1f8mIhJYuGVy}mH@zU@t_5bC$iDP{w3{@mzp(^qi)bvLSM- zqV=gCbX1PPMj1%1%UW`he`WfwPD_nI4NoI)A6}Q_@zXiSyn)_Jz8vTGT+^`7eaFhp z_aFw|Y76`yw5_1ZYb{;vA&c7IK3-lRsxUty_&R+4NEdKJjljyWJAla_aAEC)s@JBGU!Em6Eqd9FR6>{WxG2EB^HL11J(aXU0gNL~#O<)( z946NBk*UqaR;0hR^;orFW*;-M>|CO$$O-`uA}I+?kA(a0jMc+%TEt`fWK%&wk}ScI zNstuJM0<_-ttGeZ^W;5P!ERZ>{s-0+k3~n>CS*l5SnyZcFS?WW_0uwzP%_TLq0DX3 zO1>4RRzn9HcA&(Nrl2D$@+yKO0%}JtO$^ty)Wum$%2ULyRyMF&TA5rqmhwPo*?OiRMoG?9%|?yc(V)p^l`f1C?>khgvH@}kNT0K!lE;aPr8gv z>0NpHWnmm==q?sfc(229AS&N97a|v+yxWkWk(96QEy2)eU6^l+5et;0J{^Ba@rhrR zp;X$JE(yng_`7n}fp_DDOZ$S=uSC9a!i#9ruh#wk&=5YH6?W;q#`v+Df>kla2Nb`03*Dhg|?$s&I6x=&wW#EHp}LqxeAXx%iw|5@(8lUCi5wmzJ+ zQr@L3`R(VAdQQiR>zu-4RUUF|KhD_EYm%*Pod^>)@w&FQpN0|z9kI&2x)AWFA#=pI515}%T!fs1Rf&S{}T3Qqy1l5feU|Ffx8c94U4VjfeqX899to`^MQ@sw2!N4DFoQg;iF(RL4G*0LwWs*1Pk~lGm=^EaX_YE6G}K zQ#N+CQDZ1o*dGfyd5Ij%Vi&+fRU;+@X&E#m1yVJ`Q&81tKlxA}&&^8UrOIB{Y4LOw zIm8s~7eea$kP6g^=Bcio%t5x_N#+J9P(#&LcqfqNNb9@5PkgkcKSxdq$@+)ax&|p^E?n~9&Nck zk{lLgBn=YX885)zmlsPo;0*yf}+j0;Rz6puV)|0-knKHb3by$r~Xl72%yWh zF+Eg4lB7u7rqx;fF&JG@#I!U!^4f{;WNXgvd@XO+`}$J{ow%p8KfQJ_7u_vab6*L< zAFr6T@m|~HXMb1$x({b9-I?NYXB$*@*lKfq;#RQ_HWXN15c3=d>XcUqW*)UcJRGf_ zH&m%jf>ayvDplAP>nZ;F=b?95>yPIsNbqF@_KwmH@QuJ$M|3+oy3rJ``GQUJ`y;(p z!f2nBHP1hJ4^p*BA`-1|)>-l^tiW=p=YGG3eOR4L;0u?|M$dm-bpKX`fwzk;knrK8 zAv5l52!$3lM}#me6zpZ0n6!s^@kP3>&q8;@qc;jGW4j1Vm&BiO*UG~{rgu2ma9Z~jX(W$8h z_j!$5!*;sq89M1tk!mZiC)3M{W zX~qY_jmt{}@0*N@0vt7*=hg>T?K3pqb}^uXbbUv$?VHaI_X!fpK?yIe+BKw=CY0xy zjn6VX%HVxSb8bm1A5I!2B*udspOP)CaR}om9cvZCy3QBxSH<&~@eS71H(WWwJ%O%F z<)aq6M+!|4x!(woz8cmtr=LWvdRSqcJ&`iyaMF{)x67y-AcCVrUM4r;wbfhMhythm zUgWP8gNzaRJ6l_Osdt+RCF)!x4|LIlKUGM294p?3wd$6&_2H}mmRxE| zYE}t(D`FDQIY!U$>irOO{MsFBp~vFJ;+1lZXQM`$*XuvHK8YEl`(f4&ud!fd?(69* z<@Ie$h_UG?9fvq56pFa)BS363l`A=N-~K25@Qb;z1Io`4lr7W7AXPgWtqZ&$J|%n9 zgYRopYLr$wj3s@1k5j;H7a4vJQUK#0Qh>n2NrPKHQsEXFKq!wH5EMFrT|VmB2QUrv zXP(?_SUVnlv>~Pp`NG!}mZKHb1`7+RKLFh|;I^Ah7PUP*RN~%SP5tsBMdhfqc3}9Y zN<1^1yY=xERd-TIBTeLM{oXZ$EzteLUu)~&fzQ$tZ>`THVj&C(mDNbfz501r#%rjf ze3YU8MEm!K;C5}jd4l?L(FFj2`KuxLAC_ES8AR)kgk@?xFzKMf_PI3 z_7}n(Nwv{0v@_tx#3s#jW4Yns=(<=j{9C0(;+9RPlg2gEYx?Z$P|w8a5j14cpXpvY z22`QO!s6=&H-`9&38cW{!j--T$vxpt!(f-%`090!!0LrskUm?)J_+eYex^h=LG%vR%B8(rjxWWyXOjKL!2O z4-}Xe1Y_{;fjTrw^{preM{Yx$zZgH*j@Mq+4g9L5`^q6`%X2}(h zvO&MD0~F5ZKZE+j8MbXvdGl4gqq&u_AGqY#%AD|(`ldlFa|d%%Ykfy2dt)`mM30n~ zCr{z{iD%T`Cq>&O%1V6ZU=rW5Sk{3hkfy>P*C(NC_R)7_py@Q`>vw0AcwuC0X>6+R zXl`SjXa4(cvaY$&f4PJ3Z`Yu~tZ(kX<^Am%v}Y&BfYW@NWv%2U{;-Cso7|i;7;522 zmm_KfllmJS98g^T+cjv=-DwK%6$O-9YhErVCHj000eAb2Ga~z|C_S$c(ID|KO|IWJ zti@i~Qd#~e-|)dNryu%D-UJL@r?4yTQsp1DSP&FJ zp!Zz9r1)ZXI&dcHU*)!RUij74$@b)gDPw~~n&)aRz-a$AV#^P<{0B-l2L3H#!?)`% xh7=!Tot6gx{YDgja>dlku`S?4AZr~5vJxeSJe3|;;6!Zkghn$<30vBk`+tpEsWkuq literal 0 HcmV?d00001 diff --git a/ic-agent/src/agent/mod.rs b/ic-agent/src/agent/mod.rs index 3b193954..749220e5 100644 --- a/ic-agent/src/agent/mod.rs +++ b/ic-agent/src/agent/mod.rs @@ -813,6 +813,9 @@ impl Agent { Some(delegation) => { let cert: Certificate = serde_cbor::from_slice(&delegation.certificate) .map_err(AgentError::InvalidCborData)?; + if cert.delegation.is_some() { + return Err(AgentError::CertificateHasTooManyDelegations); + } self.verify(&cert, effective_canister_id)?; let canister_range_lookup = [ "subnet".as_bytes(), @@ -845,8 +848,11 @@ impl Agent { match delegation { None => Ok(self.read_root_key()), Some(delegation) => { - let cert = serde_cbor::from_slice(&delegation.certificate) + let cert: Certificate = serde_cbor::from_slice(&delegation.certificate) .map_err(AgentError::InvalidCborData)?; + if cert.delegation.is_some() { + return Err(AgentError::CertificateHasTooManyDelegations); + } self.verify_for_subnet(&cert, subnet_id)?; let public_key_path = [ "subnet".as_bytes(),