Extract cert-svc utility function to separate file 96/66396/4 accepted/tizen/common/20160420.141927 accepted/tizen/ivi/20160421.010848 accepted/tizen/mobile/20160421.010919 accepted/tizen/tv/20160421.010904 accepted/tizen/wearable/20160421.010857 submit/tizen/20160420.054036
authorTomasz Iwanek <t.iwanek@samsung.com>
Mon, 18 Apr 2016 08:08:22 +0000 (10:08 +0200)
committerTomasz Iwanek <t.iwanek@samsung.com>
Tue, 19 Apr 2016 09:05:05 +0000 (02:05 -0700)
Change-Id: I0a591e899916ac6642080babc9adf8ae91460b49

src/common/CMakeLists.txt
src/common/certificate_validation.cc [new file with mode: 0644]
src/common/certificate_validation.h [new file with mode: 0644]
src/common/step/security/step_check_signature.cc
src/common/step/security/step_check_signature.h
src/unit_tests/signature_unittest.cc

index 5583abf..17a4930 100644 (file)
@@ -2,6 +2,7 @@
 SET(SRCS
   app_installer.cc
   backup_paths.cc
+  certificate_validation.cc
   installer_context.cc
   shared_dirs.cc
   plugins/plugin_factory.cc
diff --git a/src/common/certificate_validation.cc b/src/common/certificate_validation.cc
new file mode 100644 (file)
index 0000000..9fb02ef
--- /dev/null
@@ -0,0 +1,217 @@
+// Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved
+// Use of this source code is governed by an apache-2.0 license that can be
+// found in the LICENSE file.
+
+#include "common/certificate_validation.h"
+
+#include <boost/format.hpp>
+#include <vcore/SignatureValidator.h>
+
+#include "common/utils/base64.h"
+
+namespace bf = boost::filesystem;
+namespace ci = common_installer;
+
+namespace {
+
+bool SetAuthorCertificate(ValidationCore::SignatureData data,
+    common_installer::CertificateInfo* cert_info) {
+  ValidationCore::CertificateList cert_list = data.getCertList();
+  ValidationCore::CertificateList::iterator it = cert_list.begin();
+  if (it == cert_list.end()) {
+    LOG(ERROR) << "No certificates in certificate list";
+    return false;
+  }
+  unsigned char* public_key;
+  size_t len;
+  (*it)->getPublicKeyDER(&public_key, &len);
+  std::string author_id =
+      ci::EncodeBase64(reinterpret_cast<const char*>(public_key));
+  cert_info->author_id.set(author_id);
+  cert_info->author_certificate.set(*it);
+  // cert_list has at least 3 certificates: end-user, intermediate, root
+  // currently pkgmgr can store only one intermediate cert, so just set
+  // first intermediate cert here.
+  ++it;
+  if (it == cert_list.end()) {
+    LOG(ERROR) << "No intermediate certificates in certificate list";
+    return false;
+  }
+  cert_info->author_intermediate_certificate.set(*it);
+  cert_info->author_root_certificate.set(data.getRootCaCertificatePtr());
+  return true;
+}
+
+bool SetDistributorCertificate(ValidationCore::SignatureData data,
+    common_installer::CertificateInfo* cert_info) {
+  ValidationCore::CertificateList cert_list = data.getCertList();
+  ValidationCore::CertificateList::iterator it = cert_list.begin();
+  if (it == cert_list.end()) {
+    LOG(ERROR) << "No certificates in certificate list";
+    return false;
+  }
+  cert_info->distributor_certificate.set(*it);
+  ++it;
+  if (it == cert_list.end()) {
+    LOG(ERROR) << "No intermediate certificates in certificate list";
+    return false;
+  }
+  cert_info->distributor_intermediate_certificate.set(*it);
+  cert_info->distributor_root_certificate.set(data.getRootCaCertificatePtr());
+  return true;
+}
+
+}  // namespace
+
+namespace common_installer {
+
+common_installer::PrivilegeLevel CertStoreIdToPrivilegeLevel(
+    ValidationCore::CertStoreId::Type id) {
+  switch (id) {
+    case ValidationCore::CertStoreId::VIS_PUBLIC:
+      return common_installer::PrivilegeLevel::PUBLIC;
+    case ValidationCore::CertStoreId::VIS_PARTNER:
+      return common_installer::PrivilegeLevel::PARTNER;
+    case ValidationCore::CertStoreId::VIS_PLATFORM:
+      return common_installer::PrivilegeLevel::PLATFORM;
+    default:
+      return common_installer::PrivilegeLevel::UNTRUSTED;
+  }
+}
+
+privilege_manager_visibility_e PrivilegeLevelToVisibility(
+    common_installer::PrivilegeLevel level) {
+  switch (level) {
+    case common_installer::PrivilegeLevel::PUBLIC:
+      return PRVMGR_PACKAGE_VISIBILITY_PUBLIC;
+    case common_installer::PrivilegeLevel::PARTNER:
+      return PRVMGR_PACKAGE_VISIBILITY_PARTNER;
+    case common_installer::PrivilegeLevel::PLATFORM:
+      return PRVMGR_PACKAGE_VISIBILITY_PLATFORM;
+    default:
+      assert(false && "Not reached");
+  }
+}
+
+void SetPrivilegeLevel(ValidationCore::SignatureData data,
+    common_installer::PrivilegeLevel* level) {
+  // already set
+  if (*level != common_installer::PrivilegeLevel::UNTRUSTED)
+    return;
+  *level = CertStoreIdToPrivilegeLevel(data.getVisibilityLevel());
+}
+
+bool ValidateSignatureFile(
+    const bf::path& base_path,
+    const ValidationCore::SignatureFileInfo& file_info,
+    common_installer::PrivilegeLevel* level,
+    common_installer::CertificateInfo* cert_info,
+    bool check_reference, std::string* error_message) {
+  bf::path path = base_path / file_info.getFileName();
+  LOG(INFO) << "Processing signature: " << path;
+
+  ValidationCore::SignatureValidator validator(file_info);
+  ValidationCore::SignatureData data;
+  ValidationCore::VCerr result = validator.check(
+      base_path.string(),  // app content path for checking hash of file ref.
+      true,                // ocsp check flag
+      check_reference,     // file reference hash check flag
+      data);               // output signature data
+
+  std::string errnum = boost::str(boost::format("%d") % result);
+  *error_message = validator.errorToString(result);
+  *error_message += ":<" + errnum + ">";
+
+  switch (result) {
+    case ValidationCore::E_SIG_REVOKED:
+      LOG(ERROR) << "Certificate is revoked";
+      return false;
+    case ValidationCore::E_SIG_DISREGARDED:
+        if (data.isAuthorSignature()) {
+          LOG(ERROR) << "Author-signiture is disregarded";
+          return false;
+        }
+        LOG(WARNING) << "Signature disregarded: " << path;
+        break;
+    case ValidationCore::E_SIG_NONE:
+      if (data.isAuthorSignature()) {
+        // set author certificates to be saved in pkgmgr
+        if (!SetAuthorCertificate(data, cert_info))
+          return false;
+      } else if (file_info.getFileNumber() == 1) {
+        // First distributor signature sets the privilege level
+        // (wrt spec. 0620.)
+        SetPrivilegeLevel(data, level);
+        if (!SetDistributorCertificate(data, cert_info))
+          return false;
+      }
+      // TODO(s89.jang): Set distributor2 certificate
+      break;
+    default:
+      LOG(ERROR) << "signature validation check failed : "
+                 << validator.errorToString(result);
+      return false;
+  }
+  return true;
+}
+
+bool ValidateSignatures(const bf::path& base_path,
+    PrivilegeLevel* level, common_installer::CertificateInfo* cert_info,
+    bool check_reference, std::string* error_message) {
+  // Find signature files
+  ValidationCore::SignatureFileInfoSet signature_files;
+  ValidationCore::SignatureFinder signature_finder(base_path.string());
+  if (signature_finder.find(signature_files) !=
+      ValidationCore::SignatureFinder::NO_ERROR) {
+    LOG(ERROR) << "Error while searching for signatures";
+    return false;
+  }
+  LOG(INFO) << "Number of signature files: " << signature_files.size();
+
+  // Read xml schema for signatures
+  for (auto& file_info : signature_files) {
+    std::string error;
+    if (!ValidateSignatureFile(base_path, file_info, level,
+                               cert_info, check_reference, &error)) {
+      *error_message = error;
+      return false;
+    }
+  }
+  return true;
+}
+
+bool ValidatePrivilegeLevel(common_installer::PrivilegeLevel level,
+    bool is_webapp, const char* api_version, GList* privileges,
+    std::string* error_message) {
+  if (level == common_installer::PrivilegeLevel::UNTRUSTED) {
+    if (privileges) {
+      LOG(ERROR) << "Untrusted application cannot declare privileges";
+      return false;
+    } else {
+      return true;
+    }
+  }
+
+  char* error = nullptr;
+  int status = PRVMGR_ERR_NONE;
+  // Do the privilege check only if the package has privileges
+  if (privileges) {
+    status = privilege_manager_verify_privilege(api_version,
+        is_webapp ? PRVMGR_PACKAGE_TYPE_WRT : PRVMGR_PACKAGE_TYPE_CORE,
+        privileges, PrivilegeLevelToVisibility(level), &error);
+  }
+  if (status != PRVMGR_ERR_NONE) {
+    std::string errnum =
+                   boost::str(boost::format("%d") % status);
+    LOG(ERROR) << "Error while verifing privilege level: "
+               << error << " <" << errnum << ">";
+    *error_message = error;
+    *error_message += ":<" + errnum + ">";
+    free(error);
+    return false;
+  }
+  LOG(INFO) << "Privilege level checked";
+  return true;
+}
+
+}  // namespace common_installer
diff --git a/src/common/certificate_validation.h b/src/common/certificate_validation.h
new file mode 100644 (file)
index 0000000..6287883
--- /dev/null
@@ -0,0 +1,45 @@
+// Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved
+// Use of this source code is governed by an apache-2.0 license that can be
+// found in the LICENSE file.
+
+#ifndef COMMON_CERTIFICATE_VALIDATION_H_
+#define COMMON_CERTIFICATE_VALIDATION_H_
+
+#include <boost/filesystem/path.hpp>
+#include <privilege_manager.h>
+#include <vcore/SignatureData.h>
+#include <vcore/SignatureFinder.h>
+
+#include <string>
+
+#include "common/installer_context.h"
+
+namespace common_installer {
+
+common_installer::PrivilegeLevel CertStoreIdToPrivilegeLevel(
+    ValidationCore::CertStoreId::Type id);
+
+privilege_manager_visibility_e PrivilegeLevelToVisibility(
+    common_installer::PrivilegeLevel level);
+
+void SetPrivilegeLevel(ValidationCore::SignatureData data,
+    common_installer::PrivilegeLevel* level);
+
+bool ValidateSignatureFile(
+    const boost::filesystem::path& base_path,
+    const ValidationCore::SignatureFileInfo& file_info,
+    common_installer::PrivilegeLevel* level,
+    common_installer::CertificateInfo* cert_info,
+    bool check_reference, std::string* error_message);
+
+bool ValidateSignatures(const boost::filesystem::path& base_path,
+    PrivilegeLevel* level, common_installer::CertificateInfo* cert_info,
+    bool check_reference, std::string* error_message);
+
+bool ValidatePrivilegeLevel(common_installer::PrivilegeLevel level,
+    bool is_webapp, const char* api_version, GList* privileges,
+    std::string* error_message);
+
+}  // namespace common_installer
+
+#endif  // COMMON_CERTIFICATE_VALIDATION_H_
index 8262008..e39d464 100644 (file)
@@ -6,22 +6,15 @@
 
 #include <boost/filesystem/operations.hpp>
 #include <boost/filesystem/path.hpp>
-#include <boost/format.hpp>
 #include <glib.h>
-#include <privilege_manager.h>
-
-#include <vcore/Certificate.h>
-#include <vcore/SignatureFinder.h>
-#include <vcore/SignatureValidator.h>
-#include <vcore/Error.h>
 
 #include <cassert>
 #include <cstdlib>
 #include <string>
 
-#include "common/utils/base64.h"
-#include "common/utils/glist_range.h"
+#include "common/certificate_validation.h"
 #include "common/pkgmgr_registration.h"
+#include "common/utils/glist_range.h"
 
 namespace bf = boost::filesystem;
 namespace ci = common_installer;
@@ -40,189 +33,11 @@ bool CheckPkgCertificateMismatch(const std::string& pkgid,
   return certificate_mismatch;
 }
 
-common_installer::PrivilegeLevel CertStoreIdToPrivilegeLevel(
-    ValidationCore::CertStoreId::Type id) {
-  switch (id) {
-    case ValidationCore::CertStoreId::VIS_PUBLIC:
-      return common_installer::PrivilegeLevel::PUBLIC;
-    case ValidationCore::CertStoreId::VIS_PARTNER:
-      return common_installer::PrivilegeLevel::PARTNER;
-    case ValidationCore::CertStoreId::VIS_PLATFORM:
-      return common_installer::PrivilegeLevel::PLATFORM;
-    default:
-      return common_installer::PrivilegeLevel::UNTRUSTED;
-  }
-}
-
-privilege_manager_visibility_e PrivilegeLevelToVisibility(
-    common_installer::PrivilegeLevel level) {
-  switch (level) {
-    case common_installer::PrivilegeLevel::PUBLIC:
-      return PRVMGR_PACKAGE_VISIBILITY_PUBLIC;
-    case common_installer::PrivilegeLevel::PARTNER:
-      return PRVMGR_PACKAGE_VISIBILITY_PARTNER;
-    case common_installer::PrivilegeLevel::PLATFORM:
-      return PRVMGR_PACKAGE_VISIBILITY_PLATFORM;
-    default:
-      assert(false && "Not reached");
-  }
-}
-
-void SetPrivilegeLevel(ValidationCore::SignatureData data,
-    common_installer::PrivilegeLevel* level) {
-  // already set
-  if (*level != common_installer::PrivilegeLevel::UNTRUSTED)
-    return;
-  *level = CertStoreIdToPrivilegeLevel(data.getVisibilityLevel());
-}
-
-void SetAuthorCertificate(ValidationCore::SignatureData data,
-    common_installer::CertificateInfo* cert_info) {
-  ValidationCore::CertificateList cert_list = data.getCertList();
-  ValidationCore::CertificateList::iterator it = cert_list.begin();
-  unsigned char* public_key;
-  size_t len;
-  (*it)->getPublicKeyDER(&public_key, &len);
-  std::string author_id =
-      ci::EncodeBase64(reinterpret_cast<const char*>(public_key));
-  cert_info->author_id.set(author_id);
-  cert_info->author_certificate.set(*it);
-  // cert_list has at least 3 certificates: end-user, intermediate, root
-  // currently pkgmgr can store only one intermediate cert, so just set
-  // first intermediate cert here.
-  ++it;
-  cert_info->author_intermediate_certificate.set(*it);
-  cert_info->author_root_certificate.set(data.getRootCaCertificatePtr());
-}
-
-void SetDistributorCertificate(ValidationCore::SignatureData data,
-    common_installer::CertificateInfo* cert_info) {
-  ValidationCore::CertificateList cert_list = data.getCertList();
-  ValidationCore::CertificateList::iterator it = cert_list.begin();
-  cert_info->distributor_certificate.set(*it);
-  ++it;
-  cert_info->distributor_intermediate_certificate.set(*it);
-  cert_info->distributor_root_certificate.set(data.getRootCaCertificatePtr());
-}
-
-common_installer::Step::Status ValidateSignatureFile(
-    const bf::path& base_path,
-    const ValidationCore::SignatureFileInfo& file_info,
-    common_installer::PrivilegeLevel* level,
-    common_installer::CertificateInfo* cert_info,
-    bool check_reference, std::string* error_message) {
-  bf::path path = base_path / file_info.getFileName();
-  LOG(INFO) << "Processing signature: " << path;
-
-  ValidationCore::SignatureValidator validator(file_info);
-  ValidationCore::SignatureData data;
-  ValidationCore::VCerr result = validator.check(
-      base_path.string(),  // app content path for checking hash of file ref.
-      true,                // ocsp check flag
-      check_reference,     // file reference hash check flag
-      data);               // output signature data
-
-  std::string errnum =
-                  boost::str(boost::format("%d") % result);
-  *error_message = validator.errorToString(result);
-  *error_message += ":<" + errnum + ">";
-
-  switch (result) {
-    case ValidationCore::E_SIG_REVOKED:
-      LOG(ERROR) << "Certificate is revoked";
-      return common_installer::Step::Status::SIGNATURE_INVALID;
-    case ValidationCore::E_SIG_DISREGARDED:
-        if (data.isAuthorSignature()) {
-          LOG(ERROR) << "Author-signiture is disregarded";
-          return common_installer::Step::Status::SIGNATURE_INVALID;
-        }
-        LOG(WARNING) << "Signature disregarded: " << path;
-        break;
-    case ValidationCore::E_SIG_NONE:
-      if (data.isAuthorSignature()) {
-        // set author certificates to be saved in pkgmgr
-        SetAuthorCertificate(data, cert_info);
-      } else if (file_info.getFileNumber() == 1) {
-        // First distributor signature sets the privilege level
-        // (wrt spec. 0620.)
-        SetPrivilegeLevel(data, level);
-        SetDistributorCertificate(data, cert_info);
-      }
-      // TODO(s89.jang): Set distributor2 certificate
-      break;
-    default:
-      LOG(ERROR) << "signature validation check failed : "
-                 << validator.errorToString(result);
-      return common_installer::Step::Status::SIGNATURE_INVALID;
-  }
-  return common_installer::Step::Status::OK;
-}
-
-bool ValidatePrivilegeLevel(common_installer::PrivilegeLevel level,
-    bool is_webapp, const char* api_version, GList* privileges,
-    std::string* error_message) {
-  if (level == common_installer::PrivilegeLevel::UNTRUSTED) {
-    if (privileges) {
-      LOG(ERROR) << "Untrusted application cannot declare privileges";
-      return false;
-    } else {
-      return true;
-    }
-  }
-
-  char* error = nullptr;
-  int status = PRVMGR_ERR_NONE;
-  // Do the privilege check only if the package has privileges
-  if (privileges) {
-    status = privilege_manager_verify_privilege(api_version,
-        is_webapp ? PRVMGR_PACKAGE_TYPE_WRT : PRVMGR_PACKAGE_TYPE_CORE,
-        privileges, PrivilegeLevelToVisibility(level), &error);
-  }
-  if (status != PRVMGR_ERR_NONE) {
-    std::string errnum =
-                   boost::str(boost::format("%d") % status);
-    LOG(ERROR) << "Error while verifing privilege level: "
-               << error << " <" << errnum << ">";
-    *error_message = error;
-    *error_message += ":<" + errnum + ">";
-    free(error);
-    return false;
-  }
-  LOG(INFO) << "Privilege level checked";
-  return true;
-}
-
 }  // namespace
 
 namespace common_installer {
 namespace security {
 
-Step::Status ValidateSignatures(const bf::path& base_path,
-    PrivilegeLevel* level, common_installer::CertificateInfo* cert_info,
-    bool check_reference, std::string* error_message) {
-  // Find signature files
-  ValidationCore::SignatureFileInfoSet signature_files;
-  ValidationCore::SignatureFinder signature_finder(base_path.string());
-  if (signature_finder.find(signature_files) !=
-      ValidationCore::SignatureFinder::NO_ERROR) {
-    LOG(ERROR) << "Error while searching for signatures";
-    return Step::Status::ERROR;
-  }
-  LOG(INFO) << "Number of signature files: " << signature_files.size();
-
-  // Read xml schema for signatures
-  for (auto& file_info : signature_files) {
-    std::string error;
-    Step::Status status = ValidateSignatureFile(base_path, file_info, level,
-                                        cert_info, check_reference, &error);
-    if (status != Step::Status::OK) {
-      *error_message = error;
-      return status;
-    }
-  }
-  return Step::Status::OK;
-}
-
 Step::Status StepCheckSignature::precheck() {
   if (context_->unpacked_dir_path.get().empty()) {
     LOG(ERROR) << "unpacked_dir_path attribute is empty";
@@ -238,23 +53,19 @@ Step::Status StepCheckSignature::precheck() {
   return Step::Status::OK;
 }
 
-Step::Status StepCheckSignature::process() {
-  PrivilegeLevel level = PrivilegeLevel::UNTRUSTED;
+Step::Status StepCheckSignature::CheckSignatures(bool check_reference,
+                                                 PrivilegeLevel* level) {
   std::string error_message;
-  bool check_reference = true;
-  if (getuid() == 0 &&
-      (context_->request_type.get()== ci::RequestType::ManifestDirectInstall ||
-      context_->request_type.get() == ci::RequestType::ManifestDirectUpdate))
-    check_reference = false;
-  Status status =
-      ValidateSignatures(context_->unpacked_dir_path.get(), &level,
+  if (!ValidateSignatures(context_->unpacked_dir_path.get(), level,
                          &context_->certificate_info.get(), check_reference,
-                         &error_message);
-  if (status != Status::OK) {
-    on_error(status, error_message);
-    return status;
+                         &error_message)) {
+    on_error(Status::CERT_ERROR, error_message);
+    return Status::CERT_ERROR;
   }
+  return Status::OK;
+}
 
+Step::Status StepCheckSignature::CheckSignatureMismatch() {
   const auto& cert = context_->certificate_info.get().author_certificate.get();
   if (cert) {
     bool certificate_mismatch =
@@ -267,16 +78,13 @@ Step::Status StepCheckSignature::process() {
       return Status::CERT_ERROR;
     }
   }
+  return Status::OK;
+}
 
-  if (context_->is_preload_request.get())
-    level = PrivilegeLevel::PLATFORM;
-
-  LOG(INFO) << "Privilege level: " << PrivilegeLevelToString(level);
-  context_->privilege_level.set(level);
-
+Step::Status StepCheckSignature::CheckPrivilegeLevel(PrivilegeLevel level) {
   // TODO(t.iwanek): refactoring, move to wgt backend
   bool is_webapp = context_->pkg_type.get() == "wgt";
-  error_message.clear();
+  std::string error_message;
   if (!context_->is_preload_request.get()) {
     if (!ValidatePrivilegeLevel(level, is_webapp,
         context_->manifest_data.get()->api_version,
@@ -288,6 +96,34 @@ Step::Status StepCheckSignature::process() {
       return Status::SIGNATURE_ERROR;
     }
   }
+  return Status::OK;
+}
+
+Step::Status StepCheckSignature::process() {
+  PrivilegeLevel level = PrivilegeLevel::UNTRUSTED;
+  bool check_reference = true;
+  if (getuid() == 0 &&
+      (context_->request_type.get()== ci::RequestType::ManifestDirectInstall ||
+      context_->request_type.get() == ci::RequestType::ManifestDirectUpdate))
+    check_reference = false;
+
+  Status status = CheckSignatures(check_reference, &level);
+  if (status != Status::OK)
+    return status;
+
+  status = CheckSignatureMismatch();
+  if (status != Status::OK)
+    return status;
+
+  if (context_->is_preload_request.get())
+    level = PrivilegeLevel::PLATFORM;
+
+  LOG(INFO) << "Privilege level: " << PrivilegeLevelToString(level);
+  context_->privilege_level.set(level);
+
+  status = CheckPrivilegeLevel(level);
+  if (status != Status::OK)
+    return status;
 
   LOG(INFO) << "Signature done";
   return Status::OK;
index bd4a24c..1fd82a5 100644 (file)
@@ -41,14 +41,14 @@ class StepCheckSignature : public Step {
    */
   Status precheck() override;
 
+ private:
+  Status CheckSignatures(bool check_reference, PrivilegeLevel* level);
+  Status CheckSignatureMismatch();
+  Status CheckPrivilegeLevel(PrivilegeLevel level);
+
   SCOPE_LOG_TAG(Signature)
 };
 
-// Exposed for tests
-Step::Status ValidateSignatures(const boost::filesystem::path& base_path,
-    PrivilegeLevel* level, common_installer::CertificateInfo* cert_info,
-    bool check_reference, std::string* error_message);
-
 }  // namespace security
 }  // namespace common_installer
 
index 68df0eb..2d4f5a8 100644 (file)
@@ -7,7 +7,7 @@
 
 #include <memory>
 
-#include "common/step/security/step_check_signature.h"
+#include "common/certificate_validation.h"
 
 namespace bf = boost::filesystem;
 
@@ -26,8 +26,8 @@ TEST_F(SignatureValidatorTest, HandlesInitializedSignatureDir) {
   PrivilegeLevel level = PrivilegeLevel::UNTRUSTED;
   common_installer::CertificateInfo cert_info;
   std::string error;
-  EXPECT_EQ(ValidateSignatures(*signature_file, &level, &cert_info, true,
-                               &error), Step::Status::OK);
+  EXPECT_TRUE(ValidateSignatures(*signature_file, &level, &cert_info, true,
+                               &error));
 }
 
 // Tests signature verifier with signature directory containing bad signatures
@@ -37,8 +37,8 @@ TEST_F(SignatureValidatorTest, HandlesBadSignatureDir) {
   PrivilegeLevel level = PrivilegeLevel::UNTRUSTED;
   common_installer::CertificateInfo cert_info;
   std::string error;
-  EXPECT_EQ(ValidateSignatures(*signature_file, &level, &cert_info, true,
-                               &error), Step::Status::ERROR);
+  EXPECT_FALSE(ValidateSignatures(*signature_file, &level, &cert_info, true,
+                               &error));
 }
 
 }  // namespace security