bug 1261936 - stop using the subject common name in certificate verification error messages r=Cykesiopka

MozReview-Commit-ID: G08cV5GmNDh

--HG--
extra : rebase_source : c79b34d893e7acddc8ee02a6c354dcaa1de07d61
This commit is contained in:
David Keeler 2016-04-04 16:25:24 -07:00
parent a30d4e5147
commit 7dd242bb39
4 changed files with 80 additions and 109 deletions

View File

@ -285,7 +285,6 @@ certErrorMismatch=The certificate is not valid for the name %S.
certErrorMismatchSingle2=The certificate is only valid for <a id="cert_domain_link" title="%1$S">%1$S</a>
certErrorMismatchSinglePlain=The certificate is only valid for %S
certErrorMismatchMultiple=The certificate is only valid for the following names:
certErrorMismatchNoNames=The certificate is not valid for any server names.
# LOCALIZATION NOTE (certErrorExpiredNow): Do not translate %1$S (date+time of expired certificate) or %2$S (current date+time)
certErrorExpiredNow=The certificate expired on %1$S. The current time is %2$S.

View File

@ -618,41 +618,33 @@ AppendErrorTextUntrusted(PRErrorCode errTrust,
}
}
// returns TRUE if SAN was used to produce names
// return FALSE if nothing was produced
// names => a single name or a list of names
// multipleNames => whether multiple names were delivered
static bool
GetSubjectAltNames(CERTCertificate *nssCert,
nsINSSComponent *component,
nsString &allNames,
uint32_t &nameCount)
// Returns the number of dNSName or iPAddress entries encountered in the
// subject alternative name extension of the certificate.
// Returns zero if the extension is not present, could not be decoded, or if it
// does not contain any dNSName or iPAddress entries.
static uint32_t
GetSubjectAltNames(CERTCertificate* nssCert, nsString& allNames)
{
allNames.Truncate();
nameCount = 0;
SECItem altNameExtension = {siBuffer, nullptr, 0 };
CERTGeneralName *sanNameList = nullptr;
ScopedAutoSECItem altNameExtension;
SECStatus rv = CERT_FindCertExtension(nssCert, SEC_OID_X509_SUBJECT_ALT_NAME,
&altNameExtension);
if (rv != SECSuccess) {
return false;
return 0;
}
UniquePLArenaPool arena(PORT_NewArena(DER_DEFAULT_CHUNKSIZE));
if (!arena) {
return false;
return 0;
}
sanNameList = CERT_DecodeAltNameExtension(arena.get(), &altNameExtension);
CERTGeneralName* sanNameList(CERT_DecodeAltNameExtension(arena.get(),
&altNameExtension));
if (!sanNameList) {
return false;
return 0;
}
SECITEM_FreeItem(&altNameExtension, false);
CERTGeneralName *current = sanNameList;
uint32_t nameCount = 0;
CERTGeneralName* current = sanNameList;
do {
nsAutoString name;
switch (current->type) {
@ -707,97 +699,62 @@ GetSubjectAltNames(CERTCertificate *nssCert,
current = CERT_GetNextGeneralName(current);
} while (current != sanNameList); // double linked
return true;
return nameCount;
}
static void
AppendErrorTextMismatch(const nsString &host,
nsIX509Cert* ix509,
nsINSSComponent *component,
bool wantsHtml,
nsString &returnedMessage)
static nsresult
AppendErrorTextMismatch(const nsString& host, nsIX509Cert* ix509,
nsINSSComponent* component, bool wantsHtml,
nsString& returnedMessage)
{
const char16_t *params[1];
nsresult rv;
// Prepare a default "not valid for <hostname>" string in case anything
// goes wrong (or in case the certificate is not valid for any hostnames).
nsAutoString notValidForHostnameString;
const char16_t* params[1];
params[0] = host.get();
nsresult rv = component->PIPBundleFormatStringFromName(
"certErrorMismatch", params, 1, notValidForHostnameString);
if (NS_FAILED(rv)) {
return rv;
}
notValidForHostnameString.Append('\n');
ScopedCERTCertificate nssCert(ix509->GetCert());
if (!nssCert) {
// We are unable to extract the valid names, say "not valid for name".
params[0] = host.get();
nsString formattedString;
rv = component->PIPBundleFormatStringFromName("certErrorMismatch",
params, 1,
formattedString);
if (NS_SUCCEEDED(rv)) {
returnedMessage.Append(formattedString);
returnedMessage.Append('\n');
}
return;
returnedMessage.Append(notValidForHostnameString);
return NS_OK;
}
nsString allNames;
uint32_t nameCount = 0;
bool useSAN = false;
if (nssCert)
useSAN = GetSubjectAltNames(nssCert.get(), component, allNames, nameCount);
if (!useSAN) {
char *certName = CERT_GetCommonName(&nssCert->subject);
if (certName) {
nsDependentCSubstring commonName(certName, strlen(certName));
if (IsUTF8(commonName)) {
// Bug 1024781
// We should actually check that the common name is a valid dns name or
// ip address and not any string value before adding it to the display
// list.
++nameCount;
allNames.Assign(NS_ConvertUTF8toUTF16(commonName));
}
PORT_Free(certName);
}
}
if (nameCount > 1) {
nsAutoString allNames;
uint32_t nameCount = GetSubjectAltNames(nssCert.get(), allNames);
if (nameCount == 0) {
returnedMessage.Append(notValidForHostnameString);
} else if (nameCount > 1) {
nsString message;
rv = component->GetPIPNSSBundleString("certErrorMismatchMultiple",
message);
if (NS_SUCCEEDED(rv)) {
returnedMessage.Append(message);
returnedMessage.AppendLiteral("\n ");
returnedMessage.Append(allNames);
returnedMessage.AppendLiteral(" \n");
rv = component->GetPIPNSSBundleString("certErrorMismatchMultiple", message);
if (NS_FAILED(rv)) {
return rv;
}
}
else if (nameCount == 1) {
const char16_t *params[1];
returnedMessage.Append(message);
returnedMessage.AppendLiteral("\n ");
returnedMessage.Append(allNames);
returnedMessage.AppendLiteral(" \n");
} else if (nameCount == 1) {
params[0] = allNames.get();
const char *stringID;
if (wantsHtml)
stringID = "certErrorMismatchSingle2";
else
stringID = "certErrorMismatchSinglePlain";
nsString formattedString;
rv = component->PIPBundleFormatStringFromName(stringID,
params, 1,
const char* stringID = wantsHtml ? "certErrorMismatchSingle2"
: "certErrorMismatchSinglePlain";
nsAutoString formattedString;
rv = component->PIPBundleFormatStringFromName(stringID, params, 1,
formattedString);
if (NS_SUCCEEDED(rv)) {
returnedMessage.Append(formattedString);
returnedMessage.Append('\n');
}
}
else { // nameCount == 0
nsString message;
nsresult rv = component->GetPIPNSSBundleString("certErrorMismatchNoNames",
message);
if (NS_SUCCEEDED(rv)) {
returnedMessage.Append(message);
returnedMessage.Append('\n');
if (NS_FAILED(rv)) {
return rv;
}
returnedMessage.Append(formattedString);
returnedMessage.Append('\n');
}
return NS_OK;
}
static void
@ -967,7 +924,9 @@ formatOverridableCertErrorMessage(nsISSLStatus & sslStatus,
rv = sslStatus.GetIsDomainMismatch(&isDomainMismatch);
NS_ENSURE_SUCCESS(rv, rv);
if (isDomainMismatch) {
AppendErrorTextMismatch(hostWithoutPort, ix509, component, wantsHtml, returnedMessage);
rv = AppendErrorTextMismatch(hostWithoutPort, ix509, component, wantsHtml,
returnedMessage);
NS_ENSURE_SUCCESS(rv, rv);
}
bool isNotValidAtThisTime;

View File

@ -642,7 +642,13 @@ FakeSSLStatus.prototype = {
// Helper function for add_cert_override_test. Probably doesn't need to be
// called directly.
function add_cert_override(aHost, aExpectedBits, aSecurityInfo) {
function add_cert_override(aHost, aExpectedBits, aExpectedErrorRegexp,
aSecurityInfo) {
if (aExpectedErrorRegexp) {
do_print(aSecurityInfo.errorMessage);
Assert.ok(aExpectedErrorRegexp.test(aSecurityInfo.errorMessage),
"Actual error message should match expected error regexp");
}
let sslstatus = aSecurityInfo.QueryInterface(Ci.nsISSLStatusProvider)
.SSLStatus;
let bits =
@ -658,13 +664,16 @@ function add_cert_override(aHost, aExpectedBits, aSecurityInfo) {
true);
}
// Given a host, expected error bits (see nsICertOverrideService.idl), and
// an expected error code, tests that an initial connection to the host fails
// Given a host, expected error bits (see nsICertOverrideService.idl), an
// expected error code, and optionally a regular expression that the resulting
// error message must match, tests that an initial connection to the host fails
// with the expected errors and that adding an override results in a subsequent
// connection succeeding.
function add_cert_override_test(aHost, aExpectedBits, aExpectedError) {
function add_cert_override_test(aHost, aExpectedBits, aExpectedError,
aExpectedErrorRegexp = undefined) {
add_connection_test(aHost, aExpectedError, null,
add_cert_override.bind(this, aHost, aExpectedBits));
add_cert_override.bind(this, aHost, aExpectedBits,
aExpectedErrorRegexp));
add_connection_test(aHost, PRErrorCodeSuccess, null, aSecurityInfo => {
Assert.ok(aSecurityInfo.securityState &
Ci.nsIWebProgressListener.STATE_CERT_USER_OVERRIDDEN,

View File

@ -153,12 +153,14 @@ function add_simple_tests() {
// but not the subject common name.
add_cert_override_test("mismatch.example.com",
Ci.nsICertOverrideService.ERROR_MISMATCH,
SSL_ERROR_BAD_CERT_DOMAIN);
SSL_ERROR_BAD_CERT_DOMAIN,
/The certificate is only valid for the following names:\s*doesntmatch\.example\.com, \*\.alsodoesntmatch\.example\.com/);
// This has name information in the subject common name but not the subject
// alternative names extension.
add_cert_override_test("mismatch-CN.example.com",
Ci.nsICertOverrideService.ERROR_MISMATCH,
SSL_ERROR_BAD_CERT_DOMAIN);
SSL_ERROR_BAD_CERT_DOMAIN,
/The certificate is not valid for the name mismatch-CN\.example\.com/);
// A Microsoft IIS utility generates self-signed certificates with
// properties similar to the one this "host" will present.
@ -241,7 +243,8 @@ function add_simple_tests() {
SSL_ERROR_BAD_CERT_DOMAIN);
add_cert_override_test("noValidNames.example.com",
Ci.nsICertOverrideService.ERROR_MISMATCH,
SSL_ERROR_BAD_CERT_DOMAIN);
SSL_ERROR_BAD_CERT_DOMAIN,
/The certificate is not valid for the name noValidNames\.example\.com/);
add_cert_override_test("badSubjectAltNames.example.com",
Ci.nsICertOverrideService.ERROR_MISMATCH,
SSL_ERROR_BAD_CERT_DOMAIN);
@ -268,7 +271,8 @@ function add_combo_tests() {
add_cert_override_test("mismatch-expired.example.com",
Ci.nsICertOverrideService.ERROR_MISMATCH |
Ci.nsICertOverrideService.ERROR_TIME,
SSL_ERROR_BAD_CERT_DOMAIN);
SSL_ERROR_BAD_CERT_DOMAIN,
/The certificate is only valid for <a id="cert_domain_link" title="doesntmatch\.example\.com">doesntmatch\.example\.com<\/a>/);
add_cert_override_test("mismatch-notYetValid.example.com",
Ci.nsICertOverrideService.ERROR_MISMATCH |
Ci.nsICertOverrideService.ERROR_TIME,