1592877 - "cert-error-domain-mismatch-multiple" sometimes broken

Status: NEW

(Firefox :: Security, defect, P5)

Description

[YF (Yang)]

Steps to reproduce:

1. Open https://ww1.rs.fanjian.net/
2. Click the Advanced button on security warning screen.

Actual results:

{???}
 
错误代码：SSL_ERROR_BAD_CERT_DOMAIN

Expected results:
Same as earlier version.

Extra information:
https://wrong.host.badssl.com/ work fine, displayed:

各个网站通过证书证明自己的身份。Firefox 不能信任此网站，它使用的证书对 wrong.host.badssl.com 无效。该证书只适用于下列名称： *.badssl.com, badssl.com
 
错误代码：SSL_ERROR_BAD_CERT_DOMAIN

So it might be a defect occurs when the domain list is long?
I see the subjectAltNames's length are 142 via Browser content toolbox and then breakpoint on setL10NLabel("cert-error-domain-mismatch-multiple", args);.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=09785dc4c5aa58ddb33531831f60a77371381b1e&tochange=1b513d98a80eb58847f20d4ac698ccb012ab108d

Comment 1

[Johann Hofmann [:johannh]]

Zibi, given that Stas is out, can you take a quick look at this?

Thank you!

Comment 2

[Axel Hecht [:Pike]]

Confirmed, the trigger is the alternate domain names for the cert:

