bug 1525343 - increase add-on manifest size limit r=Alex_Gaynor, a=lizzard
authorDana Keeler <dkeeler@mozilla.com>
Tue, 12 Feb 2019 22:01:16 +0000
changeset 516008 2705a920610a3b56aaf84b45166fd9e06e118fae
parent 516007 a6605042490a902cea77646ddd0f1cb8196ad2f1
child 516009 fd38f2049853b553a04f037456a1f2efd7e5c41e
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersAlex_Gaynor, lizzard
bugs1525343
milestone66.0
bug 1525343 - increase add-on manifest size limit r=Alex_Gaynor, a=lizzard Differential Revision: https://phabricator.services.mozilla.com/D19452
security/apps/AppSignatureVerification.cpp
security/manager/ssl/tests/unit/sign_app.py
security/manager/ssl/tests/unit/test_signed_apps.js
security/manager/ssl/tests/unit/test_signed_apps/big_manifest.zip
security/manager/ssl/tests/unit/test_signed_apps/huge_manifest.zip
security/manager/ssl/tests/unit/test_signed_apps/moz.build
--- a/security/apps/AppSignatureVerification.cpp
+++ b/security/apps/AppSignatureVerification.cpp
@@ -77,18 +77,18 @@ struct DigestWithAlgorithm {
 };
 
 // The digest must have a lifetime greater than or equal to the returned string.
 inline nsDependentCSubstring DigestToDependentString(const Digest& digest) {
   return nsDependentCSubstring(
       BitwiseCast<char*, unsigned char*>(digest.get().data), digest.get().len);
 }
 
-// Reads a maximum of 1MB from a stream into the supplied buffer.
-// The reason for the 1MB limit is because this function is used to read
+// Reads a maximum of 8MB from a stream into the supplied buffer.
+// The reason for the 8MB limit is because this function is used to read
 // signature-related files and we want to avoid OOM. The uncompressed length of
 // an entry can be hundreds of times larger than the compressed version,
 // especially if someone has specifically crafted the entry to cause OOM or to
 // consume massive amounts of disk space.
 //
 // @param stream  The input stream to read from.
 // @param buf     The buffer that we read the stream into, which must have
 //                already been allocated.
