From 41c2563be515a1d11d33d5f038f34352f1fa8e95 Mon Sep 17 00:00:00 2001 From: Ognjen Maric Date: Tue, 12 Dec 2023 12:58:50 +0100 Subject: [PATCH] Limit the delegation depth of read_state certificates to 1 This reflects the following change in the interface spec: https://github.com/dfinity/interface-spec/pull/239 --- packages/agent/src/bin/with_subnet_key.bin | Bin 0 -> 12382 bytes packages/agent/src/certificate.test.ts | 25 +++++++++++++++++++++ packages/agent/src/certificate.ts | 20 ++++++++++++----- 3 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 packages/agent/src/bin/with_subnet_key.bin diff --git a/packages/agent/src/bin/with_subnet_key.bin b/packages/agent/src/bin/with_subnet_key.bin new file mode 100644 index 0000000000000000000000000000000000000000..770d6ba0ef090069e9149c11e704249d7db17efc GIT binary patch literal 12382 zcmd6tbyQVb_xI1C5$P@|X{19sr9(ox8|joocXxM(5`svFbR!`p4bmYEVgN5k?-ifl zcrFiv`_Ic6jIm+tJ=glo@0xS%InT;9bg(lr@&)=psSQ%o8W zjWn`{@r}>~(+y+KT|WkXHf5Ujv>;Ggp|M zlTSUUCj3izZsBU3A2!x-9#}F}LX>JT7D*+Tq!$SlXVq_{M;YLpM$hGXo{&KMlHZ2a z>zXz4?yP?O>sj#R!D6$FN&KIe>q1$FcD1$eR+iSOqSas@_ob+QJ!O|-OHmT9?)-$b z$^;2mY3UuvNrnA1$qaY4V=?mGBF_RVOqi2D1Nt7cNDzB@!AZ9k4QE(#n$TpgcYiRL z6b*#rr1#@JuUrq)}KW>4~M3hl168eN`-no(n?)=IxE5N*T!d`H9$y5|0xju5(5B0i+r#3eX>$zkLyUp z$Ic`tEocG5zi9yrcc=A7(q1Sx)na#a5`G6H(qeJG*l(^Y-!lzp3Ry!0&dxT?rPJy$ftNv} zp3~V%jSS4Zaa`<5)H4&Xt^}Q>%KZ3)v2KhS%OkXV`kwYRlwJXLvk#eHf4v1MaQ`4TQ1rHMhH4A~MFu1rp{?*?zVO=Chf zu@t_owhashlGqzii0{cvrmSzYYGfoHFmJKuvBtxE@NP107;Z0{bM9ZHh;a$S`W3Bo z&)$md9GY7AIulx`ja2fvO4o;PTi|o(!5+TX65Pn7m9-$++z@qHuKcpW;8af^?J4zzYzk^qFZ*ZKlwt~Y=o^!K0)t?& zA3@$PAW7sx zUg}&iJezadMm7TFMCM#p;$cAX!1O020^0{Xg|@`BKT_GvPw}xg53qUNQml$z!4Ayi zHU>CJwT>$YyGe*cMxfxl>c|ig41WKNErO%q4D>PV9!2cWApV)2=?ADqT#@!H9uf{M zxEI6P#12ekQ;mm zSB;GyZY;zWRgnE~iCxh%o!J=di42>EKrC)seUUJxeSjc8axl&~Y?7i?`)JdQ+Dj+f z%^noDJwS%`AxC3MOl;@uND8bh175s141+=+Qq($)WMV(FT=qgWt+MPvMdst8TaW^; zmp1TUtYJU1(CmmjYhpn*o}}~BBnY-|bt%>wzMb>jZ)sdcqV_1xX}*1!qnE4N+U_{V z_TWMNO#R)xn4%ymo!tUQvu?_8^pY#Wwxum(q0d%1=@c}0TFt5lQy7*R&L5Ftyy5jL zj2Tt$;W-II#iO5nnYdg#K>?^hJ}F?*?W=K?R=Wi&AnRJyvb(G5-{XyczYOzhDLw0f zAD%Pu9G;CKJqbIdcr!d@*SX1DODl8coF5WQw&i0+vW^yH7zS)UeSfNLF9J;R2zo(x zd7_ni9&9BumHRzQED=g7pUs(SXwg#@9hLSGFwV^|!K$*feceKipi^916*Hdc8tj=i z2`tAH(tVXo{^jLuNUxJK;9n%?xA+1Una1K~>Oq*2pH@yT0)u!ZKN3)hbgJM#iXH>L zAA@6GHz;fkAE*_jF8>Jk1%r;svC)TV7QRW*z~kJS+|&!*e0-Oi5Hh zgFG&OfjInJll|^~s=qf~LBrwNs7Z*oLvpAjt5Anh$cS;%NU;heq{v(xx1kld)>ZEA zx`woP3t%ugxh@6UDRJ|yXquRHh<&$n!E3T>BtR2)*F-f_pi=3t{TO5K=V=^oPzLSG zH@MFA&7TWvUNT=P^CF9Z*$c~-`U1i-WNS7)-M;dP%^c2lnt3kz=ixjB9|y3m!n0+l zUNb)Dg>Zvh-C10^y2bP8n@SeC8v|=vVAB6y4*aRp&e{Dq&D8F3*LG;T7 zVwOe@c4h|PasoFG01dHjKPnVw^t}xEjH3ib>jh7bX$4N;Xi&3v{9$_=%DE7_K z`8!u$8BAOHmMx{X-$>8*y;0RgrCfVbKeK}84NRb!g_HZG11Y(+5IyJIzm`^Zzoj9!sWsJl3pnAWfN ze$M`KRYSM?JbtCx<~?IZ9!T< zT~P#F!DWSpL-6c_K@y5Msk&-n+TQAErY((YshcJ6H6aHOrvr7sx&WHH-pqK+@aG45vjZ*1#b??)eU#kgn8;`auz;dHHfU5dbq z=Mj7jAD(pN1kb(*Fxc#a<-I`?IVE zB58`xk4Zznn3SNpGG>b`&^K(l&;GUgf9B6}UN1F?7bOtST}YzDkz+5P4Xa{ep- z45c@FNxJOPO7)JFbN@h`L97YTQQ7}O%9mVo)vvf|{aclM5t%H81dVH#CytleAjM^U zD_PGwS|h?9MIZ%jN`hxU9E)ssBCTMs;YjfuXI8ur{!?1T&c(0|?vfb@KVqwljBW5HM*%O#T=O?v@R%J^Za-*IpL2de3atT~%YJbBPrrVM!g6GdZ&Z3@Cz zDV}o5y*cuTb3KJkquWF@O<8)wN(T#9b1L<5|cobbLKW z<5u;r^*@!98_x!cZOVPkkD9vpaf&U5ayO(>^+N`cq`}Sh;St*$kH`JE6Oy0IWj8y` zOiNXu9lAMNKNNg$ob_#!5j^{ObVRG_S09+dqA_+ztjKB>^QcAV+^8Vu<(^%=7T=_8 z6S<&gNPpam#AC&F%y|{{cd9y!}0jZ)=)@UiaRJOVj6s4`NykaF!=|Ajb0zjK}X;j5aHwYt?WV@{i!=2s7^60Qjo zM!ynza>~d15Qi)!O`J1IHU@l5)tgY_xs>a3Qlyl|JEc7S6wYaSn`o&MvY z<+OO5Mm}cpert6@x?l@Wf-R z*W`2MQ#F_~zwh+!_eX`ohmUUfX?{=cGypA1l!$8g!2I&*6YNmqJlMSPw0klpEKNCs z{2Z;QmE;~BxNyg3w<%6+e!rOmJnvqd#Oo-0;d!*9R=`maw~7!eQ#CP9z0TcuChH!M zS0Y$HS;hI)+hsl&D}gY5qB%wdh^VT9X%)Fv^yBm#CNO>-VU#nK6_xRR+CW4-B1{;{ z%24(3CwE+p2c^zhdJ%79>%jWT`Bc*{eB;ks%kS`|91Q!;?~hoqTHK2iuk~X87Nii2 zzeyqZ?@pRaqPRwhgI(($J@OvG&K5W4b0M4d`kvu#CQ7qG!t06P6eY(|cZ*Jf=MLzJ zqo#^D8;jHLTHhoKqJ8R^X4Z$~%Z}_anL{eIx`#<56C}GPQj^@Gt$YG8#z1;m+?bX4 zGtz%nTB?Mq`06(CqVp*c7hKb1jvIK9qYhWc$*44Qy>QKWYZq0CZj2Tov;58<9R3*nv9`& zS<&Q=nH1f{AM6m!I=Fhvecdl#)%m(qYwTn45{<58E4%CAA2A==*d{SdPOi3(+zal> zjiz{`Y<&V&RTtB737@UUx|D)6QJf>4Fd>$S_7dw;V|MHNsoRiVs|xWiR84YQa*(M< zQBZ}4e5v)kEAc=tC2bis?OP~}nGO1@Pld@9uz?2cs8M7|7$^!n3Xn*Es?l>}gAGkJ zF&5*JB+-kti&Q~PvsYG|`(K;_?0sWUPiS$!RmLYqq97+dLe~2ku~)QuH8{hy?Syak z;GaAw9X1VeBRo0sa9_+V>&(BZEdRT%f3{ApdnACLJ(AyD{Ib70D!PEH%e!qxemmMC ziZD!c>2XUVxo$P11^ESM<~=%$^{JgXdc`3eXBf`r5%{k|vY^V|G~}b?p*C$w(@4lw z^u5AVZ(dwzjS}^jp#C!Pf})2{g`r5wn=S!YpX9Sr#-V53cjvZ6OR(I}BTYV94fw)B zc|9(*Rf|(?WvXJDO)amEjzg^bCkOnoumN7bgBTEb_aF|F^>{|gfG*cwz@iTeJpG_f zqrEeU$1BlpDtf7E78-+%+%72k9&nN)QXNZiyi4~@u7D-Pm*1+ka8Edx%fviv%0qOi|>ohL6bV??FN zp&~$snpZ{me;4~t*79rC=DV|&$vGD%zJ1@V>v*ED@mOfQ+)bA4>lr&nWuldhBT?KI ze&_bilVIY2V^-Oh-}ybbpm}q=pS6@c#jDP-80tc@OGkJY-3)8z>vkATRJo}zY&?A+ zo2Loah9cdSdPtIY239rJ=wMQ6 z+`BtzNOT1^e8{%@_$s-r=Y8tY1UDU7z2U0vu0(8lYh|AzZ6RJESf}_-?Bc@ZYWC?k z_{WpueY>A5cx(qugHD7x6RdQ%WTNNl)Q6LWe6djym&qZ__W(>(m7;>s76HSOS0I0F zx@qnLDuxG%M|$`!mi#Z2_dCB7&XkiwO~Zd6qP`99b$SN=i}aj77K8RYxl04SlYd=9 z{o^MYOyyTIv=g(2*7V;A&~E`Y}TiR3o~9G$G-Z0Qzs~6J;F1YIhYk{rJ)7?E+ao2 zAX9qZr9yuxWFtqlfL^D548n`Y381o?v!LdVTyrtIwIM+cxiecq;87H`6~%mz9zkpy zeH~K5g@H}UL#u*sIp8I#N|-}iqK7PtO&LyuOn1gbPp*8bCTy#xCcq-YGv+F)t@PIf)t9R`q zAUbdF0kWMdlcvT&_^MaUz3?DFr%hdIu)G9mftYo(liC9ax`MD7DRz|gQ=zHmEHG(F zo9@RS+889gMFXi-OW7DM*%}9mP`>zutaZ042%i2X1?b$Jv}kvl)0J&V$$q;P^vI?3 z0Akp`Brob2F3cAmLAV99TCq^{8lGUqR&g>dq>E%B8|)_p8}EmkGBzH}Q;-r!^Y0(0 z91<8ptc~h)w0EH^T=E8*+He0)~i;al0U|WHd!O$r+;sB^a6!R*KZd@urMQucbgSF{99Mx z!re*z=`zpe!_*ANs);kV3ci@AbA79HsohC6d4oFFs+z<P~FNh;k$an0zU_Jr}>ScHM=zRx19_azoNB? zV#hm|9iayr#$FLWrUDeyOcTug)Y@z1KI+hIShKEK%kIt^At=g?6PuJNq<#eFAQf#H z#k#>8>r>ACfbk9X#V0&jqJ93(bfx1)+k5i$P}!ddkv|$#F{hqJthrfYo;{K@;c(QI zB(P1Z9wbJfL|Gv>=CRRT-Hd>w{aoOy8HIuw4pz0fk9x0xNW9uv;;^^Aw$R^t+}Y=Q za!;v*+lj($NXxHDo9|BQXThnesA?IPvnne7jAQH!zs3hq+o#Q;3U)kZJXSH=Xf9%m zd84KU^eAea?yG6ref334GjDfqNsmwCf{gXYskkHo!7wDn-2l<8WX{C!1G^stBhTl@ z4=LY=Q8v!}{q@}yZ+o3po(EJJu6-5MFTN)!A2(MGj{H!LWkztdI=P_gN(ic>34g8Ex30e}23B_H&};dT zr{;T7(IAGnvI=CSKD``lqjj_~UdrHqBK@-*xZYi_u3&y2>Hz@Y{w@dp#ir{mjbznL zRIJJk*W}H?L$1lgflu!jz>}1Buo&tpT9*1%D)G0P(fh40jhhnPurLBZNoBb9P{690x9gcI+e0 zJqjY6sW!2EO4IGZq*;5a+$4;bUs2(n6OP}12h^rftZ8Ey8RUHAD(xnxsK% z`aA{t;Ahh=PF6e8ehkt|KI*ZKwZ`|UAy&MCE4EG@!nW+2^v2j2Yj2^(R2!`zATmzi6T0y-QRTjNAIB#KYU}WcDW^87l=U}7;VRd!h zFXxXV-$+62skxGIGHZMzxPEPk&96Aj&M1zuo(Q-tx12vK_}Odvk0amUbK(@9tMaH- zRy>@JiuAc0{H}JJXT)|F5xO3uA^~C{8dqQOgYOc3ZbN19wPe$aAfJ!4%xY0BEACuZ zA^>L0ui9TgmC+si?ghRtl)(ZMHe_YAj@2ewGoO!xh4RRwP%66?+dav=CY9+mc)9$q z6D9xcc=>;3!CRgu`Twy)4|Wor{EVye)0*6w27ksvk+iO_K#>=r`iICo6qEGvt1wC6 zd-j6kJ5W9gs4(@`p`SW~ma7XYC)7STSk@I(x_j7+j)U(}`jl(Tp-?F&0eC<+yi1Rh z_KJal0=3kz3?3bsgSN8%dc{s2oqMf#9S{A-iVZ{53pQDh)~P?6KkN#GPgi^4NGcLU fK`5Y1yc&!9T42ysEK3e1Z-+MZ(F5VvWZ3@?Bl!97 literal 0 HcmV?d00001 diff --git a/packages/agent/src/certificate.test.ts b/packages/agent/src/certificate.test.ts index 06b8f2fe0..2ff710e62 100644 --- a/packages/agent/src/certificate.test.ts +++ b/packages/agent/src/certificate.test.ts @@ -9,6 +9,7 @@ import { fromHex, toHex } from './utils/buffer'; import { Principal } from '@dfinity/principal'; import { decodeTime } from './utils/leb'; import { lookupResultToBuffer, lookup_path } from './certificate'; +import { readFileSync } from 'fs'; function label(str: string): ArrayBuffer { return new TextEncoder().encode(str); @@ -258,3 +259,27 @@ test('certificate verification fails if the time of the certificate is > 5 minut }), ).rejects.toThrow('Invalid certificate: Certificate is signed more than 5 minutes in the future'); }); + +test('certificate verification fails on nested delegations', async () => { + // This is a recorded certificate from a read_state request to the II + // subnet, with the /subnet tree included. Thus, it could be used as its + // own delegation, according to the old interface spec definition. + const withSubnetSubtree = readFileSync('packages/agent/src/bin/with_subnet_key.bin'); + const canisterId = Principal.fromText("rdmx6-jaaaa-aaaaa-aaadq-cai"); + const subnetId = Principal.fromText("uzr34-akd3s-xrdag-3ql62-ocgoh-ld2ao-tamcv-54e7j-krwgb-2gm4z-oqe"); + jest.setSystemTime(new Date(Date.parse('2023-12-12T10:40:00.652Z'))); + let cert: Cert.Cert = cbor.decode(withSubnetSubtree); + const overlyNested = cbor.encode({ + tree: cert.tree, + signature: cert.signature, + delegation: { + subnet_id: subnetId.toUint8Array(), + certificate: withSubnetSubtree, + }, + }) + await expect(Cert.Certificate.create({ + certificate: overlyNested, + rootKey: fromHex(IC_ROOT_KEY), + canisterId: canisterId, + })).rejects.toThrow('Invalid certificate: Delegation certificates cannot be nested'); +}); diff --git a/packages/agent/src/certificate.ts b/packages/agent/src/certificate.ts index 376483c6e..af156aa5f 100644 --- a/packages/agent/src/certificate.ts +++ b/packages/agent/src/certificate.ts @@ -170,20 +170,24 @@ export class Certificate { * @throws {CertificateVerificationError} */ public static async create(options: CreateCertificateOptions): Promise { + const cert = Certificate.createUnverified(options); + + await cert.verify(); + return cert; + } + + private static createUnverified(options: CreateCertificateOptions): Certificate { let blsVerify = options.blsVerify; if (!blsVerify) { blsVerify = bls.blsVerify; } - const cert = new Certificate( + return new Certificate( options.certificate, options.rootKey, options.canisterId, blsVerify, options.maxAgeInMinutes, ); - - await cert.verify(); - return cert; } private constructor( @@ -259,7 +263,7 @@ export class Certificate { return this._rootKey; } - const cert: Certificate = await Certificate.create({ + const cert: Certificate = await Certificate.createUnverified({ certificate: d.certificate, rootKey: this._rootKey, canisterId: this._canisterId, @@ -268,6 +272,12 @@ export class Certificate { maxAgeInMinutes: Infinity, }); + if (cert.cert.delegation) { + throw new CertificateVerificationError('Delegation certificates cannot be nested'); + } + + await cert.verify(); + const canisterInRange = check_canister_ranges({ canisterId: this._canisterId, subnetId: Principal.fromUint8Array(new Uint8Array(d.subnet_id)),