Closed Bug 1315460 Opened 7 years ago Closed 4 years ago

Remove support for HTML Keygen

Categories

(Core :: DOM: Core & HTML, defect, P3)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla69
Tracking Flags:
Tracking Status
firefox69 --- fixed

People

(Reporter: ntim, Assigned: jkt)

References

(Blocks 3 open bugs)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(1 file, 4 obsolete files)

Bug 1315460 - Removal of keygen element
47 bytes, text/x-phabricator-request
Details | Review 
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
Priority: -- → P3
(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.
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.
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.
Depends on: 1424548
Attached patch bug-1315460_htmlparser.patch (obsolete) — — Splinter Review 
Removal from the html parser.
Attached patch bug-1315460_cpp.patch (obsolete) — — Splinter Review 
Getting errors in: accessible/tests/mochitest/elm/test_HTMLSpec.html after fixing.

Also getting errors when building without: nsKeygenHandler.cpp and nsKeygenHandlerContent.cpp
Attached patch bug-1315460_cpp.patch (obsolete) — — Splinter Review 

        Attachment #8937738 -
        Attachment is obsolete: true

    
    

    
  
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.
No longer depends on: 1088063
Flags: needinfo?(dkeeler)
See Also:  → 1088063

    
    

    
  
I may as well take this also, however if I am slow in completing please speak to me.
Assignee: nobody → jkt

    
    

    
  
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.
Flags: needinfo?(franziskuskiefer)

    
    

    
  
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.
Flags: needinfo?(dkeeler)

    
    

    
      
  


  

    
    

    
  
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.
Flags: needinfo?(franziskuskiefer)

    
  
Depends on: 1426672

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
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?

    
    

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

    
    

    
  
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.

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
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

    
    

    
  
Webauthn is currently no alternative cause of this bug:

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

    
    

    
      
  

    
    

    
      
  

    
  
Blocks: 1477576

    
    

    
  
@darkspirit: Donnot forget that bug #1529973 blocks this bug, the removing of the keygen-tag.



    
    

    
  
Which was created as a duplicate as concretion of things to do in honor of bug #1481890 , which is the cause of blocking this bug, the removing of the keygen-tag.



    
    

    
  
And why is bug #1315460 no longer blocking this bug, the removing of the keygen-tag?



    
    

    
  
... of course I mean bug 1088063



    
  

        Attachment #8938110 -
        Attachment is obsolete: true

    
    

    
  
The linker issues I had last time appear to magically have resolved themselves in the latest try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff2d0358d70332a9aff8aed9af68f661748c9cb5


I will split out the changes into the htmlparser again assuming that tests are all passing.



    
  
Blocks: 1558793

    
    

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

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/381a74484140


Status: NEW → RESOLVED
Closed: 4 years ago

          status-firefox69:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

    
    

    
  
Webcrypto is NOT a replacement for using client interface certificates.


How is the new way to automatically import client certicates now? Why has this been removed if there is no one?



    
  

        Attachment #8937885 -
        Attachment is obsolete: true

    
  

        Attachment #8937719 -
        Attachment is obsolete: true

    
  
Blocks: 236684

    
  
Blocks: 125555

    
  
Blocks: 1165312

    
  
Blocks: 420193

    
  
Blocks: 1175144

    
  
Blocks: 960888

    
  
Blocks: 941394

    
  
Blocks: 244878

    
    

    
  
Client certificates are still importable from the preferences panel which is the primary use case covered in bug https://bugzilla.mozilla.org/show_bug.cgi?id=1088063 and further solutions are being discussed there.

As far as I am aware no other browser has automatic enrollment of client certificates and this is a privacy vector we don't want without a user having clear intent.


Bugs aren't really the correct place for discussion, I would like to direct you to https://groups.google.com/forum/#!msg/mozilla.dev.platform/SAh1b1R5lrY/UUHU2UNHAgAJ to continue this.



    
  
Blocks: 1559366

    
    

    
  
Documentation updates:


    

  • Submitted BCD PR 4605 labeling <keygen> as removed in Firefox 69
    • 

  • Listed on Firefox 69 for developers
    • 



Keywords: dev-doc-needed → dev-doc-complete

    
  
Blocks: 1581796

    
    

    
  
Clarification question, do you intend to keep the ability to generate a key pair from within Firefox? I believe currently there isn't a way to trigger that, now that keygen has been removed. If you keep it, it would be useful for things like bug 1581796.


Flags: needinfo?(jkt)

    
    

    
  
:keeler is in a better position to answer our key generation APIs, redirecting to her.


Flags: needinfo?(jkt) → needinfo?(dkeeler)

    
    

    
  
Client certificates have a number of drawbacks (privacy, ux, etc.), and so while Firefox will support them, we don't want to encourage their use going forward. Consequently, I don't see a reason for Firefox to support key pair generation (outside of webcrypto).


Flags: needinfo?(dkeeler)
Regressions: 1603202

          You need to log in
          before you can comment on or make changes to this bug.