@@ -98,22 +98,19 @@ nsresult ReadStream(const nsCOMPtr<nsIIn
   // to check that Available() matches up with the actual length of
   // the file.
   uint64_t length;
   nsresult rv = stream->Available(&length);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  // Cap the maximum accepted size of signature-related files at 1MB (which is
-  // still crazily huge) to avoid OOM. The uncompressed length of an entry can
-  // be hundreds of times larger than the compressed version, especially if
-  // someone has speifically crafted the entry to cause OOM or to consume
-  // massive amounts of disk space.
-  static const uint32_t MAX_LENGTH = 1024 * 1024;
+  // Cap the maximum accepted size of signature-related files at 8MB (which
+  // should be much larger than necessary for our purposes) to avoid OOM.
+  static const uint32_t MAX_LENGTH = 8 * 1000 * 1000;
   if (length > MAX_LENGTH) {
     return NS_ERROR_FILE_TOO_BIG;
   }
 
   // With bug 164695 in mind we +1 to leave room for null-terminating
   // the buffer.
   SECITEM_AllocItem(buf, static_cast<uint32_t>(length + 1));
 
--- a/security/manager/ssl/tests/unit/sign_app.py
+++ b/security/manager/ssl/tests/unit/sign_app.py
@@ -169,30 +169,43 @@ def coseAlgorithmToSignatureParams(coseA
     key = pykey.ECCKey(keyName)
     # The subject must differ to avoid errors when importing into NSS later.
     ee = getCert('xpcshell signed app test signer ' + keyName,
                  keyName, issuerName, True, 'default')
     return (algId, key, ee.toDER())
 
 
 def signZip(appDirectory, outputFile, issuerName, rootName, manifestHashes,
-            signatureHashes, pkcs7Hashes, coseAlgorithms, emptySignerInfos):
+            signatureHashes, pkcs7Hashes, coseAlgorithms, emptySignerInfos,
+            headerPaddingFactor):
     """Given a directory containing the files to package up,
     an output filename to write to, the name of the issuer of
     the signing certificate, the name of trust anchor, a list of hash algorithms
     to use in the manifest file, a similar list for the signature file,
     a similar list for the pkcs#7 signature, a list of COSE signature algorithms
-    to include, and whether the pkcs#7 signer info should be kept empty,
-    packages up the files in the directory and creates the output
-    as appropriate."""
-    # This ensures each manifest file starts with the magic string and
-    # then a blank line.
-    mfEntries = ['Manifest-Version: 1.0', '']
+    to include, whether the pkcs#7 signer info should be kept empty, and how
+    many MB to pad the manifests by (to test handling large manifest files),
+    packages up the files in the directory and creates the output as
+    appropriate."""
+    # The header of each manifest starts with the magic string
+    # 'Manifest-Version: 1.0' and ends with a blank line. There can be
+    # essentially anything after the first line before the blank line.
+    mfEntries = ['Manifest-Version: 1.0']
+    if headerPaddingFactor > 0:
+        # In this format, each line can only be 72 bytes long. We make
+        # our padding 50 bytes per line (49 of content and one newline)
+        # so the math is easy.
+        singleLinePadding = 'a' * 49
+        # 1000000 / 50 = 20000
+        allPadding = [singleLinePadding] * (headerPaddingFactor * 20000)
+        mfEntries.extend(allPadding)
+    # Append the blank line.
+    mfEntries.append('')
 
-    with zipfile.ZipFile(outputFile, 'w') as outZip:
+    with zipfile.ZipFile(outputFile, 'w', zipfile.ZIP_DEFLATED) as outZip:
         for (fullPath, internalPath) in walkDirectory(appDirectory):
             with open(fullPath) as inputFile:
                 contents = inputFile.read()
             outZip.writestr(internalPath, contents)
 
             # Add the entry to the manifest we're building
             addManifestEntry(internalPath, manifestHashes, contents, mfEntries)
 
@@ -292,23 +305,27 @@ def main(outputFile, appPath, *args):
                         default=[])
     parser.add_argument('-s', '--signature-hash', action='append',
                         help='Hash algorithms to use in signature file',
                         default=[])
     parser.add_argument('-c', '--cose-sign', action='append',
                         help='Append a COSE signature with the given ' +
                              'algorithms (out of ES256, ES384, and ES512)',
                         default=[])
+    parser.add_argument('-z', '--pad-headers', action='store', default=0,
+                        help='Pad the header sections of the manifests ' +
+                             'with X MB of repetitive data')
     group = parser.add_mutually_exclusive_group()
     group.add_argument('-p', '--pkcs7-hash', action='append',
                        help='Hash algorithms to use in PKCS#7 signature',
                        default=[])
     group.add_argument('-e', '--empty-signerInfos', action='store_true',
                        help='Emit pkcs#7 SignedData with empty signerInfos')
     parsed = parser.parse_args(args)
     if len(parsed.manifest_hash) == 0:
         parsed.manifest_hash.append('sha256')
     if len(parsed.signature_hash) == 0:
         parsed.signature_hash.append('sha256')
     signZip(appPath, outputFile, parsed.issuer, parsed.root,
             map(hashNameToFunctionAndIdentifier, parsed.manifest_hash),
             map(hashNameToFunctionAndIdentifier, parsed.signature_hash),
-            parsed.pkcs7_hash, parsed.cose_sign, parsed.empty_signerInfos)
+            parsed.pkcs7_hash, parsed.cose_sign, parsed.empty_signerInfos,
+            int(parsed.pad_headers))
--- a/security/manager/ssl/tests/unit/test_signed_apps.js
+++ b/security/manager/ssl/tests/unit/test_signed_apps.js
@@ -682,13 +682,29 @@ add_signature_test(PKCS7WithSHA1OrSHA256
 });
 
 add_test(function () {
   certdb.openSignedAppFileAsync(
     Ci.nsIX509CertDB.AppXPCShellRoot, original_app_path("bug_1411458"),
     check_open_result("bug 1411458", Cr.NS_ERROR_CMS_VERIFY_NO_CONTENT_INFO));
 });
 
+// This has a big manifest file (~2MB). It should verify correctly.
+add_test(function () {
+  certdb.openSignedAppFileAsync(
+    Ci.nsIX509CertDB.AppXPCShellRoot, original_app_path("big_manifest"),
+    check_open_result("add-on with big manifest file", Cr.NS_OK));
+});
+
+// This has a huge manifest file (~10MB). Manifest files this large are not
+// supported (8MB is the limit). It should not verify correctly.
+add_test(function () {
+  certdb.openSignedAppFileAsync(
+    Ci.nsIX509CertDB.AppXPCShellRoot, original_app_path("huge_manifest"),
+    check_open_result("add-on with huge manifest file",
+                      Cr.NS_ERROR_SIGNED_JAR_ENTRY_INVALID));
+});
+
 // TODO: tampered MF, tampered SF
 // TODO: too-large MF, too-large RSA, too-large SF
 // TODO: MF and SF that end immediately after the last main header
 //       (no CR nor LF)
 // TODO: broken headers to exercise the parser
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..ca615763f3fd0924d4d80365357848f25f4a7251
GIT binary patch
literal 8110
zc%1E-SyU5w7KcN^B7)chh@@qaMNmS*4vi=f6%+`N4x8-4$R-ei0Wm6}S)zbZ1Ox>V
zqu2%&L=8qj7DYBOKm>x&5{ZN*3ZhX+Xl<5BPtS}!nVI)_sH*RtI=9aG{p+53xGx{t
z10t&q0)f_nEaE-9z7N^X*e&V2B(*_Oi4oYi=qP*&;s`zoTRnq8qn~^BvQQpyM4p)k
z+@!yk?Zd#B56ZYrZJVKk?cMd9-Q-5{i#WUPSszrn=HKJYc4njVs!NqX=zsf;_<)7!
zc1Z`5l)R)cF35eJE=oPmO#MNXMPy%6C8y&e*O&BQS>L?gklC<Lk6CGGbN2@;C+`C3
zvnsOe6+$JQ1{efVkvtDaq(qp-B}T+VA#m83Ib|VUFkQrtJap)dXc4Cf9vpJMoW0?|
zD{~D;$-CyccYIBZg<!t%97S0kyx*2vn?VT6z2At7;LfKNuV&ybTjifNoh|g(j=ufM
z*q~WKlc#*8mWY_6sFq6&61<8y5GM-)&B^{Th^LD$67J^ZYKBB$e2`YLk%a!PTf;L0
zV(O(rY8+CHwISQ66ow*`Pjlrd2{o@y^`bB}%JSTGE@Y+h%S26Ui*$?F$mY7A%JK^H
z-1d!!&91%gr2Lls(G_}!n<x&R#8q9FF$>vfVE1C3=iXF^pY04<BU-01KWmCl=1{aq
z{NBZ8jykC$D=U&z#Nn%wDwmg4A=?y*29Uk@y?YJt5c6EHq9znnxWo5}GYj4BydjdZ
zOJi~?J@bo;^BEJprqhKbnT>gcG@0%Fot%zm&+b(l;7A^{QJe0^gt48Yy|M|3IZ3T2
zB4?S?eIsO{R>^F3w_m`n2EE5=lUfgdAz%_|4Ba~gCVl<xJEm?1eb3}FSDJeLO6*ao
zG1RqZQDOJcFDT7H6ZZtgV-D#mQ37GhrbPbQ!FpStBa=}^)mV6U8cRt>xFBBjoP0Tc
z)iTa)zW;p3bHy#~#-m{j$FGOZoLOGKz|!vjINXWN!Frz!^6R!n@@@xR&Geimqxu(_
zL#o&7oyv_;u5xe|?2e{juk^J#e}9ps;5RD&6w68nTl+HLIr~)ejrb##?;V)&xA6hq
zQ$i~T`1Plib>TI^Ywx;O%lM}qY}XciSN+tFcL+@u%7axy=t(EbE<>T3n-{hS!INe7
zzNIu0sVuKW1`aMm>TPQhak8&C3fN}99yj7>yOranmU#qyT|VvILUNM0wY=|9(6KSC
zm9a_}By@?Xe^<+_c$GM<z#1<7%In^=_)=8&I)CGcW4vm#R%>EG@}9vOvlFxty7tiP
zCX1(Uht&so4&{+7OLw)F!)XV`?975b%`(~W>Y>p0#R0l7zX<~GSS-CHpYvn2&uN%l
z{wN~s(OY63#jt%WMoqU07ThXce;zHWr+j{3D_m2+(?d~C>6>@qdlZSaouZ@Ta;3+X
zpy7Pl{O;yHzKB;fBB1ytoc!)-Uuw}(PSw+E6@E)j%@xVf#Qh}hh~>y~0S!a05_%zC
zI^CFkQ56AAXvz>U7Q^Qr^{2G-*ul*cBDpFk+lowR$CJVw!*dR24IdE9>6UfoGHTP{
zB#7ei3wX;d`Y}5p={d@0uc@BRDh&Vi=lP0uQ!e6Sc!kf~;wjC>E@RPi{ubk~AL>cb
zoEW{RKv&mNBUkx5S%Fj~FUN})^Eq}Ctu?#<`lH)&LGA%DRKIYG?1vLhq4>!C$Wapq
zCQ~=;&bS)1eW@qbdkCV7psS7Vq%lka12W$GIl;!&4J_de$wPIMfo|!0r~)~!_x$G#
zG~CQ|xhZXGsdyK?CGat>ZA6^2mZWCmZqqL&te{6vvSuFUPI}374#U`jh3)I$f4R95
zX6n!=ctdVbf!UM<s|A>D45GHa?fi7Xz~tzheT-sg2!@e5-SwdbOEubmo_(<5Qk(7F
ziQGWJga`7W9d@m!fKAXa#+<satt~!zWq9g_8tbDqG+4NDd_eqV!9Md#u`OINy38lx
zsNTZm`yt%knIRn>HahL@re*?RSVL+l*>?~+LL^<tKQ{WGmXeRFQa_6AVY&nEKD^<N
zZC-~l=UBa2FUDD0+6<-kpuFLT0nzUpA7WXf&K)WOLq5Z@^+Ty-ILuw!%9BA=#u#X`
zC@2xNZ%>hSsMYf4;-d6(VAF&~sWDpQ=+Il6L7;4s4oLmaV|XIH++1CJd=Z|m^Sw^q
zd=RK>g6iFdomwA|d%=9`RviV=@|h?8du~m~%Cz0&t%juF@Xb?%ZaV^Tvf-J3&&2Va
zZdX)(c^F$#U2;`DBTW|h9RL6T00000000000000000000006-M2g^&9>Yao^6G3v}
zouc;p49A4CgY!{U{z()uV{zq33HxLIhYz(u8?OgVMqi=9buP6C1??$Ntr3yStI^u7
z;b`lxn?gimx@{H2D!UIEaCmeZ{mUa=<(Sv6iKWWfymRTJCmSA*J1G!Sh!!el?S>g4
zG3|?W56A_d3QP-zerCW1?{==zM7-G5&l+}B#!Xd%ftgg?(Sdj8!{lEJ6He%k+tx2s
zd)+HT)sEfBKzqo5)gk}e8vpb9Nd_eCkfo#2BKTW0_Fr09IwLK7zhxf$hyIn$N;}$b
x+1)bI##TBjtx~>a2j!&1igZ+3e|(Du%Kvv6g7%R6`~Q%a+=nHjS?m1X{SCFOOrHP%
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..401c08ab3b1f50039d428e55bb2d9d5341b4acfe
GIT binary patch
literal 31402
zc%1Fsc{tSj9tZGYN`$(PBvT@iWEsnplAVyoG7WOFM#)m6sUgh?h3Js#P-Gd2iprX$
zg`u(>F^sY_l|c<-V#vsrY-hUX-rKoz@BRCpKR&;CeV=(g^E~gF?=ydV{(a-H$Ys(9
z1VR+CD#+G;y}N3u5r4AhA4&cpcn1X>^(6(P14$=>@(1xaT(eyZ%@(VPj?=<ws?l58
z982-q?LsyKTLyE5_4g^7+AL?pCv_T@Z2xIh_JJnTHq$m-Tz-l;0{3sfGrd<wLzO?N
z^N$Gs;4RJe*jb7<z0^2>$Vp0YiplA$zs_`uDW6x;eym*aWRGG+j`EJ;T^JLGtAgK^
z5v9MvgFnd#BM__j??=3Yy|n@e-hRGlVvrwKf=A-8lUP274t<!KB#H^QcbKL|OYVKI
zEo038jLIEi$>Mc5qfakWBt(R*^_WHH$ezDEd`k3Yj)q=aJV#8$#K&q3Ct_7`58iLx
zOkE+{CvmeV0?nmJr3+j{Xj@@H;4%b)yX=pP*jYN6Y1-IZX_=w%j%FBtA2R*7`(1;t
zr!#LPW(Jt8p-D!oq^R2_B*ZdBD8~!lhqc(^3nWCCqLvBb>8TO2+jYWq{C%ip=hNa6
z<81cydJZp*G*ITE$E;9=Ov1EVKQZsNke2)E%{yO-+F67ko%IHBGDqc~#zzgX5;`ez
zF|3wJYNvEeeN>cBOj0LHDkf)sUJAKIEMha#f@EQ_nS|8-MOaK0g-G1y^xTw&t234K
zp%}{aZ_16FwKTn`rdSo5m>l^uE-_0;mEO=<|MF!?{$^qfHtX$<#%JX2-EUi#9Vc8k
zQS-CUa0R=Sk-(En9*%Bwb}@XS_$;(v?$J9ko{&|lP<U0XmA-G=z&+RX70il-sut&D
zecKSf%%zvUo+Y?f6sqgT63(@5gYZ?p9A5QW0&D4~$9j%|{k|&sL7I)BG;w*}#Pp(F
z|C`=hx&bz$^egqvVjJqVzV$3M{&wi%#rc&Jw2kz!u7;osK@OK(og25C^*wOC6=}y#
zu%%B{bV%KPY?7`Le5;eF(^g;QW*@#(cHqF2E@v*26-L9z2Nk_>X>JRn-aQtWGh$G2
z>;cKeVStA*(7aunQ|49RwlvhZn8u1V&|8{tT6C5^Q_oYINOzNR&pmNAEfs~5T{p3T
zC)}T=@05}i6O$HKEu<-&W~R8MYN|8(rZJ~Y>+2bn&W$$d#DV!fvXTNTYiQ!+iRqg3
z)=JmY-Es@vIhJOqnF^(1Ijw7p5$qMTu9R<mjcX_0OqKP-uVxq@lR7F_L%4d<yuCo{
z=PX9<#*UsUo!U=b(y#jr(tT*U`_@z+3EkVhQ_FRHxS~xnzr$nXnu~(E^G9;uY5&~h
z_|E<Lj<M=H<KLn^D?dfVQIzYt{njYtsk_xoue^erdQACJuE$$iLCW>8H3?s5NNN&`
zC~BBG)w?|9^bE?2l{IQaZDmdM<uN!Er{ib;R@)kqG;<-ZHao+4#)O)2@@Ry043nYD
zn7^8ZPsrohqu-d^9e$PPjXGX+j#E16#jT_VS2yj{)IRRRTxF}55s9jQo_Im|vcV<g
za<X==Zke{w8n*BWq}Z8XNxB=9{B|S5FGyU<mbzq)XWzJkWaLd*d3<#q;BmX!y`qlE
z8)~%}8ytSo%UhaAVQlg}Xl0e6Vimiq%BxSlm1XR$rH)45tqS<k*NEv=2&0>DgO6B`
zG!Q0syI5zs)#ss2vYoEa&egPtM<xeiCAON_wN;XQe`D7#50#XvAK-Yn93+q57>)0;
zp1p%K#xCyHVi{y8+?Xq#&M35IvsX?}=1R|QX{ISC-%l-VG0=H9S;Q$OC>%D7@1m#_
z%qzP(Vj7*o8WKw4TmuvCR;3$Xzj$s(vcX48BqytQaR^mZBeHYfy3vKucb87ufA&ar
zWmD@ohtY)H>uzp!KNvDtNTo+x#5$PJIp0*vdp()=KR#p^4(C7Enqj<=Si1CNsJHT}
z`L-d&T9--v^=^!SQ}gz_o_xeDq{UUW-ScG>_`9#2`9L=p*h+XS?zlUDa{tFl|I=1-
zv=XP=wqZ54Y3}Qi{9~P$;k7$4A68Bix9jfc2x+o)&_utU`o8j!x&_kZ;qJoK)g;^S
z9&==Mi<+T3=?ey9N5WgoXGW<p#VH;%8vYBWtd!EMM0hH24JDl~?RQQg5YabM5z>FY
zhMk$cjg_UN6WY#dg!WMK{3ZmVBvU;Mk&|yyQi?fL8zuJj95u4KCxiRR!Ocs9P_%eu
z?nygp7+t3=0RR910000000000000000000000000000000D!-eh-vn(i0{wKCB~Ik
z8yio`0RR910000000000000000000000000000000N^hzJCn??kwqX>D<{^D?9p@;
zP2I(lvb+}`|NMaY{Y~s_S>9O4>lX?{eXbpSi)hp)0000000000008iRwCFLAvkOa~
zI7m%zOPc8!XN6Zc8@YGcW7)R{4HxV=v)}q3JS1zR)P5>wD)=Qg(aKRbjaKKq2-=^1
zhdou`Qef4?{qQJc95eDbu+@`07NEN_rK|2y4{C>A_x^UVpyuw9+4<mS3!<g>BHoD?
z*D9Yq*kUj=7O;Mdb&V{O*ED`KdC}<6XPM&HXO>Vd7h_txtwYKF>(Iw8f3E2fRj<qA
zWmn4gy@_HUBA!c}6UJeMgr$-HyD|Bn&v_w)V1Ke;RImj3N7VmcmB@k_!Ls8YnezXr
zJ{HUh_7nfe8VL!u6$@qsTSR|kJC+M{iV8*rn?HX<T}A$T4=4`1{O|vV2>*42e>H9L
G_vv@7X!|Ju
--- a/security/manager/ssl/tests/unit/test_signed_apps/moz.build
+++ b/security/manager/ssl/tests/unit/test_signed_apps/moz.build
@@ -62,8 +62,10 @@ def SignedAppFile(name, flags, app_direc
 #
 # COSE test-cases
 #SignedAppFile('cose_signed_with_pkcs7.zip', ['-c', 'ES256', '-p', 'sha256'])
 #SignedAppFile('cose_int_signed_with_pkcs7.zip', ['-c', 'ES256', '-r', 'xpcshell signed apps test root', '-p', 'sha256'])
 #SignedAppFile('cose_multiple_signed_with_pkcs7.zip', ['-c', 'ES256', '-c', 'ES384', '-p', 'sha256'])
 #SignedAppFile('only_cose_signed.zip', ['-c', 'ES256'])
 #SignedAppFile('only_cose_multiple_signed.zip', ['-c', 'ES384', '-c', 'ES256'])
 #SignedAppFile('cose_tampered_good_pkcs7.zip', ['-m', 'sha1', '-s', 'sha1', '-p', 'sha1'], 'app_cose_tampered/')
+#SignedAppFile('big_manifest.zip', ['-p', 'sha256', '--pad-headers', '2'])
+#SignedAppFile('huge_manifest.zip', ['-p', 'sha256', '--pad-headers', '10'])