bug 1525343 - increase add-on manifest size limit r=Alex_Gaynor
authorDana Keeler <dkeeler@mozilla.com>
Tue, 12 Feb 2019 22:01:16 +0000
changeset 458826 43475e4517a9
parent 458825 d381785b0c4c
child 458827 a2e4d687840d
push id35548
push useropoprus@mozilla.com
push dateWed, 13 Feb 2019 09:48:26 +0000
treeherdermozilla-central@93e37c529818 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersAlex_Gaynor
bugs1525343
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 1525343 - increase add-on manifest size limit r=Alex_Gaynor 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'])