"*.oss.aliyuncs.com, aliyuncs.com, *.img-cn-shenzhen-internal.aliyuncs.com, *.oss-cn-hangzhou-internal.aliyuncs.com, *.vpc100-ap-southeast-1.aliyuncs.com, *.vpc100.oss-cn-shenzhen.aliyuncs.com, *.oss-cn-henan-a.aliyuncs.com, *.oss-cn-shenzhen-diku-internal.aliyuncs.com, *.oss-cn-shanghai-finance-1-pub.aliyuncs.com, *.oss-ap-southeast-3.aliyuncs.com, *.oss-us-west-1-internal.aliyuncs.com, *.oss-cn-north-2-gov-1-internal.aliyuncs.com, *.img-cn-shanghai.aliyuncs.com, *.oss-cn-suzhou-gov.aliyuncs.com, *.vpc100.oss-cn-hangzhou.aliyuncs.com, *.oss-cn-huhehaote.aliyuncs.com, *.oss-cn-gansu-a.aliyuncs.com, *.oss-ap-south-1-cross.aliyuncs.com, *.oss-cn-hangzhou-cross.aliyuncs.com, *.oss-cn-huhehaote-internal.aliyuncs.com, *.vpc100.oss-cn-qingdao.aliyuncs.com, *.vpc100.oss-cn-shanghai.aliyuncs.com, *.img-cn-shanghai-internal.aliyuncs.com, *.oss-cn-ningxiazhongwei.aliyuncs.com, *.oss-cn-shanghai-finance-1-internal.aliyuncs.com, *.oss-cn-guizhou-a.aliyuncs.com, *.img-cn-qingdao.aliyuncs.com, *.oss-cn-north-2-gov-1.aliyuncs.com, *.oss-me-east-1.aliyuncs.com, *.oss-ap-northeast-1.aliyuncs.com, *.oss-us-east-1-cross.aliyuncs.com, *.oss-cn-hongkong.aliyuncs.com, *.oss-cn-qingdao-internal.aliyuncs.com, *.vpc100-oss-cn-shenzhen.aliyuncs.com, *.oss-cn-qingdao-diku-internal.aliyuncs.com, *.oss-cn-qingdao-internal-cross.aliyuncs.com, *.img-cn-hangzhou-internal.aliyuncs.com, *.oss-cn-hzjbp-a-internal.aliyuncs.com, *.oss-cn-shenzhen-internal-cross.aliyuncs.com, *.oss-cn-szfinance.aliyuncs.com, *.cn-huhehaote.oss.aliyuncs.com, *.img-ap-southeast-2-internal.aliyuncs.com, *.cn-hangzhou.oss.aliyuncs.com, *.oss-eu-west-1-cross.aliyuncs.com, *.vpc100-oss-cn-huhehaote.aliyuncs.com, *.oss-cn-shenzhen-finance-1-internal.aliyuncs.com, *.oss-cn-shenzhen-cross.aliyuncs.com, *.oss-ap-southeast-2.aliyuncs.com, *.oss-accelerate.aliyuncs.com, *.oss-cn-beijing-internal-cross.aliyuncs.com, *.oss-cn-qingdao.aliyuncs.com, *.oss-cn-fujian-a.aliyuncs.com, *.img-cn-shenzhen.aliyuncs.com, *.oss-ap-south-1.aliyuncs.com, *.vpc100-oss-ap-southeast-2.aliyuncs.com, *.vpc100-us-west-1.aliyuncs.com, *.oss-cn-shanghai.aliyuncs.com, *.vpc100-oss-cn-qingdao.aliyuncs.com, *.oss-cn-shanghai-finance-1.aliyuncs.com, *.oss-eu-central-1-internal.aliyuncs.com, *.oss-us-east-1-internal.aliyuncs.com, *.oss-cn-hangzhou-zmf-internal.aliyuncs.com, *.vpc100-oss-cn-shanghai.aliyuncs.com, *.img-cn-hongkong-internal.aliyuncs.com, *.oss-me-east-1-cross.aliyuncs.com, *.vpc100.oss-cn-hongkong.aliyuncs.com, *.oss-cn-hangzhou-diku-internal.aliyuncs.com, *.oss-cn-hongkong-cross.aliyuncs.com, *.oss-cn-haidian-a.aliyuncs.com, *.oss-ap-northeast-1-internal.aliyuncs.com, *.img-ap-southeast-1.aliyuncs.com, *.oss-us-east-1.aliyuncs.com, *.img-cn-beijing-internal.aliyuncs.com, *.oss-ap-southeast-5-internal.aliyuncs.com, *.oss-cn-zhangjiakou-internal.aliyuncs.com, *.oss-us-siliconvalley-internal.aliyuncs.com, *.oss-ap-southeast-3-internal.aliyuncs.com, *.oss-cn-shanghai-finance-1-pub-internal.aliyuncs.com, *.oss-tw-gaoxiong.aliyuncs.com, *.img-cn-hongkong.aliyuncs.com, *.oss-eu-west-1.aliyuncs.com, *.oss-cn-hangzhou-zmf.aliyuncs.com, *.oss-cn-haidian-a-internal.aliyuncs.com, *.oss-cn-beijing-internal.aliyuncs.com, *.oss-cn-beijing-cross.aliyuncs.com, *.oss-eu-west-1-internal.aliyuncs.com, *.oss-cn-hongkong-internal.aliyuncs.com, *.vpc100-oss-us-west-1.aliyuncs.com, *.oss-ap-southeast-2-internal.aliyuncs.com, *.img-cn-beijing.aliyuncs.com, *.oss-cn-shenzhen.aliyuncs.com, *.oss-cn-hzfinance.aliyuncs.com, *.oss-cn-chengdu-internal.aliyuncs.com, *.oss-eu-central-1.aliyuncs.com, *.oss-cn-zhangjiakou-cross.aliyuncs.com, *.oss-us-west-1.aliyuncs.com, *.vpc100-oss-us-east-1.aliyuncs.com, *.img-cn-shanghai-cross.aliyuncs.com, *.oss-cn-shanghai-internal.aliyuncs.com, *.oss-ap-southeast-1-internal.aliyuncs.com, *.vpc100.oss-cn-beijing.aliyuncs.com, *.oss-cn-hzfinance-internal.aliyuncs.com, *.oss-ap-south-1-internal.aliyuncs.com, *.vpc100-oss-cn-hongkong.aliyuncs.com, *.oss-cn-qingdao-cross.aliyuncs.com, *.img-ap-southeast-2.aliyuncs.com, *.oss-cn-shenzhen-finance-1.aliyuncs.com, *.oss-cn-suzhou-gov-internal.aliyuncs.com, *.oss-cn-shenzhen-internal.aliyuncs.com, *.vpc100-oss-ap-southeast-1-a.aliyuncs.com, *.cn-hangzhou.oss-internal-cross.aliyun-inc.com, *.oss-cn-hangzhou-am101.aliyuncs.com, *.oss-us-west-1-cross.aliyuncs.com, *.vpc100-ap-southeast-2.aliyuncs.com, *.oss-internal.aliyuncs.com, *.img-cn-hangzhou.aliyuncs.com, *.oss-cn-chengdu.aliyuncs.com, *.vpc100-oss-ap-southeast-1.aliyuncs.com, *.aliyuncs.com, *.oss-cn-hangzhou-hsa.aliyuncs.com, *.oss-cn-zhangjiakou.aliyuncs.com, *.vpc100-oss-cn-hangzhou.aliyuncs.com, *.oss-ap-northeast-1-cross.aliyuncs.com, *.oss-cn-beijing.aliyuncs.com, *.oss-cn-hangzhou.aliyuncs.com, *.vpc100-oss-ap-southeast-3.aliyuncs.com, *.vpc100-oss-ap-southeast-5.aliyuncs.com, *.vpc100-oss-cn-zhangjiakou.aliyuncs.com, *.cn-hangzhou.oss-internal.aliyuncs.com, *.oss-eu-central-1-cross.aliyuncs.com, *.oss-ap-southeast-5.aliyuncs.com, *.oss-me-east-1-internal.aliyuncs.com, *.vpc100-oss-cn-beijing.aliyuncs.com, *.oss-cn-szfinance-internal.aliyuncs.com, *.oss-cn-shanghai-cross.aliyuncs.com, *.oss-us-siliconvalley.aliyuncs.com, *.oss-ap-southeast-5-cross.aliyuncs.com, *.cn-beijing.oss.aliyuncs.com, *.img-ap-southeast-1-internal.aliyuncs.com, *.oss-ap-southeast-1-cross.aliyuncs.com, *.oss-ap-southeast-2-cross.aliyuncs.com, oss.aliyuncs.com"

