Remove obsolete NSS version checks.

Remove obsolete NSS version checks.

BUG=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245646

[Ryan Sleevi, 2014-01-17 04:00:10]

LGTM
On Jan 16, 2014 6:42 PM, <mattm@chromium.org> wrote:

> Reviewers: Ryan Sleevi,
>
> Description:
> Remove obsolete NSS version checks.
>
> BUG=none
>
> Please review this at https://codereview.chromium.org/141503002/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+3, -43 lines):
>   M chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
>   M net/cert/cert_verify_proc_nss.cc
>   M net/cert/nss_cert_database_unittest.cc
>
>
> Index: chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
> diff --git
a/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
> b/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
> index 002cb87d9351ce44679ddd1d88c177ad51c1aff6..
> 11ae9e51d22cca46bf44dd3679958fce6de550cc 100644
> --- a/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
> +++ b/chrome/browser/chromeos/policy/policy_cert_verifier_browsertest.cc
> @@ -173,10 +173,7 @@ TEST_F(PolicyCertVerifierTest, VerifyTrustedCert) {
>  }
>
>  TEST_F(PolicyCertVerifierTest, VerifyUsingAdditionalTrustAnchor) {
> -  if (!SupportsAdditionalTrustAnchors()) {
> -    LOG(INFO) << "Test skipped on this platform. NSS >= 3.14.2 required.";
> -    return;
> -  }
> +  ASSERT_TRUE(SupportsAdditionalTrustAnchors());
>
>    // |test_server_cert_| is untrusted, so Verify() fails.
>    {
> Index: net/cert/cert_verify_proc_nss.cc
> diff --git a/net/cert/cert_verify_proc_nss.cc b/net/cert/cert_verify_proc_
> nss.cc
> index c401eba7837a6bed9379f63ca9bf146259f3d312..
> e48888218b523da7786d719f724000b30efa0a8c 100644
> --- a/net/cert/cert_verify_proc_nss.cc
> +++ b/net/cert/cert_verify_proc_nss.cc
> @@ -33,17 +33,6 @@
>  #include "net/cert/x509_util_ios.h"
>  #endif  // defined(OS_IOS)
>
> -#define NSS_VERSION_NUM (NSS_VMAJOR * 10000 + NSS_VMINOR * 100 +
> NSS_VPATCH)
> -#if NSS_VERSION_NUM < 31305
> -// Added in NSS 3.13.5.
> -#define SEC_ERROR_CERT_SIGNATURE_ALGORITHM_DISABLED -8016
> -#endif
> -
> -#if NSS_VERSION_NUM < 31402
> -// Added in NSS 3.14.2.
> -#define cert_pi_useOnlyTrustAnchors static_cast<CERTValParamInType>(14)
> -#endif
> -
>  namespace net {
>
>  namespace {
> @@ -744,8 +733,7 @@ CertVerifyProcNSS::CertVerifyProcNSS() {}
>  CertVerifyProcNSS::~CertVerifyProcNSS() {}
>
>  bool CertVerifyProcNSS::SupportsAdditionalTrustAnchors() const {
> -  // This requires APIs introduced in 3.14.2.
> -  return NSS_VersionCheck("3.14.2");
> +  return true;
>  }
>
>  int CertVerifyProcNSS::VerifyInternal(
> @@ -801,7 +789,7 @@ int CertVerifyProcNSS::VerifyInternal(
>      verify_result->cert_status |= CERT_STATUS_REV_CHECKING_ENABLED;
>
>    ScopedCERTCertList trust_anchors;
> -  if (SupportsAdditionalTrustAnchors() && !additional_trust_anchors.empty())
> {
> +  if (!additional_trust_anchors.empty()) {
>      trust_anchors.reset(
>          CertificateListToCERTCertList(additional_trust_anchors));
>    }
> Index: net/cert/nss_cert_database_unittest.cc
> diff --git a/net/cert/nss_cert_database_unittest.cc
> b/net/cert/nss_cert_database_unittest.cc
> index f6958cf9875dda6880da9e3545b69847e53a5682..
> 39e1a33a498225b41dd4e0d27ef49a2357ed4dfb 100644
> --- a/net/cert/nss_cert_database_unittest.cc
> +++ b/net/cert/nss_cert_database_unittest.cc
> @@ -591,13 +591,6 @@ TEST_F(CertDatabaseNSSTest,
> ImportServerCert_SelfSigned) {
>  }
>
>  TEST_F(CertDatabaseNSSTest, ImportServerCert_SelfSigned_Trusted) {
> -  // When using CERT_PKIXVerifyCert (which we do), server trust only
> works from
> -  // 3.13.4 onwards.  See https://bugzilla.mozilla.org/
> show_bug.cgi?id=647364.
> -  if (!NSS_VersionCheck("3.13.4")) {
> -    LOG(INFO) << "test skipped on NSS < 3.13.4";
> -    return;
> -  }
> -
>    CertificateList certs;
>    ASSERT_TRUE(ReadCertIntoList("punycodetest.der", &certs));
>
> @@ -666,12 +659,6 @@ TEST_F(CertDatabaseNSSTest, ImportCaAndServerCert) {
>  }
>
>  TEST_F(CertDatabaseNSSTest, ImportCaAndServerCert_DistrustServer) {
> -  // Explicit distrust only works starting in NSS 3.13.
> -  if (!NSS_VersionCheck("3.13")) {
> -    LOG(INFO) << "test skipped on NSS < 3.13";
> -    return;
> -  }
> -
>    CertificateList ca_certs = CreateCertificateListFromFile(
>        GetTestCertsDirectory(), "root_ca_cert.pem",
>        X509Certificate::FORMAT_AUTO);
> @@ -760,12 +747,6 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa) {
>    EXPECT_EQ(OK, error);
>    EXPECT_EQ(0U, verify_result.cert_status);
>
> -  // Explicit distrust only works starting in NSS 3.13.
> -  if (!NSS_VersionCheck("3.13")) {
> -    LOG(INFO) << "test partially skipped on NSS < 3.13";
> -    return;
> -  }
> -
>    // Trust the root cert and distrust the intermediate.
>    EXPECT_TRUE(cert_db_->SetCertTrust(
>        ca_certs[0].get(), CA_CERT, NSSCertDatabase::TRUSTED_SSL));
> @@ -929,12 +910,6 @@ TEST_F(CertDatabaseNSSTest, TrustIntermediateCa3) {
>  }
>
>  TEST_F(CertDatabaseNSSTest, TrustIntermediateCa4) {
> -  // Explicit distrust only works starting in NSS 3.13.
> -  if (!NSS_VersionCheck("3.13")) {
> -    LOG(INFO) << "test skipped on NSS < 3.13";
> -    return;
> -  }
> -
>    NSSCertDatabase::ImportCertFailureList failed;
>
>    CertificateList ca_certs = CreateCertificateListFromFile(
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[mattm, 2014-01-17 04:07:54]

+joaodasilva for chrome/browser/chromeos/policy/OWNERS

[Joao da Silva, 2014-01-17 09:04:52]

lgtm

https://codereview.chromium.org/141503002/diff/1/net/cert/cert_verify_proc_ns...
File net/cert/cert_verify_proc_nss.cc (right):

https://codereview.chromium.org/141503002/diff/1/net/cert/cert_verify_proc_ns...
net/cert/cert_verify_proc_nss.cc:736: return true;
Why not remove this function altogether?

[Joao da Silva, 2014-01-17 13:22:21]

https://codereview.chromium.org/141503002/diff/1/net/cert/cert_verify_proc_ns...
File net/cert/cert_verify_proc_nss.cc (right):

https://codereview.chromium.org/141503002/diff/1/net/cert/cert_verify_proc_ns...
net/cert/cert_verify_proc_nss.cc:736: return true;
On 2014/01/17 09:04:53, Joao da Silva wrote:
> Why not remove this function altogether?

Meh, ignore this comment. Other implementations return false.

[commit-bot: I haz the power, 2014-01-17 20:14:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mattm@chromium.org/141503002/1

[commit-bot: I haz the power, 2014-01-17 22:38:03]

Change committed as 245646