diff --git a/security/pkix/include/pkix/Input.h b/security/pkix/include/pkix/Input.h index dc0c04b69a2d..b924ab4ae93b 100644 --- a/security/pkix/include/pkix/Input.h +++ b/security/pkix/include/pkix/Input.h @@ -80,6 +80,9 @@ public: { } + // This is intentionally not explicit in order to allow value semantics. + Input(const Input&) = default; + // Initialize the input. data must be non-null and len must be less than // 65536. Init may not be called more than once. Result Init(const uint8_t* data, size_t len) @@ -271,7 +274,7 @@ public: Result SkipToEnd(/*out*/ Input& skipped) { - return Skip(static_cast(end - input), skipped); + return Skip(static_cast(end - input), skipped); } Result EnsureLength(Input::size_type len) @@ -286,6 +289,8 @@ public: class Mark final { + public: + Mark(const Mark&) = default; // Intentionally not explicit. private: friend class Reader; Mark(const Reader& input, const uint8_t* mark) : input(input), mark(mark) { } diff --git a/security/pkix/lib/pkixder.h b/security/pkix/lib/pkixder.h index 2f6276c614e2..356264bd829f 100644 --- a/security/pkix/lib/pkixder.h +++ b/security/pkix/lib/pkixder.h @@ -55,7 +55,7 @@ enum Constructed CONSTRUCTED = 1 << 5 }; -enum Tag +enum Tag : uint8_t { BOOLEAN = UNIVERSAL | 0x01, INTEGER = UNIVERSAL | 0x02, diff --git a/security/pkix/lib/pkixnames.cpp b/security/pkix/lib/pkixnames.cpp index 8517adfdf3d0..ca9ca459e933 100644 --- a/security/pkix/lib/pkixnames.cpp +++ b/security/pkix/lib/pkixnames.cpp @@ -1642,13 +1642,14 @@ FinishIPv6Address(/*in/out*/ uint8_t (&address)[16], int numComponents, } // Shift components that occur after the contraction over. - int componentsToMove = numComponents - contractionIndex; - memmove(address + (2u * (8 - componentsToMove)), - address + (2u * contractionIndex), + size_t componentsToMove = static_cast(numComponents - + contractionIndex); + memmove(address + (2u * static_cast(8 - componentsToMove)), + address + (2u * static_cast(contractionIndex)), componentsToMove * 2u); // Fill in the contracted area with zeros. - memset(address + (2u * contractionIndex), 0u, - (8u - numComponents) * 2u); + memset(address + (2u * static_cast(contractionIndex)), 0u, + (8u - static_cast(numComponents)) * 2u); return true; } @@ -1785,7 +1786,6 @@ ParseIPv6Address(Input hostname, /*out*/ uint8_t (&out)[16]) if (contractionIndex != -1) { return false; // multiple contractions are not allowed. } - uint8_t b; if (input.Read(b) != Success || b != ':') { assert(false); return false; diff --git a/security/pkix/lib/pkixtime.cpp b/security/pkix/lib/pkixtime.cpp index 499784e4ecb3..ace23dd411f8 100644 --- a/security/pkix/lib/pkixtime.cpp +++ b/security/pkix/lib/pkixtime.cpp @@ -24,8 +24,15 @@ #include "pkix/Time.h" #include "pkixutil.h" + #ifdef WIN32 +#ifdef _MSC_VER +#pragma warning(push, 3) +#endif #include "windows.h" +#ifdef _MSC_VER +#pragma warning(pop) +#endif #else #include "sys/time.h" #endif @@ -53,7 +60,8 @@ Now() // - http://pubs.opengroup.org/onlinepubs/009695399/functions/gettimeofday.html timeval tv; (void) gettimeofday(&tv, nullptr); - seconds = (DaysBeforeYear(1970) * Time::ONE_DAY_IN_SECONDS) + tv.tv_sec; + seconds = (DaysBeforeYear(1970) * Time::ONE_DAY_IN_SECONDS) + + static_cast(tv.tv_sec); #endif return TimeFromElapsedSecondsAD(seconds); diff --git a/security/pkix/lib/pkixutil.h b/security/pkix/lib/pkixutil.h index 165d9a6b2086..3515ff3c45d1 100644 --- a/security/pkix/lib/pkixutil.h +++ b/security/pkix/lib/pkixutil.h @@ -53,7 +53,7 @@ public: Result Init(); const Input GetDER() const { return der; } - const der::Version GetVersion() const { return version; } + der::Version GetVersion() const { return version; } const SignedDataWithSignature& GetSignedData() const { return signedData; } const Input GetIssuer() const { return issuer; } // XXX: "validity" is a horrible name for the structure that holds @@ -232,24 +232,25 @@ WrappedVerifySignedData(TrustDomain& trustDomain, // MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM // } // } -#if defined(__clang__) && (__clang_major__ == 3 && __clang_minor__ < 5) - // Earlier versions of Clang will warn if not all cases are covered - // (-Wswitch-enum) AND they always, inappropriately, assume the default case - // is unreachable. This was fixed in - // http://llvm.org/klaus/clang/commit/28cd22d7c2d2458575ce9cc19dfe63c6321010ce/ -# define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM // empty -#elif defined(__GNUC__) || defined(__clang__) - // GCC and recent versions of clang will warn if not all cases are covered - // (-Wswitch-enum). They do not assume that the default case is unreachable. -# define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \ - default: assert(false); __builtin_unreachable(); +#if defined(__clang__) +// Clang will warn if not all cases are covered (-Wswitch-enum) AND it will +// warn if a switch statement that covers every enum label has a default case +// (-W-covered-switch-default). Versions prior to 3.5 warned about unreachable +// code in such default cases (-Wunreachable-code) even when +// -W-covered-switch-default was disabled, but that changed in Clang 3.5. +#define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM // empty +#elif defined(__GNUC__) +// GCC will warn if not all cases are covered (-Wswitch-enum). It does not +// assume that the default case is unreachable. +#define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \ + default: assert(false); __builtin_unreachable(); #elif defined(_MSC_VER) - // MSVC will warn if not all cases are covered (C4061, level 4). It does not - // assume that the default case is unreachable. -# define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \ - default: assert(false); __assume(0); +// MSVC will warn if not all cases are covered (C4061, level 4). It does not +// assume that the default case is unreachable. +#define MOZILLA_PKIX_UNREACHABLE_DEFAULT_ENUM \ + default: assert(false); __assume(0); #else -# error Unsupported compiler for MOZILLA_PKIX_UNREACHABLE_DEFAULT. +#error Unsupported compiler for MOZILLA_PKIX_UNREACHABLE_DEFAULT. #endif } } // namespace mozilla::pkix diff --git a/security/pkix/moz.build b/security/pkix/moz.build index 9ebaee7cc4d3..f93520526e81 100644 --- a/security/pkix/moz.build +++ b/security/pkix/moz.build @@ -25,18 +25,7 @@ TEST_DIRS += [ 'test/lib', ] -CXXFLAGS += ['-Wall'] -# -Wall with Visual C++ enables too many problematic warnings -if CONFIG['_MSC_VER']: - CXXFLAGS += [ - '-wd4514', # 'function': unreferenced inline function has been removed - '-wd4668', # 'symbol' is not defined as a preprocessor macro... - '-wd4710', # 'function': function not inlined - '-wd4711', # function 'function' selected for inline expansion - '-wd4820', # 'bytes' bytes padding added after construct 'member_name' - ] - -FAIL_ON_WARNINGS = True +include('warnings.mozbuild') Library('mozillapkix') diff --git a/security/pkix/test/gtest/moz.build b/security/pkix/test/gtest/moz.build index 92ac437d7942..b0069b7af95f 100644 --- a/security/pkix/test/gtest/moz.build +++ b/security/pkix/test/gtest/moz.build @@ -30,11 +30,24 @@ LOCAL_INCLUDES += [ FINAL_LIBRARY = 'xul-gtest' -include('/ipc/chromium/chromium-config.mozbuild') +include('../../warnings.mozbuild') -if CONFIG['_MSC_VER']: +# These warnings are disabled in order to minimize the amount of boilerplate +# required to implement tests, and/or because they originate in the GTest +# framework in a way we cannot otherwise work around. +if CONFIG['CLANG_CXX']: CXXFLAGS += [ - '-wd4275', # non dll-interface class used as base for dll-interface class + '-Wno-exit-time-destructors', + '-Wno-global-constructors', + '-Wno-old-style-cast', + '-Wno-used-but-marked-unused', + ] +elif CONFIG['_MSC_VER']: + CXXFLAGS += [ + '-wd4350', # behavior change: 'std::_Wrap_alloc>::... + '-wd4275', # non dll-interface class used as base for dll-interface class + '-wd4548', # Expression before comma has no effect + '-wd4625', # copy constructor could not be generated. + '-wd4626', # assugment operator could not be generated. + '-wd4640', # construction of local static object is not thread safe. ] - -FAIL_ON_WARNINGS = True diff --git a/security/pkix/test/gtest/pkixbuild_tests.cpp b/security/pkix/test/gtest/pkixbuild_tests.cpp index 2e96584f2089..6bf6e9b8487b 100644 --- a/security/pkix/test/gtest/pkixbuild_tests.cpp +++ b/security/pkix/test/gtest/pkixbuild_tests.cpp @@ -22,8 +22,20 @@ * limitations under the License. */ +#if defined(_MSC_VER) && _MSC_VER < 1900 +// When building with -D_HAS_EXCEPTIONS=0, MSVC's header triggers +// warning C4702: unreachable code. +// https://connect.microsoft.com/VisualStudio/feedback/details/809962 +#pragma warning(push) +#pragma warning(disable: 4702) +#endif + #include -#include "cert.h" + +#if defined(_MSC_VER) && _MSC_VER < 1900 +#pragma warning(pop) +#endif + #include "pkix/pkix.h" #include "pkixgtest.h" #include "pkixtestutil.h" @@ -110,7 +122,7 @@ private: return Success; } - Result FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time time) + Result FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time) override { ByteString subjectDER(InputToByteString(encodedIssuerName)); @@ -147,8 +159,7 @@ private: return TestVerifySignedData(signedData, subjectPublicKeyInfo); } - Result DigestBuf(Input item, /*out*/ uint8_t *digestBuf, size_t digestBufLen) - override + Result DigestBuf(Input, /*out*/ uint8_t*, size_t) override { ADD_FAILURE(); return Result::FATAL_ERROR_LIBRARY_FAILURE; @@ -261,9 +272,8 @@ public: } // The CertPolicyId argument is unused because we don't care about EV. - Result GetCertTrust(EndEntityOrCA endEntityOrCA, const CertPolicyId&, - Input candidateCert, /*out*/ TrustLevel& trustLevel) - override + Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input candidateCert, + /*out*/ TrustLevel& trustLevel) override { Input rootCert; Result rv = rootCert.Init(rootDER.data(), rootDER.length()); @@ -278,8 +288,7 @@ public: return Success; } - Result FindIssuer(Input encodedIssuerName, IssuerChecker& checker, Time time) - override + Result FindIssuer(Input, IssuerChecker& checker, Time) override { // keepGoing is an out parameter from IssuerChecker.Check. It would tell us // whether or not to continue attempting other potential issuers. We only @@ -343,7 +352,7 @@ TEST_F(pkixbuild, NoRevocationCheckingForExpiredCert) ByteString certDER(CreateEncodedCertificate( v3, sha256WithRSAEncryption, serialNumber, issuerDER, - oneDayBeforeNow - Time::ONE_DAY_IN_SECONDS, + oneDayBeforeNow - ONE_DAY_IN_SECONDS_AS_TIME_T, oneDayBeforeNow, subjectDER, *reusedKey, nullptr, *reusedKey, sha256WithRSAEncryption)); @@ -389,8 +398,7 @@ public: return Success; } - Result VerifySignedData(const SignedDataWithSignature& signedData, - Input subjectPublicKeyInfo) override + Result VerifySignedData(const SignedDataWithSignature&, Input) override { ADD_FAILURE(); return Result::FATAL_ERROR_LIBRARY_FAILURE; @@ -463,7 +471,7 @@ public: return Success; } - Result FindIssuer(Input subjectCert, IssuerChecker& checker, Time) override + Result FindIssuer(Input, IssuerChecker& checker, Time) override { Input issuerInput; EXPECT_EQ(Success, issuerInput.Init(issuer.data(), issuer.length())); diff --git a/security/pkix/test/gtest/pkixcert_extension_tests.cpp b/security/pkix/test/gtest/pkixcert_extension_tests.cpp index ad722ed58c8d..1b01f7e45740 100644 --- a/security/pkix/test/gtest/pkixcert_extension_tests.cpp +++ b/security/pkix/test/gtest/pkixcert_extension_tests.cpp @@ -63,7 +63,7 @@ CreateCertWithOneExtension(const char* subjectStr, const ByteString& extension) class TrustEverythingTrustDomain final : public TrustDomain { private: - Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input candidateCert, + Result GetCertTrust(EndEntityOrCA, const CertPolicyId&, Input, /*out*/ TrustLevel& trustLevel) override { trustLevel = TrustLevel::TrustAnchor; diff --git a/security/pkix/test/gtest/pkixcheck_CheckKeyUsage_tests.cpp b/security/pkix/test/gtest/pkixcheck_CheckKeyUsage_tests.cpp index b939ca56c51b..54b4cd9e601b 100644 --- a/security/pkix/test/gtest/pkixcheck_CheckKeyUsage_tests.cpp +++ b/security/pkix/test/gtest/pkixcheck_CheckKeyUsage_tests.cpp @@ -22,7 +22,7 @@ * limitations under the License. */ -#include "gtest/gtest.h" +#include "pkixgtest.h" #include "pkix/pkixtypes.h" #include "pkixtestutil.h" diff --git a/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp b/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp index ad11b801e06d..d730d91e5bd0 100644 --- a/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp +++ b/security/pkix/test/gtest/pkixcheck_CheckValidity_tests.cpp @@ -22,7 +22,7 @@ * limitations under the License. */ -#include "gtest/gtest.h" +#include "pkixgtest.h" #include "pkix/pkixtypes.h" #include "pkixtestutil.h" diff --git a/security/pkix/test/gtest/pkixder_input_tests.cpp b/security/pkix/test/gtest/pkixder_input_tests.cpp index 56bee20820e2..adfefb4af6c7 100644 --- a/security/pkix/test/gtest/pkixder_input_tests.cpp +++ b/security/pkix/test/gtest/pkixder_input_tests.cpp @@ -24,7 +24,7 @@ #include #include -#include +#include "pkixgtest.h" #include "pkixder.h" @@ -467,7 +467,7 @@ TEST_F(pkixder_input_tests, MarkAndGetInput) } // Cannot run this test on debug builds because of the NotReached -#ifndef DEBUG +#ifdef NDEBUG TEST_F(pkixder_input_tests, MarkAndGetInputDifferentInput) { const uint8_t der[] = { 0x11, 0x22, 0x33, 0x44 }; @@ -847,7 +847,7 @@ TEST_F(pkixder_input_tests, NestedOf) [&readValues](Reader& r) { return NestedOfHelper(r, readValues); })); - ASSERT_EQ((size_t) 3, readValues.size()); + ASSERT_EQ(3u, readValues.size()); ASSERT_EQ(0x01, readValues[0]); ASSERT_EQ(0x02, readValues[1]); ASSERT_EQ(0x03, readValues[2]); @@ -865,7 +865,7 @@ TEST_F(pkixder_input_tests, NestedOfWithTruncatedData) [&readValues](Reader& r) { return NestedOfHelper(r, readValues); })); - ASSERT_EQ((size_t) 0, readValues.size()); + ASSERT_EQ(0u, readValues.size()); } TEST_F(pkixder_input_tests, MatchRestAtEnd) diff --git a/security/pkix/test/gtest/pkixder_pki_types_tests.cpp b/security/pkix/test/gtest/pkixder_pki_types_tests.cpp index 917d9ab64c94..16802c7d3ad7 100644 --- a/security/pkix/test/gtest/pkixder_pki_types_tests.cpp +++ b/security/pkix/test/gtest/pkixder_pki_types_tests.cpp @@ -25,7 +25,7 @@ #include #include -#include "gtest/gtest.h" +#include "pkixgtest.h" #include "pkix/pkixtypes.h" #include "pkixder.h" diff --git a/security/pkix/test/gtest/pkixder_universal_types_tests.cpp b/security/pkix/test/gtest/pkixder_universal_types_tests.cpp index 52a3562e26d5..7fd4163a2d29 100644 --- a/security/pkix/test/gtest/pkixder_universal_types_tests.cpp +++ b/security/pkix/test/gtest/pkixder_universal_types_tests.cpp @@ -24,7 +24,7 @@ #include #include -#include +#include "pkixgtest.h" #include "pkixder.h" #include "pkixtestutil.h" @@ -287,8 +287,9 @@ TEST_F(pkixder_universal_types_tests, EnumeratedInvalidZeroLength) // other encodings is actually encouraged. // e.g. TWO_CHARS(53) => '5', '3' -#define TWO_CHARS(t) static_cast('0' + ((t) / 10u)), \ - static_cast('0' + ((t) % 10u)) +#define TWO_CHARS(t) \ + static_cast('0' + (static_cast(t) / 10u)), \ + static_cast('0' + (static_cast(t) % 10u)) // Calls TimeChoice on the UTCTime variant of the given generalized time. template @@ -555,7 +556,7 @@ static const uint8_t DAYS_IN_MONTH[] = { TEST_F(pkixder_universal_types_tests, TimeMonthDaysValidRange) { - for (uint8_t month = 1; month <= 12; ++month) { + for (uint16_t month = 1; month <= 12; ++month) { for (uint8_t day = 1; day <= DAYS_IN_MONTH[month]; ++day) { const uint8_t DER[] = { 0x18, // Generalized Time @@ -604,7 +605,7 @@ TEST_F(pkixder_universal_types_tests, TimeDayInvalid0) TEST_F(pkixder_universal_types_tests, TimeMonthDayInvalidPastEndOfMonth) { - for (uint8_t month = 1; month <= 12; ++month) { + for (int16_t month = 1; month <= 12; ++month) { const uint8_t DER[] = { 0x18, // Generalized Time 15, // Length = 15 @@ -905,7 +906,7 @@ TEST_F(pkixder_universal_types_tests, Integer_0_127) Input input(DER); Reader reader(input); - uint8_t value = i + 1; // initialize with a value that is NOT i. + uint8_t value = i + 1u; // initialize with a value that is NOT i. ASSERT_EQ(Success, Integer(reader, value)); ASSERT_EQ(i, value); } diff --git a/security/pkix/test/gtest/pkixgtest.cpp b/security/pkix/test/gtest/pkixgtest.cpp index 35bcd41b07b6..c1e4081caf33 100644 --- a/security/pkix/test/gtest/pkixgtest.cpp +++ b/security/pkix/test/gtest/pkixgtest.cpp @@ -30,10 +30,15 @@ namespace mozilla { namespace pkix { namespace test { +const std::time_t ONE_DAY_IN_SECONDS_AS_TIME_T = + static_cast(Time::ONE_DAY_IN_SECONDS); + // This assumes that time/time_t are POSIX-compliant in that time() returns // the number of seconds since the Unix epoch. const std::time_t now(time(nullptr)); -const std::time_t oneDayBeforeNow(time(nullptr) - Time::ONE_DAY_IN_SECONDS); -const std::time_t oneDayAfterNow(time(nullptr) + Time::ONE_DAY_IN_SECONDS); +const std::time_t oneDayBeforeNow(time(nullptr) - + ONE_DAY_IN_SECONDS_AS_TIME_T); +const std::time_t oneDayAfterNow(time(nullptr) + + ONE_DAY_IN_SECONDS_AS_TIME_T); } } } // namespace mozilla::pkix::test diff --git a/security/pkix/test/gtest/pkixgtest.h b/security/pkix/test/gtest/pkixgtest.h index 5e303b0baaef..84a7b085039c 100644 --- a/security/pkix/test/gtest/pkixgtest.h +++ b/security/pkix/test/gtest/pkixgtest.h @@ -26,7 +26,36 @@ #include +#if defined(__clang__) +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wdeprecated" +#pragma clang diagnostic ignored "-Wmissing-noreturn" +#pragma clang diagnostic ignored "-Wshift-sign-overflow" +#pragma clang diagnostic ignored "-Wsign-conversion" +#pragma clang diagnostic ignored "-Wundef" +#elif defined(__GNUC__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wextra" +#elif defined(_MSC_VER) +#pragma warning(push, 3) +// C4224: Nonstandard extension used: formal parameter 'X' was previously +// defined as a type. +#pragma warning(disable: 4224) +// C4826: Conversion from 'type1 ' to 'type_2' is sign - extended. This may +// cause unexpected runtime behavior. +#pragma warning(disable: 4826) +#endif + #include "gtest/gtest.h" + +#if defined(__clang__) +#pragma clang diagnostic pop +#elif defined(__GNUC__) +#pragma GCC diagnostic pop +#elif defined(_MSC_VER) +#pragma warning(pop) +#endif + #include "pkix/Result.h" // PrintTo must be in the same namespace as the type we're overloading it for. @@ -47,6 +76,8 @@ PrintTo(const Result& result, ::std::ostream* os) namespace mozilla { namespace pkix { namespace test { +extern const std::time_t ONE_DAY_IN_SECONDS_AS_TIME_T; + extern const std::time_t now; extern const std::time_t oneDayBeforeNow; extern const std::time_t oneDayAfterNow; diff --git a/security/pkix/test/gtest/pkixnames_tests.cpp b/security/pkix/test/gtest/pkixnames_tests.cpp index cf2838fcc65b..9341a3a4320a 100644 --- a/security/pkix/test/gtest/pkixnames_tests.cpp +++ b/security/pkix/test/gtest/pkixnames_tests.cpp @@ -72,7 +72,8 @@ struct PresentedMatchesReference { \ ByteString(reinterpret_cast(a), sizeof(a) - 1), \ ByteString(reinterpret_cast(b), sizeof(b) - 1), \ - Result::ERROR_BAD_DER \ + Result::ERROR_BAD_DER, \ + false \ } static const PresentedMatchesReference DNSID_MATCH_PARAMS[] = diff --git a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp index 989e8137adf6..624995031d27 100644 --- a/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp +++ b/security/pkix/test/gtest/pkixocsp_CreateEncodedOCSPRequest_tests.cpp @@ -22,7 +22,7 @@ * limitations under the License. */ -#include "gtest/gtest.h" +#include "pkixgtest.h" #include "pkix/pkix.h" #include "pkixder.h" #include "pkixtestutil.h" @@ -71,7 +71,7 @@ private: return TestDigestBuf(item, digestBuf, digestBufLen); } - Result CheckPublicKey(Input subjectPublicKeyInfo) override + Result CheckPublicKey(Input) override { ADD_FAILURE(); return Result::FATAL_ERROR_LIBRARY_FAILURE; diff --git a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp index c55e08dbb354..97aaa2d19b89 100644 --- a/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp +++ b/security/pkix/test/gtest/pkixocsp_VerifyEncodedOCSPResponse.cpp @@ -53,7 +53,7 @@ public: return Result::FATAL_ERROR_LIBRARY_FAILURE; } - Result CheckRevocation(EndEntityOrCA endEntityOrCA, const CertID&, Time time, + Result CheckRevocation(EndEntityOrCA, const CertID&, Time, /*optional*/ const Input*, /*optional*/ const Input*) final override { @@ -119,7 +119,8 @@ public: abort(); } - serialNumberDER = CreateEncodedSerialNumber(++rootIssuedCount); + serialNumberDER = + CreateEncodedSerialNumber(static_cast(++rootIssuedCount)); if (ENCODING_FAILED(serialNumberDER)) { abort(); } @@ -143,7 +144,7 @@ public: } static ScopedTestKeyPair rootKeyPair; - static long rootIssuedCount; + static uint32_t rootIssuedCount; OCSPTestTrustDomain trustDomain; // endEntityCertID references rootKeyPair, rootNameDER, and serialNumberDER. @@ -154,7 +155,7 @@ public: }; /*static*/ ScopedTestKeyPair pkixocsp_VerifyEncodedResponse::rootKeyPair; -/*static*/ long pkixocsp_VerifyEncodedResponse::rootIssuedCount = 0; +/*static*/ uint32_t pkixocsp_VerifyEncodedResponse::rootIssuedCount = 0; /////////////////////////////////////////////////////////////////////////////// // responseStatus @@ -258,7 +259,7 @@ public: context.signatureAlgorithm = signatureAlgorithm; context.certs = certs; - context.certStatus = certStatus; + context.certStatus = static_cast(certStatus); context.thisUpdate = thisUpdate; context.nextUpdate = nextUpdate ? *nextUpdate : 0; context.includeNextUpdate = nextUpdate != nullptr; @@ -408,7 +409,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_successful, check_validThrough) ASSERT_FALSE(expired); // The response was created to be valid until one day after now, so the // value we got for validThrough should be after that. - Time oneDayAfterNowAsPKIXTime(TimeFromEpochInSeconds(oneDayAfterNow)); + Time oneDayAfterNowAsPKIXTime( + TimeFromEpochInSeconds(static_cast(oneDayAfterNow))); ASSERT_TRUE(validThrough > oneDayAfterNowAsPKIXTime); } { @@ -518,7 +520,8 @@ protected: /*optional*/ const ByteString* extensions, const TestKeyPair& signerKeyPair) { - ByteString serialNumberDER(CreateEncodedSerialNumber(serialNumber)); + ByteString serialNumberDER(CreateEncodedSerialNumber( + static_cast(serialNumber))); if (ENCODING_FAILED(serialNumberDER)) { return ByteString(); } @@ -634,8 +637,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_expired) ScopedTestKeyPair signerKeyPair(GenerateKeyPair()); ByteString signerDER(CreateEncodedCertificate( ++rootIssuedCount, sha256WithRSAEncryption, rootName, - now - (10 * Time::ONE_DAY_IN_SECONDS), - now - (2 * Time::ONE_DAY_IN_SECONDS), + now - (10 * ONE_DAY_IN_SECONDS_AS_TIME_T), + now - (2 * ONE_DAY_IN_SECONDS_AS_TIME_T), signerName, *signerKeyPair, extensions, *rootKeyPair)); ASSERT_FALSE(ENCODING_FAILED(signerDER)); @@ -670,8 +673,8 @@ TEST_F(pkixocsp_VerifyEncodedResponse_DelegatedResponder, good_future) ByteString signerDER(CreateEncodedCertificate( ++rootIssuedCount, sha256WithRSAEncryption, rootName, - now + (2 * Time::ONE_DAY_IN_SECONDS), - now + (10 * Time::ONE_DAY_IN_SECONDS), + now + (2 * ONE_DAY_IN_SECONDS_AS_TIME_T), + now + (10 * ONE_DAY_IN_SECONDS_AS_TIME_T), signerName, *signerKeyPair, extensions, *rootKeyPair)); ASSERT_FALSE(ENCODING_FAILED(signerDER)); @@ -1017,13 +1020,14 @@ TEST_F(pkixocsp_VerifyEncodedResponse_GetCertTrust, ActivelyDistrusted) { ASSERT_TRUE(trustDomain.SetCertTrust(signerCertDER, TrustLevel::ActivelyDistrusted)); - Input response; + Input responseInput; ASSERT_EQ(Success, - response.Init(responseString.data(), responseString.length())); + responseInput.Init(responseString.data(), + responseString.length())); bool expired; ASSERT_EQ(Result::ERROR_OCSP_INVALID_SIGNING_CERT, VerifyEncodedOCSPResponse(trustDomain, *endEntityCertID, Now(), END_ENTITY_MAX_LIFETIME_IN_DAYS, - response, expired)); + responseInput, expired)); ASSERT_FALSE(expired); } diff --git a/security/pkix/test/lib/pkixtestnss.cpp b/security/pkix/test/lib/pkixtestnss.cpp index 85a350bc32f5..752ea46e44ce 100644 --- a/security/pkix/test/lib/pkixtestnss.cpp +++ b/security/pkix/test/lib/pkixtestnss.cpp @@ -115,7 +115,8 @@ public: } SECItem signatureItem; - if (SEC_SignData(&signatureItem, tbs.data(), tbs.length(), + if (SEC_SignData(&signatureItem, tbs.data(), + static_cast(tbs.length()), privateKey.get(), signatureAlgorithmOidTag) != SECSuccess) { return MapPRErrorCodeToResult(PR_GetError()); diff --git a/security/pkix/test/lib/pkixtestutil.cpp b/security/pkix/test/lib/pkixtestutil.cpp index da0a0be09a37..df23c56504d8 100644 --- a/security/pkix/test/lib/pkixtestutil.cpp +++ b/security/pkix/test/lib/pkixtestutil.cpp @@ -127,10 +127,10 @@ TLV(uint8_t tag, const ByteString& value) result.push_back(tag); if (value.length() < 128) { - result.push_back(value.length()); + result.push_back(static_cast(value.length())); } else if (value.length() < 256) { result.push_back(0x81u); - result.push_back(value.length()); + result.push_back(static_cast(value.length())); } else if (value.length() < 65536) { result.push_back(0x82u); result.push_back(static_cast(value.length() / 256)); @@ -158,7 +158,7 @@ OCSPResponseContext::OCSPResponseContext(const CertID& certID, time_t time) , certStatus(good) , revocationTime(0) , thisUpdate(time) - , nextUpdate(time + Time::ONE_DAY_IN_SECONDS) + , nextUpdate(time + static_cast(Time::ONE_DAY_IN_SECONDS)) , includeNextUpdate(true) { } @@ -202,7 +202,7 @@ ByteString Boolean(bool value) { ByteString encodedValue; - encodedValue.push_back(value ? 0xff : 0x00); + encodedValue.push_back(value ? 0xffu : 0x00u); return TLV(der::BOOLEAN, encodedValue); } @@ -266,22 +266,22 @@ TimeToEncodedTime(time_t time, TimeEncoding encoding) ByteString value; if (encoding == GeneralizedTime) { - value.push_back('0' + (year / 1000)); - value.push_back('0' + ((year % 1000) / 100)); + value.push_back(static_cast('0' + (year / 1000))); + value.push_back(static_cast('0' + ((year % 1000) / 100))); } - value.push_back('0' + ((year % 100) / 10)); - value.push_back('0' + (year % 10)); - value.push_back('0' + ((exploded.tm_mon + 1) / 10)); - value.push_back('0' + ((exploded.tm_mon + 1) % 10)); - value.push_back('0' + (exploded.tm_mday / 10)); - value.push_back('0' + (exploded.tm_mday % 10)); - value.push_back('0' + (exploded.tm_hour / 10)); - value.push_back('0' + (exploded.tm_hour % 10)); - value.push_back('0' + (exploded.tm_min / 10)); - value.push_back('0' + (exploded.tm_min % 10)); - value.push_back('0' + (exploded.tm_sec / 10)); - value.push_back('0' + (exploded.tm_sec % 10)); + value.push_back(static_cast('0' + ((year % 100) / 10))); + value.push_back(static_cast('0' + (year % 10))); + value.push_back(static_cast('0' + ((exploded.tm_mon + 1) / 10))); + value.push_back(static_cast('0' + ((exploded.tm_mon + 1) % 10))); + value.push_back(static_cast('0' + (exploded.tm_mday / 10))); + value.push_back(static_cast('0' + (exploded.tm_mday % 10))); + value.push_back(static_cast('0' + (exploded.tm_hour / 10))); + value.push_back(static_cast('0' + (exploded.tm_hour % 10))); + value.push_back(static_cast('0' + (exploded.tm_min / 10))); + value.push_back(static_cast('0' + (exploded.tm_min % 10))); + value.push_back(static_cast('0' + (exploded.tm_sec / 10))); + value.push_back(static_cast('0' + (exploded.tm_sec % 10))); value.push_back('Z'); return TLV(encoding == GeneralizedTime ? der::GENERALIZED_TIME : der::UTCTime, @@ -315,18 +315,15 @@ TimeToTimeChoice(time_t time) } Time -YMDHMS(int16_t year, int16_t month, int16_t day, - int16_t hour, int16_t minutes, int16_t seconds) +YMDHMS(uint16_t year, uint16_t month, uint16_t day, + uint16_t hour, uint16_t minutes, uint16_t seconds) { assert(year <= 9999); assert(month >= 1); assert(month <= 12); assert(day >= 1); - assert(hour >= 0); assert(hour < 24); - assert(minutes >= 0); assert(minutes < 60); - assert(seconds >= 0); assert(seconds < 60); uint64_t days = DaysBeforeYear(year); @@ -439,6 +436,31 @@ EmptyExtension(Input extnID, Critical critical) return TLV(der::SEQUENCE, encoded); } +std::string +GetEnv(const char* name) +{ + std::string result; + +#ifndef _MSC_VER + // XXX: Not thread safe. + const char* value = getenv(name); + if (value) { + result = value; + } +#else + char* value = nullptr; + size_t valueLength = 0; + if (_dupenv_s(&value, &valueLength, name) != 0) { + abort(); + } + if (value) { + result = value; + free(value); + } +#endif + return result; +} + void MaybeLogOutput(const ByteString& result, const char* suffix) { @@ -447,8 +469,8 @@ MaybeLogOutput(const ByteString& result, const char* suffix) // This allows us to more easily debug the generated output, by creating a // file in the directory given by MOZILLA_PKIX_TEST_LOG_DIR for each // NOT THREAD-SAFE!!! - const char* logPath = getenv("MOZILLA_PKIX_TEST_LOG_DIR"); - if (logPath) { + std::string logPath(GetEnv("MOZILLA_PKIX_TEST_LOG_DIR")); + if (!logPath.empty()) { static int counter = 0; std::ostringstream counterStream; @@ -829,7 +851,7 @@ BasicOCSPResponse(OCSPResponseContext& context) // value OCTET STRING // } static ByteString -OCSPExtension(OCSPResponseContext& context, OCSPResponseExtension& extension) +OCSPExtension(OCSPResponseExtension& extension) { ByteString encoded; encoded.append(extension.id); @@ -850,7 +872,7 @@ Extensions(OCSPResponseContext& context) ByteString value; for (OCSPResponseExtension* extension = context.extensions; extension; extension = extension->next) { - ByteString extensionEncoded(OCSPExtension(context, *extension)); + ByteString extensionEncoded(OCSPExtension(*extension)); if (ENCODING_FAILED(extensionEncoded)) { return ByteString(); } @@ -915,8 +937,11 @@ ResponderID(OCSPResponseContext& context) responderIDType = 2; // byKey } - return TLV(der::CONSTRUCTED | der::CONTEXT_SPECIFIC | responderIDType, - contents); + // XXX: MSVC 2015 wrongly warns about signed/unsigned conversion without the + // static_cast. + uint8_t tag = static_cast(der::CONSTRUCTED | der::CONTEXT_SPECIFIC | + responderIDType); + return TLV(tag, contents); } // KeyHash ::= OCTET STRING -- SHA-1 hash of responder's public key @@ -1047,7 +1072,10 @@ CertStatus(OCSPResponseContext& context) case 0: case 2: { - return TLV(der::CONTEXT_SPECIFIC | context.certStatus, ByteString()); + // XXX: MSVC 2015 wrongly warns about signed/unsigned conversion without + // the static cast. + return TLV(static_cast(der::CONTEXT_SPECIFIC | + context.certStatus), ByteString()); } case 1: { diff --git a/security/pkix/test/lib/pkixtestutil.h b/security/pkix/test/lib/pkixtestutil.h index e579e5bc3c87..40240bed3c8c 100644 --- a/security/pkix/test/lib/pkixtestutil.h +++ b/security/pkix/test/lib/pkixtestutil.h @@ -93,12 +93,8 @@ const ByteString md2WithRSAEncryption(alg_md2WithRSAEncryption, MOZILLA_PKIX_ARRAY_LENGTH(alg_md2WithRSAEncryption)); // e.g. YMDHMS(2016, 12, 31, 1, 23, 45) => 2016-12-31:01:23:45 (GMT) -mozilla::pkix::Time YMDHMS(int16_t year, int16_t month, int16_t day, - int16_t hour, int16_t minutes, int16_t seconds); - -// e.g. YMDHMS(2016, 12, 31, 1, 23, 45) => 2016-12-31:01:23:45 (GMT) -mozilla::pkix::Time YMDHMS(int16_t year, int16_t month, int16_t day, - int16_t hour, int16_t minutes, int16_t seconds); +mozilla::pkix::Time YMDHMS(uint16_t year, uint16_t month, uint16_t day, + uint16_t hour, uint16_t minutes, uint16_t seconds); ByteString TLV(uint8_t tag, const ByteString& value); ByteString Boolean(bool value); @@ -168,7 +164,7 @@ template inline ByteString RFC822Name(const char (&bytes)[L]) { - return RFC822Name(ByteString(reinterpret_cast(bytes), + return RFC822Name(ByteString(reinterpret_cast(&bytes), L - 1)); } @@ -183,7 +179,7 @@ template inline ByteString DNSName(const char (&bytes)[L]) { - return DNSName(ByteString(reinterpret_cast(bytes), + return DNSName(ByteString(reinterpret_cast(&bytes), L - 1)); } diff --git a/security/pkix/warnings.mozbuild b/security/pkix/warnings.mozbuild new file mode 100644 index 000000000000..c8c8d2fb5fc5 --- /dev/null +++ b/security/pkix/warnings.mozbuild @@ -0,0 +1,43 @@ +FAIL_ON_WARNINGS = True + +if CONFIG['CLANG_CXX']: + CXXFLAGS += [ + '-Weverything', + + '-Wno-c++98-compat', + '-Wno-c++98-compat-pedantic', + '-Wno-missing-prototypes', + '-Wno-missing-variable-declarations', + '-Wno-padded', + '-Wno-reserved-id-macro', # XXX: Will be removed in bug 1128413, Part 4. + '-Wno-shadow', # XXX: Clang's rules are too strict for constructors. + '-Wno-undef', # XXX: Will be removed in bug 1128413, Part 4. + '-Wno-weak-vtables', # We rely on the linker to merge the duplicate vtables. + ] +elif CONFIG['_MSC_VER']: + CXXFLAGS += [ + '-sdl', # Enable additional security checks based on Microsoft's SDL. + + '-Wall', + + '-wd4514', # 'function': unreferenced inline function has been removed + '-wd4668', # warning C4668: 'X' is not defined as a preprocessor macro, + # replacing with '0' for '#if/#elif'. + '-wd4710', # 'function': function not inlined + '-wd4711', # function 'function' selected for inline expansion + '-wd4800', # forcing value to bool 'true' or 'false' + '-wd4820', # 'bytes' bytes padding added after construct 'member_name' + + # XXX: We cannot use /Za (Disable Microsoft Extensions) because windows.h + # won't copmile with it. + '-Zc:forScope', # Standard C++ rules for variable scope in for loops. + '-Zc:inline', # Standard C++ rules requiring definition inline functions. + '-Zc:rvalueCast', # Standard C++ rules for result of cast being an rvalue. + '-Zc:strictStrings', # Standard C++ rule that string literals are const. + ] +else: + CXXFLAGS += [ + '-Wall', + '-Wextra', + '-pedantic-errors', + ]