Fluent thinks that's an DOS attack on it, and bails out, in https://searchfox.org/mozilla-central/rev/1fe0cf575841dbf3b7e159e88ba03260cd1354c0/intl/l10n/Fluent.jsm#213-214.

Arguably, Fluent is right in this particular case, I wonder if there's a better UX for this case. I just tried by setting just the text content to this list, and it's not helpful.

Johann?

PS: Looking at the blame of MAX_PLACEABLE_LENGTH, I'd think this behavior goes back to at least October 2018, but maybe it was broken at one point.

Comment 3

[Johann Hofmann [:johannh]]

Yeah, that's interesting, I agree that fluent needs to protect against this, at the same time there might be a better way to fail than {???}. And it would be nice to get a console error. Those are separate issues though, this definitely needs a solution on certerror side.

The first thing that comes to mind would be limiting the maximum amount of SAN that can be displayed, maybe to 50? It's definitely better than the current situation.

Not sure why you moved the component.

Comment 4

[Johann Hofmann [:johannh]]

To restate the problem, our cert error pages show a weird message when a website has too many SubjectAltNames in its certificate.

Solving this issue should be very simple. We can simply reduce the list to a sane number of maximum items (maybe 50? but it would be nice to see how cluttered that looks) here: https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/browser/base/content/aboutNetError.js#1003-1005

But it would be great to also have a test for this, which will be slightly more challenging, because it involves adding a new certificate to our tests (one with a lot of SANs). Luckily adding a new cert is well documented, here: https://firefox-source-docs.mozilla.org/build/buildsystem/test_certificates.html

You can specify SANs like this: https://searchfox.org/mozilla-central/rev/557a0e222dd104c5d805ba344c45d6abc27d3db0/build/pgo/certs/expired.certspec#3

For writing the actual test, you can take a look at this one: https://searchfox.org/mozilla-central/source/browser/base/content/test/about/browser_aboutCertError_noSubjectAltName.js which does something very similar (check that things still work fine if there are no SANs).

Lupita, would you like to give this a try?

Comment 5

[Lupita Arroyo]

Definitely! I'll be working on it.

Comment 6

[Lupita Arroyo]

Johann, can I be assigned to this bug please?

Comment 7

[Johann Hofmann [:johannh]]

Absolutely, sorry for that, forgot to assign you :)

Comment 8

[Lupita Arroyo]

Attachment: Bug 1592877 - Test for fix when a certificate is not valid and has more than 50plus SANs. r=prathiksha

Comment 9

[Lupita Arroyo]

I have submitted a patch for review. However, when I looked over the SubjectAltNames, it was already truncated to 50 SANs even if I had 100 of them in the cert. Just making sure this is all right.

Comment 10

[Lupita Arroyo]

I get fails:

FAIL Uncaught exception - correct error code has been set inside the advanced button panel - threw exception: TypeError: can't access property "textContent", errorCode is null
FAIL Found an unexpected tab at the end of test run: https://many-sans.example.com/ -

when I try to change the server-location.txt name to "many-subject-alt-names" or "many-sans" as suggested. Any suggestions as to why?

Comment 11

[Lupita Arroyo]

Disregard that question. I fixed it!

Comment 12

[Lupita Arroyo]

Attachment: Bug 1592877 - Truncate SANs to 50 in aboutNetError.js and add a test for it. r=johannh

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5616dd0ad683
Truncate SANs to 50 in aboutNetError.js and add a test for it. r=johannh

Comment 14

[Arthur Iakab [arthur_iakab]]

Backed out changeset 5616dd0ad683 (bug 1592877) for causing browser-chrome failures on browser_aboutCertError_manySANsError.js

Backout revision https://hg.mozilla.org/integration/autoland/rev/acc1632e35c7afc826d16bea8e1dd812b4f5117c

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5616dd0ad683f36496e9ae88f025bb271352dacf

Failure logs https://treeherder.mozilla.org/logviewer.html#?job_id=296787307&repo=autoland

Lupita Arroyo can you please take a look?

Comment 15

[:prathiksha]

Lupita, are you still looking into this?

Comment 16

[disha]

Attachment: Bug 1592877 - Truncated SANs to 50 and added test for it in browser_aboutCertError_manySANsError.js. r=prathiksha

Comment 17

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:sgalich, could you have a look please?
For more information, please visit auto_nag documentation.