From bf706a28b53345558f7dae6eee6e785a44cc6b26 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 28 Jan 2022 16:42:48 +0100 Subject: [PATCH] Ensure new add-on submissions with a reserved GUID are already signed with privileged certificate (#18707) --- src/olympia/devhub/tests/test_views.py | 38 +++++++++++++++--- .../fixtures/files/mozilla_guid_signed.xpi | Bin 0 -> 4226 bytes src/olympia/files/utils.py | 13 ++++++ src/olympia/signing/tests/test_views.py | 25 ++++++++++-- 4 files changed, 67 insertions(+), 9 deletions(-) create mode 100644 src/olympia/files/fixtures/files/mozilla_guid_signed.xpi diff --git a/src/olympia/devhub/tests/test_views.py b/src/olympia/devhub/tests/test_views.py index a773503c0b..3835edf0de 100644 --- a/src/olympia/devhub/tests/test_views.py +++ b/src/olympia/devhub/tests/test_views.py @@ -1517,7 +1517,25 @@ class TestUploadDetail(UploadMixin, TestCase): ] @mock.patch('olympia.devhub.tasks.run_addons_linter') - def test_restricted_addon_allowed(self, run_addons_linter_mock): + def test_restricted_guid_addon_allowed_because_signed_and_has_permission( + self, run_addons_linter_mock + ): + user = user_factory() + self.grant_permission(user, 'SystemAddon:Submit') + assert self.client.login(email=user.email) + run_addons_linter_mock.return_value = self.validation_ok() + self.upload_file('../../../files/fixtures/files/mozilla_guid_signed.xpi') + upload = FileUpload.objects.get() + response = self.client.get( + reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json']) + ) + data = json.loads(force_str(response.content)) + assert data['validation']['messages'] == [] + + @mock.patch('olympia.devhub.tasks.run_addons_linter') + def test_restricted_guid_addon_not_allowed_because_not_signed( + self, run_addons_linter_mock + ): user = user_factory() self.grant_permission(user, 'SystemAddon:Submit') assert self.client.login(email=user.email) @@ -1528,12 +1546,22 @@ class TestUploadDetail(UploadMixin, TestCase): reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json']) ) data = json.loads(force_str(response.content)) - assert data['validation']['messages'] == [] + assert data['validation']['messages'] == [ + { + 'tier': 1, + 'message': ( + 'Add-ons using an ID ending with this suffix need to be signed ' + 'with privileged certificate before being submitted' + ), + 'fatal': True, + 'type': 'error', + } + ] @mock.patch('olympia.devhub.tasks.run_addons_linter') - def test_restricted_addon_not_allowed(self, run_addons_linter_mock): - user_factory(email='redpanda@mozilla.com') - assert self.client.login(email='redpanda@mozilla.com') + def test_restricted_guid_addon_not_allowed(self, run_addons_linter_mock): + user = user_factory() + assert self.client.login(email=user.email) run_addons_linter_mock.return_value = self.validation_ok() self.upload_file('../../../files/fixtures/files/mozilla_guid.xpi') upload = FileUpload.objects.get() diff --git a/src/olympia/files/fixtures/files/mozilla_guid_signed.xpi b/src/olympia/files/fixtures/files/mozilla_guid_signed.xpi new file mode 100644 index 0000000000000000000000000000000000000000..47d5892fa1533b32f753435ff3c132e269c3dce1 GIT binary patch literal 4226 zcmZ{ncT|(jw#E|-AcPWnkt)4QRX_}(hfqWBpcDgy-cb+*0g+w>X@Yc6q$z?FDZ&S# zN-xqegx-sIqTjvuc<#5(ylc-pv)7tup1uE?pB~~0h!y|1*ip^}sc?Fk^-#tH?+Dtx1_qwQ;ln;Xum-getah)l}R-sk%UOiQH_?lK^v7MC=Mk& zoQ=*RNE1fPpXwZo0>ZmY2_+MDbzEUS4H|im65_|^+CJ-nN3WZ%Xl1|0-EFU@ty^7w=%HeL+|(?1l9Vr?IBcRqB?*$;t-*q}T67R!JIt9?a78EYBz$^i&2UGjllWIHqy5x9<; z*zaDlRoIyE7Vg>o-OKeW_+I{+{%A+|xTf8v^$j_EAM7Cj0PUX=X!jtLtE-);m$%(- zH8(zOGyZ?7eNA^VG&G~`{;r6Tl8xkzZrvz?4=ZuO5?OeNsS+l_jX`E%^o;N%?r7}< z{v>aR9ycE>90=;6CWa(_gU~;fl+2lxty#kr&$@5h7#+_x9ygtR6$0owA(TQmapn_j z(owW8ql-s~cYLO7Ay;7(2}zKcA;nf4U?rC`PwTccSt|_#AV9;F7eq$zq=zw>cQ~^> zh6sR#E-I#Us3`6zgLo8m2*jTzwYGLWZ2?O#nyNCjk-W_%A!XD$9TTaDqfS%7r&qD-e2xVx%O>g>A~tU!L1N^GP(lFv1@#B zH6Ooog^W-xEk4?>t??(^vAtw>r%=%XwLL*a>ul26-ey!uNmTN?*EJtBC)~Ndrn@=7 z(Um6r#7?>|biwcBSt%@3{^wD+$Z~*DU-iZ=p{WY=vx^*56g0UY9dWrqpFBxukO7QwJ;ZZ zbTS4Q!y0++*@hvRIyp~Nx?aEvWTEOxQ(i!#cCTN~gR#bUcx0-(GG{gp3*D5Mv{c-9 z<7S$3zi6GC=?!y9-b_2_s_K4w`#hOx%p)!?10{n=Q#^2E3ko?b;dt*nrl0~K9$4pO z&3nvDQa&GAN2{%8%m2btPv%iUXiqK{JcVN{0Vyl07urd$`#c(h{OR(z#O? zA>^KWG8M@w#A{{X6g?4}PdC1}IuThwvEY6&ccGkMZ&EbYoeVb#X3FSLaZr9uX(LwV z$wC{@y(yl0P;T~SudZ`v&nZ+iK5(@r3(NkIs&%%(>OHs!+-}#ntfvsWWMH)ZCAK;< zOH4h=xT10`p3hIPbhkaV2D4}^Zb1@4DlC;`VMBAO*Bqwvu3OoalP|)AcH1EjDbL1n zF^Zs{*|SvY0M(~n{fj)Aw!q)U3Vz&IN>^q<#-+XHyU+dC`WOoZl)C6za6_3`{U(W1 z`r$LJX3aN&4^AUecb!db_hFw0HsX7&=lC^u#U9o?%5;YIcs^>Hh&rgL(6;OpgYOI@WYpAdgf?#@4{YluJ((u^vSJSD5bmHgC;0$5QTPSl-4!lu{ z6YU`}rgbG^N@AWYP`RQTNGsRpveny&^AV=BEX1kue2pj;dYudwXF!v;pGiWqR2&0a z(3@Akisa}?9Y54@FsE)-G0Cvok~a z8U{X?v|o$qgukR_1#pKH-ee!!r=7d&)Zu>%ENgSfa3h_l5VEIXCU9zy!Sx!*%@LEJ z)(hwA`>Ae@3B4X9Hp$=Om_6e4J|CTlQCVBs`htG)xu8UQ=GK;mE{8Tn&po{?9gz6B zA@uFptYW~-#$xfCJ4D;rUSu3(LgL`(c`eX8OuV~cnm%$@zSeXFk@RFqGd$MvPR{?!5{~b0` zmyMfly?zlQ5+ZwVuB#tOvc~j=VG*gsu6%9kAF%O#2V*PcP+pjWK&IDW%rzwcQz_#! z4ae7-`99YUOplm`-ZQw5l8tB6r^<|{BDhH}%1};4s;uS``v}d`nQKZqt99w*RW&gW z@9Livx#sWKGKv_pZ6V(TL(R)RcAa3k>lB|gAWu=p~(Ah1;es&OTe#v5J zEPVKU!d>5(GeU{)H)<8W0Ew-v@>6-^O$FFzt-jsh_Gz3%6whp=@3!Sk?NUZ zxdU}pkwm>9Wnq!C#uR5+8rq>;pxO(_P&OLTt;K%3DmswCl91Zc606%{R7;9g`A(Y9 z6t}avNQ*3HhZbV(!3CD(+AONx@|FDbov(P`txxVkTePDkHVaUR^-Y6KSz~^(c|PUk zcPSJThV$lfXjm!BfIE4lkFd|NH~7T?z%U&^rle}{87LyhSeT^sK@s7A6rCbWiC2x_ zIxla8BA`lAo;-!=Y&@GIV7bWzbbO^v0rs4;T;_*qWkAo@{A_otNxDiy^1zQ~Z9KAV zVn5@{*eMBsNbco&yKzOka}!0q5{Tr548>X?9Jx2YVK_XeGdwq~-9FOBIw<=Q?6z*C zD1spT*6E%5sc-3En~X0cEs1q2uHi|?_VmjtP{595DEkfr`iO6n_JoVkXBv9(VVlUC zjBVG*p%iNt~RZ9h4SJu5C^8@W`b> z(!{Cp@GG2r>7t2}*;o|SFlQs5~_y)M=6Saf5?kb(K z72KrS1$?B>)KZhqUkLZETL~Yd8mFzvn}b>H*G3A zfyaaHz8c<3I;$hZGGSkLZEkG~ibxlv#Ms={g-lK&ZWoT|l$KYRU$!1T3RS?Te& z^>r4mG|SJZ=?k{&Yi(Fnj-+gEoZ1Kr?r_tBMP%NlK4oJ<2@Id2(?49VyV zVPBEh1dKsVU=XN|A7DF|?Jnqf#N`85r7s#i8o)&W{&5}#<;sgPIOx-C4#~an5W0J7 zH%VmeB*JRm)S!~RNEISZOUWDbB~d;J!5{U>Sn4<`^pR74L1?XGMtGQRs}H(W1m0o| zbSeq4=fNC_=J=e}1+N%MyBp{aq=Fv8$pZVT!Of9k&?OJ-;!$cpWGIcb^DD(T(Ehn< zd)Gb{e4OyT{_MV6^8gN?hHOIk$ z0#V%T6tE>bF!dRKRe7AHi5TO0k>0{96Dw9+8-8F!ICMkmgSxB~!yKuG@hcsdT;4{V zETn9LV^tGLJZa<@E?C`gE@whCtGu1a?Sx9;o#V=OXWIt%^Q&*Swg4+0))if zzP!hgp2v-Fg| GcK-qZ26RRM literal 0 HcmV?d00001 diff --git a/src/olympia/files/utils.py b/src/olympia/files/utils.py index 10addb4ee2..430d2536cd 100644 --- a/src/olympia/files/utils.py +++ b/src/olympia/files/utils.py @@ -923,6 +923,19 @@ def check_xpi_info(xpi_info, addon=None, xpi_file=None, user=None): gettext('You cannot submit a Mozilla Signed Extension') ) + if ( + not addon + and guid + and guid.lower().endswith(amo.RESERVED_ADDON_GUIDS) + and not xpi_info.get('is_mozilla_signed_extension') + ): + raise forms.ValidationError( + gettext( + 'Add-ons using an ID ending with this suffix need to be signed with ' + 'privileged certificate before being submitted' + ) + ) + if not acl.langpack_submission_allowed(user, xpi_info): raise forms.ValidationError(gettext('You cannot submit a language pack')) diff --git a/src/olympia/signing/tests/test_views.py b/src/olympia/signing/tests/test_views.py index d4ff8875e3..fd5a3fb048 100644 --- a/src/olympia/signing/tests/test_views.py +++ b/src/olympia/signing/tests/test_views.py @@ -332,7 +332,7 @@ class TestUploadVersion(BaseUploadVersionTestMixin, TestCase): 'You cannot submit a Mozilla Signed Extension' ) - def test_system_addon_allowed(self): + def test_restricted_guid_addon_allowed_because_signed_and_has_permission(self): guid = 'systemaddon@mozilla.org' self.grant_permission(self.user, 'SystemAddon:Submit') qs = Addon.unfiltered.filter(guid=guid) @@ -341,7 +341,7 @@ class TestUploadVersion(BaseUploadVersionTestMixin, TestCase): 'PUT', guid=guid, version='0.0.1', - filename='src/olympia/files/fixtures/files/mozilla_guid.xpi', + filename='src/olympia/files/fixtures/files/mozilla_guid_signed.xpi', ) assert response.status_code == 201 assert qs.exists() @@ -352,7 +352,24 @@ class TestUploadVersion(BaseUploadVersionTestMixin, TestCase): assert latest_version assert latest_version.channel == amo.RELEASE_CHANNEL_UNLISTED - def test_restricted_addon_not_allowed(self): + def test_restricted_guid_addon_not_allowed_because_not_signed(self): + guid = 'systemaddon@mozilla.org' + self.grant_permission(self.user, 'SystemAddon:Submit') + qs = Addon.unfiltered.filter(guid=guid) + assert not qs.exists() + response = self.request( + 'PUT', + guid=guid, + version='0.0.1', + filename='src/olympia/files/fixtures/files/mozilla_guid.xpi', + ) + assert response.status_code == 400 + assert response.data['error'] == ( + 'Add-ons using an ID ending with this suffix need to be signed with ' + 'privileged certificate before being submitted' + ) + + def test_restricted_guid_addon_not_allowed_because_lacking_permission(self): guid = 'systemaddon@mozilla.com' qs = Addon.unfiltered.filter(guid=guid) assert not qs.exists() @@ -367,7 +384,7 @@ class TestUploadVersion(BaseUploadVersionTestMixin, TestCase): 'You cannot submit an add-on using an ID ending with this suffix' ) - def test_restricted_addon_update_allowed(self): + def test_restricted_guid_addon_update_allowed(self): """Updates to restricted IDs are allowed from anyone.""" guid = 'systemaddon@mozilla.org' self.user.update(email='pinkpanda@notzilla.com')