Tim Nguyen :ntim Reporter
	Description • 8 years ago

Keygen support should be removed for the following reasons:
- Firefox never got proper support for HTML keygen (see bug 101019).

- Using a keygen element in Chrome gives you this warning in the console:
The <keygen> element is deprecated and will be removed in M56, around January 2017. See https://www.chromestatus.com/features/5716060992962560 for more details.

- Most importantly, it's been removed from the HTML spec: https://github.com/w3c/html/issues/43

	:Cykesiopka
	Comment 1 • 8 years ago

(In reply to Tim Nguyen :ntim (use needinfo?) from comment #0)
> Keygen support should be removed for the following reasons:
> [...]
-  The implementation in PSM to do the actual key generation etc is a horrid mess.

Also wanted to leave an FYI that Bug 1284945 extended telemetry for keygen usage till Firefox 60.
Maybe we can remove earlier than that point, maybe not.

	Domenic Denicola
	Comment 6 • 7 years ago

We've removed keygen from the specification that Firefox follows (which is not the specification quoted in the OP): https://github.com/whatwg/html/pull/2221 . Note that the parser behavior is retained, but it becomes a HTMLUnknownElement.

Tests for the removal are at https://github.com/w3c/web-platform-tests/pull/4402.

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 7 • 6 years ago

Once this bug closes it should also close: https://bugzilla.mozilla.org/show_bug.cgi?id=101019 We have code comments pointing to this bug that should be removed.

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 8 • 6 years ago

Removal from the html parser.

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 9 • 6 years ago

Getting errors in: accessible/tests/mochitest/elm/test_HTMLSpec.html after fixing.

Also getting errors when building without: nsKeygenHandler.cpp and nsKeygenHandlerContent.cpp

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 11 • 6 years ago

Moving Bug 1088063 to See also as it's not a blocker here. The removal has been standardised and a new API to add via JS hasn't been worked on. We should consider elevating the priority of that work perhaps though.

:keeler are you the right person to review the nss component of this work?

I'm going to make sure tests pass and enable the web platform ones too next.

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 12 • 6 years ago

I may as well take this also, however if I am slow in completing please speak to me.

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 13 • 6 years ago

This looks like it will break the Cert-O-Matic page within NSS, is this going to be a big issue? We won't have any workable replacement without the Bug 1088063.

As this hooks into NSS and is a pretty large patch to remove I would like to expedite this pretty quick.

	Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)
	Comment 14 • 6 years ago

Code in security/manager/ is part of PSM - I can review that or find a suitable reviewer. I don't think you'll need to change anything in NSS itself for this bug.

	Franziskus Kiefer [:franziskus]
	Comment 16 • 6 years ago

As David said, no changes to NSS needed. The tool in question (which is actually a test) isn't built as part of Firefox and I don't think anyone is using it. I might remove it from NSS but that's independent of this bug.

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 20 • 6 years ago

There is also a comment in security/manager/ssl/tests/unit/test_sdr_preexisting.js that uses code from KeyGen however I'm not sure if this is worth keeping?

	Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)
	Comment 21 • 6 years ago

That file (even the code-in-comments) should be orthogonal to keygen, unless I'm missing something.

	Jonathan Kingston [:jkt] he/him Assignee
	Comment 22 • 6 years ago

Ah yeah I wrote that comment at our work week without checking it, I needed to switch profiles and didn't want to lose what I had to check :P. I have a few tests to check before asking for a review.

	Markus Schraeder
	Comment 26 • 6 years ago

As discussed in #webextensions on irc.mozilla.org, jtk wants to talk to keeler about this issue.

15:40:31   twisniewski | are you deploying firefox in an enterprise setting?
15:40:45         pRiVi | twisniewski: What does that mean?
15:40:55         pRiVi | We are securing websites by client certificates
15:41:09   twisniewski | who are your users? do you work for a business, and are managing a set of computers for them with firefox?
15:41:18         pRiVi | this is what we get into trouble by firefox - just firefox - by removing the keygen
15:41:26         pRiVi | twisniewski: Home users
15:41:30         pRiVi | users of our customers
15:41:38         pRiVi | endusers
15:41:42         pRiVi | not able to do about:config
15:41:53         pRiVi | no domain, no domain controller, nothing
15:42:12         pRiVi | Chrome, IE, edge, .. works, on windows and on macos
15:42:25         pRiVi | but if you remove keygen, firefox will stop working for us
15:43:00          ntim | pRiVi: how does the extension fit into the story ?
15:43:05   twisniewski | ok, i understand. it sounds like this use-case has not been explored yet. but i don't believe we are planning to remove keygen before
                       | we explore it.
15:43:24   twisniewski | (even if there is a bug on file with a patch)
15:43:30         pRiVi | ntim: If I give the user a extension url, he can add this and then maybe make security.enterprise_roots.enabled
15:44:47           jkt | pRiVi: I don't really see why you need to give your users certs anyway
15:45:05         pRiVi | jkt: We secure webservers. https://www.cryptomagic.eu/cryptoweb/
15:45:10      freonbot | [ CryptoWeb ] - www.cryptomagic.eu
15:45:15         pRiVi | we have about 600 comapnies and about 12.000 users
15:45:30         pRiVi | the webservers are only available to users having a certificate
15:45:53         pRiVi | we must move ALL away from firefox to other browsers if keygen will go away
15:47:09       @zombie | ntim: run the test without your implementation, it should obviously fail, but test if it leaks
15:48:01         pRiVi | jkt: You can use your buggy, old, unpatched PHP server, and are still as secure as your users are not hackers.
15:48:31         pRiVi | jkt: and you can automatically login based on client installation not password or token
15:49:10         pRiVi | jkt: And, if you have a DDOS, you just can go easy to many, many servers.
15:49:14       @zombie | pRiVi: this doesn't sound like extensions issue, please post a comment in bug 1315460 or file another bug blocking it where you
                       | (calmly) describe your use case, and people will take it from there
15:49:15      freonbot | Bug http://bugzil.la/1315460 jkt@mozilla.com, NEW, , P3, Remove support for HTML Keygen
15:49:44         pRiVi | zombie: will do, thanks.
15:49:48           jkt | pRiVi: yeah ^. It sounds like maybe we could hammer this out as an extension API perhaps.
15:50:06         pRiVi | jkt: so I should also file this?
15:50:26           jkt | there has been some talk about using the windows cert store but I think there are reasons not to do this

	Markus Schraeder
	Comment 27 • 6 years ago

Webauthn is currently no alternative cause of this bug:

https://bugzilla.mozilla.org/show_bug.cgi?id=1481890

	Pulsebot
	Comment 36 • 5 years ago

Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/381a74484140
Removal of keygen element r=keeler,baku,jld,hsivonen