diff --git a/security/apps/AppSignatureVerification.cpp b/security/apps/AppSignatureVerification.cpp index 8b245ecc6cd5..351db0a6f50c 100644 --- a/security/apps/AppSignatureVerification.cpp +++ b/security/apps/AppSignatureVerification.cpp @@ -82,8 +82,8 @@ inline nsDependentCSubstring DigestToDependentString(const Digest& digest) { BitwiseCast(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 @@ -103,12 +103,9 @@ nsresult ReadStream(const nsCOMPtr& stream, 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; } diff --git a/security/manager/ssl/tests/unit/sign_app.py b/security/manager/ssl/tests/unit/sign_app.py index d1e4dbd993d6..e5586216a649 100755 --- a/security/manager/ssl/tests/unit/sign_app.py +++ b/security/manager/ssl/tests/unit/sign_app.py @@ -174,20 +174,33 @@ def coseAlgorithmToSignatureParams(coseAlgorithm, issuerName): 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() @@ -297,6 +310,9 @@ def main(outputFile, appPath, *args): 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', @@ -311,4 +327,5 @@ def main(outputFile, appPath, *args): 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)) diff --git a/security/manager/ssl/tests/unit/test_signed_apps.js b/security/manager/ssl/tests/unit/test_signed_apps.js index 8d0301cb4b31..009d999c862a 100644 --- a/security/manager/ssl/tests/unit/test_signed_apps.js +++ b/security/manager/ssl/tests/unit/test_signed_apps.js @@ -687,6 +687,22 @@ add_test(function () { 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 diff --git a/security/manager/ssl/tests/unit/test_signed_apps/big_manifest.zip b/security/manager/ssl/tests/unit/test_signed_apps/big_manifest.zip new file mode 100644 index 000000000000..ca615763f3fd Binary files /dev/null and b/security/manager/ssl/tests/unit/test_signed_apps/big_manifest.zip differ diff --git a/security/manager/ssl/tests/unit/test_signed_apps/huge_manifest.zip b/security/manager/ssl/tests/unit/test_signed_apps/huge_manifest.zip new file mode 100644 index 000000000000..401c08ab3b1f Binary files /dev/null and b/security/manager/ssl/tests/unit/test_signed_apps/huge_manifest.zip differ diff --git a/security/manager/ssl/tests/unit/test_signed_apps/moz.build b/security/manager/ssl/tests/unit/test_signed_apps/moz.build index 67a58507b245..7d6fdca2861f 100644 --- a/security/manager/ssl/tests/unit/test_signed_apps/moz.build +++ b/security/manager/ssl/tests/unit/test_signed_apps/moz.build @@ -67,3 +67,5 @@ def SignedAppFile(name, flags, app_directory='app/'): #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'])