Closed Bug 1571710 Opened 4 months ago Closed 2 months ago

Certificate PEM appears in about:certificate instead of download links

Categories

(Firefox :: Security, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 71
Tracking Status
firefox71 --- fixed

People

(Reporter: evilpie, Assigned: danielleleb12)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

The certificate text under Miscellaneous seems to be percent-encoded. We probably need to call unescape?

(In reply to Tom Schuster [:evilpie] from comment #0)

Created attachment 9083331 [details]
Screenshot from 2019-08-06 15-37-27.png

The certificate text under Miscellaneous seems to be percent-encoded. We probably need to call unescape?

Hello Tom, I think it looks like that because we are rendering it in the html, but the intention is not to do so, in the front-end we will receive the element and make it a downloadable item , and I hope doing so will avoid that problem

Oh, right! Do we have a bug for implementing the download links or should we use this one?

Flags: needinfo?(danielleleb12)
Flags: needinfo?(carolina.jimenez.g)

We don't have a bug for that, I think we can use this one!

Flags: needinfo?(carolina.jimenez.g)

Can I work on this?

Flags: needinfo?(jhofmann)

I actually already started working on this, but forgot to assign it to myself - did you already start on it too?

Flags: needinfo?(danielleleb12)

No, I didn't. Go ahead!

Flags: needinfo?(jhofmann)
Assignee: nobody → danielleleb12
Status: NEW → ASSIGNED
Priority: -- → P1

Depends on D41032

Attachment #9083798 - Attachment description: Bug 1571710 - Adds link to download certificate. [WIP - still needs support for downloading chain] r=johannh → Bug 1571710 - Adds link to download certificate. r=johannh
Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/49b0496a73ed
Adds link to download certificate. r=fluent-reviewers,flod

Depends on D41078

Keywords: checkin-needed
Keywords: checkin-needed
Keywords: checkin-needed
Keywords: checkin-needed
Keywords: checkin-needed

Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/36d75347441b
Adds link to download certificate. r=johannh,fluent-reviewers,flod
https://hg.mozilla.org/integration/autoland/rev/83efa3e53a72
Adds test to download link. r=johannh

Keywords: checkin-needed

Backed out 2 changesets (Bug 1571710) for browser-chrome failures at toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=261967536&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=83efa3e53a72a21fe1048f20a3a5054681fc2f69

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=261972139&repo=autoland&lineNumber=23168

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=261967536&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=738ec8b075811c40b352be8295d40e0c49133bb3

[task 2019-08-16T10:21:57.263Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | data-l10n-id must contain the original label - true == true - 
[task 2019-08-16T10:21:57.263Z] 10:21:57     INFO - Buffered messages finished
[task 2019-08-16T10:21:57.263Z] 10:21:57     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | Info must be equal - "" == "-----BEGIN%20CERTIFICATE-----%0D%0AMIIHQjCCBiqgAwIBAgIQCgYwQn9bvO1pVzllk7ZFHzANBgkqhkiG9w0BAQsFADB1%0D%0AMQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3%0D%0Ad3cuZGlnaWNlcnQuY29tMTQwMgYDVQQDEytEaWdpQ2VydCBTSEEyIEV4dGVuZGVk%0D%0AIFZhbGlkYXRpb24gU2VydmVyIENBMB4XDTE4MDUwODAwMDAwMFoXDTIwMDYwMzEy%0D%0AMDAwMFowgccxHTAbBgNVBA8MFFByaXZhdGUgT3JnYW5pemF0aW9uMRMwEQYLKwYB%0D%0ABAGCNzwCAQMTAlVTMRkwFwYLKwYBBAGCNzwCAQITCERlbGF3YXJlMRAwDgYDVQQF%0D%0AEwc1MTU3NTUwMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQG%0D%0AA1UEBxMNU2FuIEZyYW5jaXNjbzEVMBMGA1UEChMMR2l0SHViLCBJbmMuMRMwEQYD%0D%0AVQQDEwpnaXRodWIuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA%0D%0Axjyq8jyXDDrBTyitcnB90865tWBzpHSbindG/XqYQkzFMBlXmqkzC+FdTRBYyneZ%0D%0Aw5Pz+XWQvL+74JW6LsWNc2EF0xCEqLOJuC9zjPAqbr7uroNLghGxYf13YdqbG5oj%0D%0A/4x+ogEG3dF/U5YIwVr658DKyESMV6eoYV9mDVfTuJastkqcwero+5ZAKfYVMLUE%0D%0AsMwFtoTDJFmVf6JlkOWwsxp1WcQ/MRQK1cyqOoUFUgYylgdh3yeCDPeF22Ax8AlQ%0D%0AxbcaI+GwfQL1FB7Jy+h+KjME9lE/UpgV6Qt2R1xNSmvFCBWu+NFX6epwFP/JRbkM%0D%0AfLz0beYFUvmMgLtwVpEPSwIDAQABo4IDeTCCA3UwHwYDVR0jBBgwFoAUPdNQpdag%0D%0Are7zSmAKZdMh1Pj41g8wHQYDVR0OBBYEFMnCU2FmnV+rJfQmzQ84mqhJ6kipMCUG%0D%0AA1UdEQQeMByCCmdpdGh1Yi5jb22CDnd3dy5naXRodWIuY29tMA4GA1UdDwEB/wQE%0D%0AAwIFoDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwdQYDVR0fBG4wbDA0%0D%0AoDKgMIYuaHR0cDovL2NybDMuZGlnaWNlcnQuY29tL3NoYTItZXYtc2VydmVyLWcy%0D%0ALmNybDA0oDKgMIYuaHR0cDovL2NybDQuZGlnaWNlcnQuY29tL3NoYTItZXYtc2Vy%0D%0AdmVyLWcyLmNybDBLBgNVHSAERDBCMDcGCWCGSAGG/WwCATAqMCgGCCsGAQUFBwIB%0D%0AFhxodHRwczovL3d3dy5kaWdpY2VydC5jb20vQ1BTMAcGBWeBDAEBMIGIBggrBgEF%0D%0ABQcBAQR8MHowJAYIKwYBBQUHMAGGGGh0dHA6Ly9vY3NwLmRpZ2ljZXJ0LmNvbTBS%0D%0ABggrBgEFBQcwAoZGaHR0cDovL2NhY2VydHMuZGlnaWNlcnQuY29tL0RpZ2lDZXJ0%0D%0AU0hBMkV4dGVuZGVkVmFsaWRhdGlvblNlcnZlckNBLmNydDAMBgNVHRMBAf8EAjAA%0D%0AMIIBfgYKKwYBBAHWeQIEAgSCAW4EggFqAWgAdgCkuQmQtBhYFIe7E6LMZ3AKPDWY%0D%0ABPkb37jjd80OyA3cEAAAAWNBYm0KAAAEAwBHMEUCIQDRZp38cTWsWH2GdBpe/uPT%0D%0AWnsu/m4BEC2+dIcvSykZYgIgCP5gGv6yzaazxBK2NwGdmmyuEFNSg2pARbMJlUFg%0D%0AU5UAdgBWFAaaL9fC7NP14b1Esj7HRna5vJkRXMDvlJhV1onQ3QAAAWNBYm0tAAAE%0D%0AAwBHMEUCIQCi7omUvYLm0b2LobtEeRAYnlIo7n6JxbYdrtYdmPUWJQIgVgw1AZ51%0D%0AvK9ENinBg22FPxb82TvNDO05T17hxXRC2IYAdgC72d+8H4pxtZOUI5eqkntHOFeV%0D%0ACqtS6BqQlmQ2jh7RhQAAAWNBYm3fAAAEAwBHMEUCIQChzdTKUU2N+XcqcK0OJYrN%0D%0A8EYynloVxho4yPk6Dq3EPgIgdNH5u8rC3UcslQV4B9o0a0w204omDREGKTVuEpxG%0D%0AeOQwDQYJKoZIhvcNAQELBQADggEBAHAPWpanWOW/ip2oJ5grAH8mqQfaunuCVE+v%0D%0Aac+88lkDK/LVdFgl2B6kIHZiYClzKtfczG93hWvKbST4NRNHP9LiaQqdNC17e5vN%0D%0AHnXVUGw+yxyjMLGqkgepOnZ2Rb14kcTOGp4i5AuJuuaMwXmCo7jUwPwfLe1NUlVB%0D%0AKqg6LK0Hcq4K0sZnxE8HFxiZ92WpV2AVWjRMEc/2z2shNoDvxvFUYyY1Oe67xINk%0D%0AmyQKc+ygSBZzyLnXSFVWmHr3u5dcaaQGGAR42v6Ydr4iL38Hd4dOiBma+FXsXBIq%0D%0AWUjbST4VXmdaol7uzFMojA4zkxQDZAvF5XgJlAFadfySna/teik=%0D%0A-----END%20CERTIFICATE-----%0D%0A" - 
[task 2019-08-16T10:21:57.263Z] 10:21:57     INFO - Stack trace:
[task 2019-08-16T10:21:57.263Z] 10:21:57     INFO - resource://testing-common/content-task.js line 62 > eval:null:96
[task 2019-08-16T10:21:57.263Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | The element exists in adjustedCerts - {"sectionTitle":"Fingerprints","sectionItems":[{"label":"SHA-256","info":"31:11:50:0C:4A:66:01:2C:DA:E3:33:EC:3F:CA:1C:9D:DE:45:C9:54:44:0E:7E:E4:13:71:6B:FF:36:63:C0:74"},{"label":"SHA-1","info":"CA:06:F5:6B:25:8B:7A:0D:4F:2B:05:47:09:39:47:86:51:15:19:84"}],"Critical":false} == true - 
[task 2019-08-16T10:21:57.264Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | sectionItems must be the same length - 2 == 2 - 
[task 2019-08-16T10:21:57.264Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | data-l10n-id must contain the original label - true == true - 
[task 2019-08-16T10:21:57.264Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | Info must be equal - "31:11:50:0C:4A:66:01:2C:DA:E3:33:EC:3F:CA:1C:9D:DE:45:C9:54:44:0E:7E:E4:13:71:6B:FF:36:63:C0:74" == "31:11:50:0C:4A:66:01:2C:DA:E3:33:EC:3F:CA:1C:9D:DE:45:C9:54:44:0E:7E:E4:13:71:6B:FF:36:63:C0:74" - 
[task 2019-08-16T10:21:57.264Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | data-l10n-id must contain the original label - true == true - 
[task 2019-08-16T10:21:57.264Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | Info must be equal - "CA:06:F5:6B:25:8B:7A:0D:4F:2B:05:47:09:39:47:86:51:15:19:84" == "CA:06:F5:6B:25:8B:7A:0D:4F:2B:05:47:09:39:47:86:51:15:19:84" - 
[task 2019-08-16T10:21:57.264Z] 10:21:57     INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | The element exists in adjustedCerts - {"sectionTitle":"Basic Constraints","sectionItems":[{"label":"Certificate Authority"}],"Critical":true} == true - 
Flags: needinfo?(danielleleb12)
Summary: Certificate text seems to be percent encoded → Certificate PEM appears in about:certificate instead of download links
Duplicate of this bug: 1576982

I've attached an image of what things are supposed to be like. Not 100% certain how this got broken in the move from the WebExtension, but it looks like there is a patch to fix it.

Hey Danielle, is there anything blocking landing this? You resolved the backout failures, right?

Currently adding the small test case you mentioned in the patch - I'll update it once I'm done. I no longer have access to the try server though.

Flags: needinfo?(danielleleb12)

(In reply to danielleleb12 from comment #15)

Currently adding the small test case you mentioned in the patch - I'll update it once I'm done. I no longer have access to the try server though.

Awesome, thanks. You can probably just file another bug for access, I'm happy to vouch again if necessary.

Duplicate of this bug: 1580219

Apologies for the slow progress on this - there was a hurricane here this weekend and internet was out until last night.

This has been rebased and updated (along with https://bugzilla.mozilla.org/show_bug.cgi?id=1573143), so should hopefully be almost ready to land, I'm just waiting on Phabricator's source code analysis (I pushed the rebased commits last night, and it shows the source code analysis has now been running since then, not sure why it's taking so long..). This bug's patches should land first (I've adjusted the patch's parent revisions in Phabricator to reflect that).

Awesome, thanks, and no worries at all. Let me know if you need me to do anything.

Blocks: 1575331
Blocks: 1580455
No longer blocks: 1575331

Hey Danielle, do you think you'll be able to drive this home anytime soon? It would be great to get this landed. If not I would be happy to take over and do the rebase and landing etc., I don't think there's a lot remaining to be done here.

Thanks! :)

Flags: needinfo?(danielleleb12)
No longer blocks: 1580455

Hey Johann, not sure what happened with this - I tried rebasing it then was waiting for the 'source code analysis' to clear, but it never did (it's been showing as in progress since my last comment here). I'll try rebasing it again now - but if you could land it, that would be great. It should be done anyway! (Apologies, it's been harder than I expected to keep up with this following the internship and adjusting to the new job).

Flags: needinfo?(danielleleb12)

No worries at all, that's very understandable. I'll give it a shot!

Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ac5745f0242
Adds link to download certificate. r=fluent-reviewers,flod,nhnt11
https://hg.mozilla.org/integration/autoland/rev/1681a2659835
Adds test to download link. r=johannh
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
You need to log in before you can comment on or make changes to this bug.