Backed out changeset 0a5795108e0a

MozReview-Commit-ID: DT67yB63lSn

--HG--
extra : rebase_source : ae106a256be2361a116cd1a14adb5a6d4b49ebe3

security/apps/AppSignatureVerification.cpp

@@ -286,6 +286,69 @@ VerifyEntryContentDigest(nsIZipReader* zip, const nsACString& aFilename,
   return VerifyStreamContentDigest(stream, digestFromManifest, buf);
 }
 
+// @oaram aDir       directory containing the unpacked signed archive
+// @param aFilename  path of the target file relative to aDir
+// @param digestFromManifest The digest that we're supposed to check the file's
+//                           contents against, from the manifest
+// @param buf A scratch buffer that we use for doing the I/O
+nsresult
+VerifyFileContentDigest(nsIFile* aDir, const nsAString& aFilename,
+                        const DigestWithAlgorithm& digestFromManifest,
+                        SECItem& buf)
+{
+  // Find the file corresponding to the manifest path
+  nsCOMPtr<nsIFile> file;
+  nsresult rv = aDir->Clone(getter_AddRefs(file));
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  // We don't know how to handle JARs with signed directory entries.
+  // It's technically possible in the manifest but makes no sense on disk.
+  // Inside an archive we just ignore them, but here we have to treat it
+  // as an error because the signed bytes never got unpacked.
+  int32_t pos = 0;
+  int32_t slash;
+  int32_t namelen = aFilename.Length();
+  if (namelen == 0 || aFilename[namelen - 1] == '/') {
+    return NS_ERROR_SIGNED_JAR_ENTRY_INVALID;
+  }
+
+  // Append path segments one by one
+  do {
+    slash = aFilename.FindChar('/', pos);
+    int32_t segend = (slash == kNotFound) ? namelen : slash;
+    rv = file->Append(Substring(aFilename, pos, (segend - pos)));
+    if (NS_FAILED(rv)) {
+      return NS_ERROR_SIGNED_JAR_ENTRY_INVALID;
+    }
+    pos = slash + 1;
+  }  while (pos < namelen && slash != kNotFound);
+
+  bool exists;
+  rv = file->Exists(&exists);
+  if (NS_FAILED(rv) || !exists) {
+    return NS_ERROR_SIGNED_JAR_ENTRY_MISSING;
+  }
+
+  bool isDir;
+  rv = file->IsDirectory(&isDir);
+  if (NS_FAILED(rv) || isDir) {
+    // We only support signed files, not directory entries
+    return NS_ERROR_SIGNED_JAR_ENTRY_INVALID;
+  }
+
+  // Open an input stream for that file and verify it.
+  nsCOMPtr<nsIInputStream> stream;
+  rv = NS_NewLocalFileInputStream(getter_AddRefs(stream), file, -1, -1,
+                                  nsIFileInputStream::CLOSE_ON_EOF);
+  if (NS_FAILED(rv) || !stream) {
+    return NS_ERROR_SIGNED_JAR_ENTRY_MISSING;
+  }
+
+  return VerifyStreamContentDigest(stream, digestFromManifest, buf);
+}
+
 // On input, nextLineStart is the start of the current line. On output,
 // nextLineStart is the start of the next line.
 nsresult
@@ -1454,11 +1517,534 @@ nsNSSCertificateDB::OpenSignedAppFileAsync(
   return task->Dispatch("SignedJAR");
 }
 
