Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-02-25 20:01:50 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

bug 1497258 - remove unsound OCSP assertion from NSSCertDBTrustDomain::CheckRevocation r=mayhemer

Browse Source 
In reimplementing the OCSP fetching code in bug 1456489, we improperly
translated an assertion that relied on the nullness of a pointer to rely on the
length of a data structure that was populated by reference. It turns out that
this made the assertion invalid because we could return a successful result and
have filled the data structure with zero-length data and it still would be valid
to operate on (the decoding code returns a malformed input result in this case).
To fix this, we can simply remove the assertion. This patch also adds a test to
exercise this case.

Differential Revision: https://phabricator.services.mozilla.com/D8883

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Dana Keeler 2018-10-17 19:35:28 +00:00
parent e1b4840fb5
commit c4dab46f9c
2 changed files with 15 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
security/certverifier/NSSCertDBTrustDomain.cpp
View File

@ -582,7 +582,6 @@ NSSCertDBTrustDomain::CheckRevocation(EndEntityOrCA endEntityOrCA,
Result tempRV = DoOCSPRequest(aiaLocation, mOriginAttributes,
std::move(ocspRequest), GetOCSPTimeout(),
ocspResponse);
MOZ_ASSERT((tempRV != Success) || ocspResponse.length() > 0);
if (tempRV != Success) {
rv = tempRV;
} else if (response.Init(ocspResponse.begin(), ocspResponse.length())

21
security/manager/ssl/tests/unit/test_ocsp_required.js
View File

@ -4,12 +4,14 @@
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
"use strict";
// In which we connect to a domain (as faked by a server running locally)
// and start up an OCSP responder (also basically faked) that gives a
// response with a bad signature. With security.OCSP.require set to true,
// this should fail (but it also shouldn't cause assertion failures).
// In which we connect to a domain (as faked by a server running locally) and
// start up an OCSP responder (also basically faked) that gives a response with
// a bad signature (and later, an empty response). With security.OCSP.require
// set to true, these connections should fail (but they also shouldn't cause
// assertion failures).
var gOCSPRequestCount = 0;
var gOCSPResponse;
function run_test() {
do_get_profile();
@ -22,13 +24,14 @@ function run_test() {
let args = [["bad-signature", "default-ee", "unused", 0]];
let ocspResponses = generateOCSPResponses(args, "ocsp_certs");
let ocspResponseBadSignature = ocspResponses[0];
// Start by replying with a response with a bad signature.
gOCSPResponse = ocspResponses[0];
let ocspResponder = new HttpServer();
ocspResponder.registerPrefixHandler("/", function (request, response) {
response.setStatusLine(request.httpVersion, 200, "OK");
response.setHeader("Content-Type", "application/ocsp-response");
response.write(ocspResponseBadSignature);
response.write(gOCSPResponse);
gOCSPRequestCount++;
});
ocspResponder.start(8888);
@ -49,6 +52,12 @@ function add_tests() {
equal(gOCSPRequestCount, 1,
"OCSP request count should be 1 due to OCSP response caching");
gOCSPRequestCount = 0;
// Now set the OCSP responder to reply with 200 OK but empty content.
gOCSPResponse = "";
clearOCSPCache();
run_next_test();
});
add_connection_test("ocsp-stapling-none.example.com",
SEC_ERROR_OCSP_MALFORMED_RESPONSE);
}