+//
+// Signature verification for archives unpacked into a file structure
+//
+
+// Finds the "*.rsa" signature file in the META-INF directory and returns
+// the name. It is an error if there are none or more than one .rsa file
+nsresult
+FindSignatureFilename(nsIFile* aMetaDir,
+                      /*out*/ nsAString& aFilename)
+{
+  nsCOMPtr<nsISimpleEnumerator> entries;
+  nsresult rv = aMetaDir->GetDirectoryEntries(getter_AddRefs(entries));
+  nsCOMPtr<nsIDirectoryEnumerator> files = do_QueryInterface(entries);
+  if (NS_FAILED(rv) || !files) {
+    return NS_ERROR_SIGNED_JAR_NOT_SIGNED;
+  }
+
+  bool found = false;
+  nsCOMPtr<nsIFile> file;
+  rv = files->GetNextFile(getter_AddRefs(file));
+
+  while (NS_SUCCEEDED(rv) && file) {
+    nsAutoString leafname;
+    rv = file->GetLeafName(leafname);
+    if (NS_SUCCEEDED(rv)) {
+      if (StringEndsWith(leafname, NS_LITERAL_STRING(".rsa"))) {
+        if (!found) {
+          found = true;
+          aFilename = leafname;
+        } else {
+          // second signature file is an error
+          rv = NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+          break;
+        }
+      }
+      rv = files->GetNextFile(getter_AddRefs(file));
+    }
+  }
+
+  if (!found) {
+    rv = NS_ERROR_SIGNED_JAR_NOT_SIGNED;
+  }
+
+  files->Close();
+  return rv;
+}
+
+// Loads the signature metadata file that matches the given filename in
+// the passed-in Meta-inf directory. If bufDigest is not null then on
+// success bufDigest will contain the SHA1 or SHA256 digest of the entry
+// (depending on what aDigestAlgorithm is).
+nsresult
+LoadOneMetafile(nsIFile* aMetaDir,
+                const nsAString& aFilename,
+                /*out*/ SECItem& aBuf,
+                /*optional, in*/ SECOidTag aDigestAlgorithm = SEC_OID_SHA1,
+                /*optional, out*/ Digest* aBufDigest = nullptr)
+{
+  nsCOMPtr<nsIFile> metafile;
+  nsresult rv = aMetaDir->Clone(getter_AddRefs(metafile));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = metafile->Append(aFilename);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  bool exists;
+  rv = metafile->Exists(&exists);
+  if (NS_FAILED(rv) || !exists) {
+    // we can call a missing .rsa file "unsigned" but FindSignatureFilename()
+    // already found one: missing other metadata files means a broken signature.
+    return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+  }
+
+  nsCOMPtr<nsIInputStream> stream;
+  rv = NS_NewLocalFileInputStream(getter_AddRefs(stream), metafile);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = ReadStream(stream, aBuf);
+  stream->Close();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  if (aBufDigest) {
+    rv = aBufDigest->DigestBuf(aDigestAlgorithm, aBuf.data, aBuf.len - 1);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  return NS_OK;
+}
+
+// Parses MANIFEST.MF and verifies the contents of the unpacked files
+// listed in the manifest.
+// The filenames of all entries will be returned in aMfItems. aBuf must
+// be a pre-allocated scratch buffer that is used for doing I/O.
+nsresult
+ParseMFUnpacked(const char* aFilebuf, nsIFile* aDir, SECOidTag aDigestAlgorithm,
+                /*out*/ nsTHashtable<nsStringHashKey>& aMfItems,
+                ScopedAutoSECItem& aBuf)
+{
+  const char* digestNameToFind = nullptr;
+  switch (aDigestAlgorithm) {
+    case SEC_OID_SHA256:
+      digestNameToFind = "sha256-digest";
+      break;
+    case SEC_OID_SHA1:
+      digestNameToFind = "sha1-digest";
+      break;
+    default:
+      MOZ_ASSERT_UNREACHABLE("bad argument to ParseMF");
+      return NS_ERROR_FAILURE;
+  }
+
+  const char* nextLineStart = aFilebuf;
+  nsresult rv = CheckManifestVersion(nextLineStart,
+                                     NS_LITERAL_CSTRING(JAR_MF_HEADER));
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  // Skip the rest of the header section, which ends with a blank line.
+  {
+    nsAutoCString line;
+    do {
+      rv = ReadLine(nextLineStart, line);
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
+    } while (line.Length() > 0);
+
+    // Manifest containing no file entries is OK, though useless.
+    if (*nextLineStart == '\0') {
+      return NS_OK;
+    }
+  }
+
+  nsAutoString curItemName;
+  nsAutoCString digest;
+
+  for (;;) {
+    nsAutoCString curLine;
+    rv = ReadLine(nextLineStart, curLine);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    if (curLine.Length() == 0) {
+      // end of section (blank line or end-of-file)
+
+      if (curItemName.Length() == 0) {
+        // '...Each section must start with an attribute with the name as
+        // "Name",...', so every section must have a Name attribute.
+        return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+      }
+
+      if (digest.IsEmpty()) {
+        // We require every entry to have a digest, since we require every
+        // entry to be signed and we don't allow duplicate entries.
+        return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+      }
+
+      if (aMfItems.Contains(curItemName)) {
+        // Duplicate entry
+        return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+      }
+
+      // Verify that the file's content digest matches the digest from this
+      // MF section.
+      DigestWithAlgorithm digestWithAlgorithm = { digest, aDigestAlgorithm };
+      rv = VerifyFileContentDigest(aDir, curItemName, digestWithAlgorithm,
+                                   aBuf);
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
+
+      aMfItems.PutEntry(curItemName);
+
+      if (*nextLineStart == '\0') {
+        // end-of-file
+        break;
+      }
+
+      // reset so we know we haven't encountered either of these for the next
+      // item yet.
+      curItemName.Truncate();
+      digest.Truncate();
+
+      continue; // skip the rest of the loop below
+    }
+
+    nsAutoCString attrName;
+    nsAutoCString attrValue;
+    rv = ParseAttribute(curLine, attrName, attrValue);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    // Lines to look for:
+
+    // (1) Digest:
+    if (attrName.EqualsIgnoreCase(digestNameToFind)) {
+      if (!digest.IsEmpty()) {
+        // multiple SHA* digests in section
+        return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+      }
+
+      rv = Base64Decode(attrValue, digest);
+      if (NS_FAILED(rv)) {
+        return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+      }
+
+      continue;
+    }
+
+    // (2) Name: associates this manifest section with a file in the jar.
+    if (attrName.LowerCaseEqualsLiteral("name")) {
+      if (MOZ_UNLIKELY(curItemName.Length() > 0)) {
+        // multiple names in section
+        return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+      }
+
+      if (MOZ_UNLIKELY(attrValue.Length() == 0)) {
+        return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+      }
+
+      curItemName = NS_ConvertUTF8toUTF16(attrValue);
+
+      continue;
+    }
+
+    // (3) Magic: the only other must-understand attribute
+    if (attrName.LowerCaseEqualsLiteral("magic")) {
+      // We don't understand any magic, so we can't verify an entry that
+      // requires magic. Since we require every entry to have a valid
+      // signature, we have no choice but to reject the entry.
+      return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+    }
+
+    // unrecognized attributes must be ignored
+  }
+
+  return NS_OK;
+}
+
+// recursively check a directory tree for files not in the list of
+// verified files we found in the manifest. For each file we find
+// Check it against the files found in the manifest. If the file wasn't
+// in the manifest then it's unsigned and we can stop looking. Otherwise
+// remove it from the collection so we can check leftovers later.
+//
+// @param aDir   Directory to check
+// @param aPath  Relative path to that directory (to check against aItems)
+// @param aItems All the files found
+// @param *Filename  signature files that won't be in the manifest
+nsresult
+CheckDirForUnsignedFiles(nsIFile* aDir,
+                         const nsString& aPath,
+                         /* in/out */ nsTHashtable<nsStringHashKey>& aItems,
+                         const nsAString& sigFilename,
+                         const nsAString& sfFilename,
+                         const nsAString& mfFilename)
+{
+  nsCOMPtr<nsISimpleEnumerator> entries;
+  nsresult rv = aDir->GetDirectoryEntries(getter_AddRefs(entries));
+  nsCOMPtr<nsIDirectoryEnumerator> files = do_QueryInterface(entries);
+  if (NS_FAILED(rv) || !files) {
+    return NS_ERROR_SIGNED_JAR_ENTRY_MISSING;
+  }
+
+  bool inMeta = StringBeginsWith(aPath, NS_LITERAL_STRING(JAR_META_DIR));
+
+  while (NS_SUCCEEDED(rv)) {
+    nsCOMPtr<nsIFile> file;
+    rv = files->GetNextFile(getter_AddRefs(file));
+    if (NS_FAILED(rv) || !file) {
+      break;
+    }
+
+    nsAutoString leafname;
+    rv = file->GetLeafName(leafname);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    nsAutoString curName(aPath + leafname);
+
+    bool isDir;
+    rv = file->IsDirectory(&isDir);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+
+    // if it's a directory we need to recurse
+    if (isDir) {
+      curName.AppendLiteral(u"/");
+      rv = CheckDirForUnsignedFiles(file, curName, aItems,
+                                    sigFilename, sfFilename, mfFilename);
+    } else {
+      // The files that comprise the signature mechanism are not covered by the
+      // signature.
+      //
+      // XXX: This is OK for a single signature, but doesn't work for
+      // multiple signatures because the metadata for the other signatures
+      // is not signed either.
+      if (inMeta && ( leafname == sigFilename ||
+                      leafname == sfFilename ||
+                      leafname == mfFilename )) {
+        continue;
+      }
+
+      // make sure the current file was found in the manifest
+      nsStringHashKey* item = aItems.GetEntry(curName);
+      if (!item) {
+        return NS_ERROR_SIGNED_JAR_UNSIGNED_ENTRY;
+      }
+
+      // Remove the item so we can check for leftover items later
+      aItems.RemoveEntry(item);
+    }
+  }
+  files->Close();
+  return rv;
+}
+
+/*
+ * Verify the signature of a directory structure as if it were a
+ * signed JAR file (used for unpacked JARs)
+ */
+nsresult
+VerifySignedDirectory(AppTrustedRoot aTrustedRoot,
+                      nsIFile* aDirectory,
+                      /*out, optional */ nsIX509Cert** aSignerCert)
+{
+  NS_ENSURE_ARG_POINTER(aDirectory);
+
+  if (aSignerCert) {
+    *aSignerCert = nullptr;
+  }
+
+  // Make sure there's a META-INF directory
+
+  nsCOMPtr<nsIFile> metaDir;
+  nsresult rv = aDirectory->Clone(getter_AddRefs(metaDir));
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  rv = metaDir->Append(NS_LITERAL_STRING(JAR_META_DIR));
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  bool exists;
+  rv = metaDir->Exists(&exists);
+  if (NS_FAILED(rv) || !exists) {
+    return NS_ERROR_SIGNED_JAR_NOT_SIGNED;
+  }
+  bool isDirectory;
+  rv = metaDir->IsDirectory(&isDirectory);
+  if (NS_FAILED(rv) || !isDirectory) {
+    return NS_ERROR_SIGNED_JAR_NOT_SIGNED;
+  }
+
+  // Find and load the Signature (RSA) file
+
+  nsAutoString sigFilename;
+  rv = FindSignatureFilename(metaDir, sigFilename);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  ScopedAutoSECItem sigBuffer;
+  rv = LoadOneMetafile(metaDir, sigFilename, sigBuffer);
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_SIGNED_JAR_NOT_SIGNED;
+  }
+
+  // Load the signature (SF) file and verify the signature.
+  // The .sf and .rsa files must have the same name apart from the extension.
+
+  nsAutoString sfFilename(Substring(sigFilename, 0, sigFilename.Length() - 3)
+                          + NS_LITERAL_STRING("sf"));
+
+  ScopedAutoSECItem sfBuffer;
+  rv = LoadOneMetafile(metaDir, sfFilename, sfBuffer);
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+  }
+
+  // Calculate both the SHA-1 and SHA-256 hashes of the signature file - we
+  // don't know what algorithm the PKCS#7 signature used.
+  Digest sfCalculatedSHA1Digest;
+  rv = sfCalculatedSHA1Digest.DigestBuf(SEC_OID_SHA1, sfBuffer.data,
+                                        sfBuffer.len - 1);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  Digest sfCalculatedSHA256Digest;
+  rv = sfCalculatedSHA256Digest.DigestBuf(SEC_OID_SHA256, sfBuffer.data,
+                                          sfBuffer.len - 1);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  sigBuffer.type = siBuffer;
+  UniqueCERTCertList builtChain;
+  SECOidTag digestToUse;
+  rv = VerifySignature(aTrustedRoot, sigBuffer, sfCalculatedSHA1Digest.get(),
+                       sfCalculatedSHA256Digest.get(), digestToUse, builtChain);
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+  }
+
+  // Get the expected manifest hash from the signed .sf file
+
+  nsAutoCString mfDigest;
+  rv = ParseSF(BitwiseCast<char*, unsigned char*>(sfBuffer.data), digestToUse,
+               mfDigest);
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+  }
+
+  // Load manifest (MF) file and verify signature
+
+  nsAutoString mfFilename(NS_LITERAL_STRING("manifest.mf"));
+  ScopedAutoSECItem manifestBuffer;
+  Digest mfCalculatedDigest;
+  rv = LoadOneMetafile(metaDir, mfFilename, manifestBuffer, digestToUse,
+                       &mfCalculatedDigest);
+  if (NS_FAILED(rv)) {
+    return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+  }
+
+  nsDependentCSubstring calculatedDigest(
+    DigestToDependentString(mfCalculatedDigest));
+  if (!mfDigest.Equals(calculatedDigest)) {
+    return NS_ERROR_SIGNED_JAR_MANIFEST_INVALID;
+  }
+
+  // Parse manifest and verify signed hash of all listed files
+
+  // Allocate the I/O buffer only once per JAR, instead of once per entry, in
+  // order to minimize malloc/free calls and in order to avoid fragmenting
+  // memory.
+  ScopedAutoSECItem buf(128 * 1024);
+
+  nsTHashtable<nsStringHashKey> items;
+  rv = ParseMFUnpacked(BitwiseCast<char*, unsigned char*>(manifestBuffer.data),
+                       aDirectory, digestToUse, items, buf);
+  if (NS_FAILED(rv)){
+    return rv;
+  }
+
+  // We've checked that everything listed in the manifest exists and is signed
+  // correctly. Now check on disk for extra (unsigned) files.
+  // Deletes found entries from items as it goes.
+  rv = CheckDirForUnsignedFiles(aDirectory, EmptyString(), items,
+                                sigFilename, sfFilename, mfFilename);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+
+  // We verified that every entry that we require to be signed is signed. But,
+  // were there any missing entries--that is, entries that are mentioned in the
+  // manifest but missing from the directory tree? (There shouldn't be given
+  // ParseMFUnpacked() checking them all, but it's a cheap sanity check.)
+  if (items.Count() != 0) {
+    return NS_ERROR_SIGNED_JAR_ENTRY_MISSING;
+  }
+
+  // Return the signer's certificate to the reader if they want it.
+  // XXX: We should return an nsIX509CertList with the whole validated chain.
+  if (aSignerCert) {
+    CERTCertListNode* signerCertNode = CERT_LIST_HEAD(builtChain);
+    if (!signerCertNode || CERT_LIST_END(signerCertNode, builtChain) ||
+        !signerCertNode->cert) {
+      return NS_ERROR_FAILURE;
+    }
+    nsCOMPtr<nsIX509Cert> signerCert =
+      nsNSSCertificate::Create(signerCertNode->cert);
+    NS_ENSURE_TRUE(signerCert, NS_ERROR_OUT_OF_MEMORY);
+    signerCert.forget(aSignerCert);
+  }
+
+  return NS_OK;
+}
+
+class VerifySignedDirectoryTask final : public CryptoTask
+{
+public:
+  VerifySignedDirectoryTask(AppTrustedRoot aTrustedRoot, nsIFile* aUnpackedJar,
+                            nsIVerifySignedDirectoryCallback* aCallback)
+    : mTrustedRoot(aTrustedRoot)
+    , mDirectory(aUnpackedJar)
+    , mCallback(new nsMainThreadPtrHolder<nsIVerifySignedDirectoryCallback>(
+        "VerifySignedDirectoryTask::mCallback", aCallback))
+  {
+  }
+
+private:
+  virtual nsresult CalculateResult() override
+  {
+    return VerifySignedDirectory(mTrustedRoot,
+                                 mDirectory,
+                                 getter_AddRefs(mSignerCert));
+  }
+
+  // This class doesn't directly hold NSS resources so there's nothing that
+  // needs to be released
+  virtual void ReleaseNSSResources() override { }
+
+  virtual void CallCallback(nsresult rv) override
+  {
+    (void) mCallback->VerifySignedDirectoryFinished(rv, mSignerCert);
+  }
+
+  const AppTrustedRoot mTrustedRoot;
+  const nsCOMPtr<nsIFile> mDirectory;
+  nsMainThreadPtrHandle<nsIVerifySignedDirectoryCallback> mCallback;
+  nsCOMPtr<nsIX509Cert> mSignerCert; // out
+};
+
 NS_IMETHODIMP
-nsNSSCertificateDB::VerifySignedDirectoryAsync(AppTrustedRoot, nsIFile*,
+nsNSSCertificateDB::VerifySignedDirectoryAsync(
+  AppTrustedRoot aTrustedRoot, nsIFile* aUnpackedJar,
   nsIVerifySignedDirectoryCallback* aCallback)
 {
+  NS_ENSURE_ARG_POINTER(aUnpackedJar);
   NS_ENSURE_ARG_POINTER(aCallback);
-  return aCallback->VerifySignedDirectoryFinished(
-    NS_ERROR_SIGNED_JAR_NOT_SIGNED, nullptr);
+  RefPtr<VerifySignedDirectoryTask> task(new VerifySignedDirectoryTask(aTrustedRoot,
+                                                                       aUnpackedJar,
+                                                                       aCallback));
+  return task->Dispatch("UnpackedJar");
 }

security/manager/ssl/nsIX509CertDB.idl

@@ -28,8 +28,6 @@ interface nsIOpenSignedAppFileCallback : nsISupports
                                  in nsIX509Cert aSignerCert);
 };
 
-// Only relevant while we transition away from legacy add-ons. rv will always be
-// NS_ERROR_SIGNED_JAR_NOT_SIGNED. aSignerCert will always be null.
 [scriptable, function, uuid(d5f97827-622a-488f-be08-d850432ac8ec)]
 interface nsIVerifySignedDirectoryCallback : nsISupports
 {
@@ -270,9 +268,16 @@ interface nsIX509CertDB : nsISupports {
                               in nsIOpenSignedAppFileCallback callback);
 
   /**
-   * Vestigial implementation of verifying signed unpacked add-ons. trustedRoot
-   * and aUnpackedDir are ignored. The callback is always called with
-   * NS_ERROR_SIGNED_JAR_NOT_SIGNED and a null signer cert.
+   *  Verifies the signature on a directory representing an unpacked signed
+   *  JAR file. To be considered valid, there must be exactly one signature
+   *  on the directory structure and that signature must have signed every
+   *  entry. Further, the signature must come from a certificate that
+   *  is trusted for code signing.
+   *
+   *  On success NS_OK and the trusted certificate that signed the
+   *  unpacked JAR are returned.
+   *
+   *  On failure, an error code is returned.
    */
   [must_use]
   void verifySignedDirectoryAsync(in AppTrustedRoot trustedRoot,

security/manager/ssl/tests/unit/test_signed_dir.js

@@ -2,8 +2,9 @@
 // http://creativecommons.org/publicdomain/zero/1.0/
 "use strict";
 
-// Tests that signed extensions extracted/unpacked into a directory do not pass
-// signature verification, because that's no longer supported.
+// Tests that signed extensions extracted/unpacked into a directory work pass
+// verification when non-tampered, and fail verification when tampered via
+// various means.
 
 const { ZipUtils } = ChromeUtils.import("resource://gre/modules/ZipUtils.jsm", {});
 
@@ -26,15 +27,62 @@ var gSignedXPI =
 var gTarget = FileUtils.getDir("TmpD", ["test_signed_dir"]);
 gTarget.createUnique(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
 
+/**
+ * Each property below is optional. Defining none of them means "don't tamper".
+ *
+ * @typedef {TamperInstructions}
+ * @type Object
+ * @property {String[][]} copy
+ *           Format: [[path,newname], [path2,newname2], ...]
+ *           Copy the file located at |path| and name it |newname|.
+ * @property {String[]} delete
+ *           List of paths to files to delete.
+ * @property {String[]} corrupt
+ *           List of paths to files to corrupt.
+ */
+
 /**
  * Extracts the signed XPI into a directory, and tampers the files in that
  * directory if instructed.
  *
+ * @param {TamperInstructions} tamper
+ *        Instructions on whether to tamper any files, and if so, how.
  * @returns {nsIFile}
  *          The directory where the XPI was extracted to.
  */
-function prepare() {
+function prepare(tamper) {
   ZipUtils.extractFiles(gSignedXPI, gTarget);
+
+  // copy files
+  if (tamper.copy) {
+    tamper.copy.forEach(i => {
+      let f = gTarget.clone();
+      i[0].split("/").forEach(seg => { f.append(seg); });
+      f.copyTo(null, i[1]);
+    });
+  }
+
+  // delete files
+  if (tamper.delete) {
+    tamper.delete.forEach(i => {
+      let f = gTarget.clone();
+      i.split("/").forEach(seg => { f.append(seg); });
+      f.remove(true);
+    });
+  }
+
+  // corrupt files
+  if (tamper.corrupt) {
+    tamper.corrupt.forEach(i => {
+      let f = gTarget.clone();
+      i.split("/").forEach(seg => { f.append(seg); });
+      let s = FileUtils.openFileOutputStream(f, FileUtils.MODE_WRONLY);
+      const str = "Kilroy was here";
+      s.write(str, str.length);
+      s.close();
+    });
+  }
+
   return gTarget;
 }
 
@@ -48,8 +96,8 @@ function checkResult(expectedRv, dir, resolve) {
   };
 }
 
-function verifyDirAsync(expectedRv) {
-  let targetDir = prepare();
+function verifyDirAsync(expectedRv, tamper) {
+  let targetDir = prepare(tamper);
   return new Promise((resolve, reject) => {
     certdb.verifySignedDirectoryAsync(
       Ci.nsIX509CertDB.AddonsPublicRoot, targetDir,
@@ -57,8 +105,93 @@ function verifyDirAsync(expectedRv) {
   });
 }
 
-add_task(async function testAPIFails() {
-  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_NOT_SIGNED);
+//
+// the tests
+//
+add_task(async function testValid() {
+  await verifyDirAsync(Cr.NS_OK, {} /* no tampering */);
+});
+
+add_task(async function testNoMetaDir() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_NOT_SIGNED,
+                       {delete: ["META-INF"]});
+});
+
+add_task(async function testEmptyMetaDir() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_NOT_SIGNED,
+                       {delete: ["META-INF/mozilla.rsa",
+                                 "META-INF/mozilla.sf",
+                                 "META-INF/manifest.mf"]});
+});
+
+add_task(async function testTwoRSAFiles() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MANIFEST_INVALID,
+                       {copy: [["META-INF/mozilla.rsa", "extra.rsa"]]});
+});
+
+add_task(async function testCorruptRSAFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MANIFEST_INVALID,
+                       {corrupt: ["META-INF/mozilla.rsa"]});
+});
+
+add_task(async function testMissingSFFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MANIFEST_INVALID,
+                       {delete: ["META-INF/mozilla.sf"]});
+});
+
+add_task(async function testCorruptSFFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MANIFEST_INVALID,
+                       {corrupt: ["META-INF/mozilla.sf"]});
+});
+
+add_task(async function testExtraInvalidSFFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_UNSIGNED_ENTRY,
+                       {copy: [["META-INF/mozilla.rsa", "extra.sf"]]});
+});
+
+add_task(async function testExtraValidSFFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_UNSIGNED_ENTRY,
+                       {copy: [["META-INF/mozilla.sf", "extra.sf"]]});
+});
+
+add_task(async function testMissingManifest() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MANIFEST_INVALID,
+                       {delete: ["META-INF/manifest.mf"]});
+});
+
+add_task(async function testCorruptManifest() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MANIFEST_INVALID,
+                       {corrupt: ["META-INF/manifest.mf"]});
+});
+
+add_task(async function testMissingFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_ENTRY_MISSING,
+                       {delete: ["bootstrap.js"]});
+});
+
+add_task(async function testCorruptFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MODIFIED_ENTRY,
+                       {corrupt: ["bootstrap.js"]});
+});
+
+add_task(async function testExtraFile() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_UNSIGNED_ENTRY,
+                       {copy: [["bootstrap.js", "extra"]]});
+});
+
+add_task(async function testMissingFileInDir() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_ENTRY_MISSING,
+                       {delete: ["lib/ui.js"]});
+});
+
+add_task(async function testCorruptFileInDir() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_MODIFIED_ENTRY,
+                       {corrupt: ["lib/ui.js"]});
+});
+
+add_task(async function testExtraFileInDir() {
+  await verifyDirAsync(Cr.NS_ERROR_SIGNED_JAR_UNSIGNED_ENTRY,
+                       {copy: [["lib/ui.js", "extra"]]});
 });
 
 registerCleanupFunction(function() {

toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini

@@ -253,6 +253,8 @@ tags = blocklist
 [test_signed_updatepref.js]
 run-if = addon_signing
 skip-if = require_signing || !allow_legacy_extensions
+[test_signed_verify.js]
+run-if = addon_signing
 [test_signed_inject.js]
 run-if = addon_signing
 # Bug 1394122

toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini

@@ -59,7 +59,5 @@ tags = webextensions
 [test_schema_change.js]
 [test_registerchrome.js]
 [test_system_allowed.js]
-[test_signed_verify.js]
-run-if = addon_signing
 
 [include:xpcshell-shared.ini]