1156204 - [Settings] Add support for PKCS#12 files

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Arthur Chen [:arthurcc][inactive after 6/30]]

Recently gecko added support for PKCS#12 files[1]. We should list files with filename extension of `pfx` and `p12` in the certificate import panel.


[1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1012549

Comment 1

[:gerard-majax]

I've tested and it's not enough: my certificates are seen by the Settings app, but then importing somehow fails.

Comment 2

[:gerard-majax]

So far I could debug in Gecko WifiCertService, I'm getting into the CryptoTask and ReadBlob() succeeded. It fails later.

Comment 3

[Arthur Chen [:arthurcc][inactive after 6/30]]

As described in comment 1 there are errors when importing PKCS#12 files. Vincent, could you help confirm this? Thanks.

Comment 4

[Vincent Chang[:vchang][changyihsin]]

Hi Chuck, not sure if you could provide some comments about comment 2.

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Hi Alexandre,
  Did you provide password for importing pkcs12 file?
  Current settings app doesn't provide password for certificate because it's not necessary.
  But when it comes to pkcs12, you have to provide password in WifiManager.importCert[1].

[1] https://dxr.mozilla.org/mozilla-central/source/dom/webidl/MozWifiManager.webidl#254

Comment 6

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Arthur Chen [:arthurcc] from comment #0)
> Recently gecko added support for PKCS#12 files[1]. We should list files with
> filename extension of `pfx` and `p12` in the certificate import panel.
> 
> 
> [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1012549

Hi Arthur,
  Please note that to support importing pkcs12 files, adding file extension is not enough.
  You also have to ask user for password and pass it to API call, as mentioned in comment 5.

  Please let me know if you need any help!

Comment 7

[:gerard-majax]

That's probably what is missing, my pkcs12 is password protected. And I have not been prompted for a password.
Besides, maybe getting a better error message than "Import cert failed" could help, too.

Comment 8

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

I'm not sure b2g implements the password-prompting machinery. Something like that would be necessary here.

Comment 9

[Arthur Chen [:arthurcc][inactive after 6/30]]

Hey Chuck,

Thanks for the clarification. Is the password required while importing other types of certificate? How can I tell if a certificate needs a password before importing it?

Comment 10

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I think the only way is to determine by the file extension, only pfx and p12 require password.

Comment 11

[GitHub Autolander Bot]

Attachment: [gaia] crh0716:1156204 > mozilla-b2g:master

Comment 12

[Arthur Chen [:arthurcc][inactive after 6/30]]

Jenny, importing some types of certificates requires password. Could you also take this into consideration while proposing specs for bug 1155995?

Lissy, I've attached a WIP. Could you try whether it works in your end?

Thanks!

Comment 13

[:gerard-majax]

Thanks, giving this WIP a try.

Comment 14

[:gerard-majax]

(In reply to Arthur Chen [:arthurcc] from comment #12)
> Jenny, importing some types of certificates requires password. Could you
> also take this into consideration while proposing specs for bug 1155995?
> 
> Lissy, I've attached a WIP. Could you try whether it works in your end?
> 
> Thanks!

I'm getting the password prompt, the certificate name prompt, and no failure is reported, but the list of imported certificates is still empty.

Comment 15

[Arthur Chen [:arthurcc][inactive after 6/30]]

If you did not encounter any error it is likely that `mozWifiManager.getImportedCerts` did not return the correct result.

Chuck, any idea on this? Can we tell the password is incorrect while importing or it simply returns a generic error? Thanks in advance!

Comment 16

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Hi Arthur,
  I still can't get emulator up running, but I remember there will be a common error for import error of any cause.
  The WIP patch treat return value of API as Promise, but wifi API uses DOMReqeust now, maybe that's why you can't get error.

Comment 17

[Arthur Chen [:arthurcc][inactive after 6/30]]

Makes sense. I've updated the PR to use DOMRequest. Let's see if there is any error reported.

Comment 18

[:gerard-majax]

(In reply to Arthur Chen [:arthurcc] from comment #17)
> Makes sense. I've updated the PR to use DOMRequest. Let's see if there is
> any error reported.

Still no error, no certificate imported, and nothing in logcat.

Comment 19

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: pkcs12_debug.patch

Hi Alexandre,
  This patch can dump log to trace the execution of wifi certificate service, please apply it and upload the log you get.
  So we can find out what goes wrong.

Comment 20

[:gerard-majax]

(In reply to Chuck Lee [:chucklee, OOO forever, will response to NI, Review, and Feedback requests] from comment #19)
> Created attachment 8599267 [details] [diff] [review]
> pkcs12_debug.patch
> 
> Hi Alexandre,
>   This patch can dump log to trace the execution of wifi certificate
> service, please apply it and upload the log you get.
>   So we can find out what goes wrong.

I can, but I feel really bad knowing that we have an API unable to give us usable error feedback.

Comment 21

[:gerard-majax]

Isn't bug 875188 somehow related to this ?

Comment 22

[:gerard-majax]

The log is pretty short:
> 04-30 11:59:53.659   327   327 D Chucklee: ######## ImportCertTask::ImportCertTask()
> 04-30 11:59:53.659   327  1659 D Chucklee: ######## ImportCertTask::CalculateResult()
> 04-30 11:59:53.669   327  1659 D Chucklee: ######## ImportPkcs12Blob()@359
> 04-30 11:59:53.669   327  1659 D Chucklee: ######## ImportPkcs12Blob()@375
> 04-30 11:59:53.679   327  1659 D Chucklee: ######## ImportPkcs12Blob()@391
> 04-30 11:59:53.679   327  1659 D Chucklee: ######## HandleNicknameUpdate()@316, certType 0x00000080, default_nickname == NULL
> 04-30 11:59:53.679   327  1659 D Chucklee: ######## HandleNicknameUpdate()@333, nickname WIFI_USERCERT_lissyx-sdcard0
> 04-30 11:59:53.679   327  1659 D Chucklee: ######## ImportPkcs12Blob()@404
> 04-30 11:59:53.679   327  1659 D Chucklee: ######## ImportPkcs12Blob()@413
> 04-30 11:59:53.679   327  1659 D Chucklee: ######## ImportPkcs12Blob()@429
> 04-30 11:59:53.799   327  1659 D Chucklee: ######## ImportPkcs12Blob()@438, Done, usage 02
> 04-30 11:59:53.799   327   327 D Chucklee: ######## ImportCertTask::CallCallback()
> 04-30 11:59:53.799   327   327 I Gecko   : ######## Gecko:WifiWorker.js:importCert(), result data: {"id":33,"nickname":"lissyx-sdcard0","status":0,"usageFlag":2}
> 04-30 11:59:53.809   327   327 I Gecko   : ######## Gecko:WifiWorker.js:importCert(), usage: ["UserCert"]

Comment 23

[:gerard-majax]

Arthur, is it expected that I get no error displayed when I input an invalid password?

With the logging patch in this case, I do get:
> 04-30 12:03:19.589   327  1873 D Chucklee: ######## ImportPkcs12Blob()@396, Can't Verify PKCS12 decoder
> 04-30 12:03:19.589   327   327 D Chucklee: ######## ImportCertTask::CallCallback()
> 04-30 12:03:19.589   327   327 I Gecko   : ######## Gecko:WifiWorker.js:importCert(), result data: {"id":52,"nickname":"lissyx-sdcard1","status":-1,"usageFlag":0}

Comment 24

[:gerard-majax]

And the import do succeeds if I take a look at the certificate database from my device's profile:
> $ certutil -L -d sql:.
> WIFI_USERCERT_lissyx-sdcard0                                 u,u,u

Comment 25

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #24)
> And the import do succeeds if I take a look at the certificate database from
> my device's profile:
> > $ certutil -L -d sql:.
> > WIFI_USERCERT_lissyx-sdcard0                                 u,u,u

And of course, the private key is also here:
> $ certutil -K -d sql:.
> certutil: Checking token "NSS Certificate DB" in slot "NSS User Private Key and Certificate Services"
> < 0> rsa      xxx   WIFI_USERCERT_lissyx-sdcard0

Comment 26

[Arthur Chen [:arthurcc][inactive after 6/30]]

If the API returns an error, an alert will pop up.

Comment 27

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I think Bug 867899 is irrelevant to this.
The log shows the execution result is already sent back to IPC, but fails to trigger DOMRequest, which I didn't expect could go wrong.
Since both success and error event can't be triggered, the issue might be WifiWorker failed to send message to DOMWifiManager through IPC [1], or DOMWifiManager can't find corresponding request object of message [2][3].
If I still can't get my emulator back to work this weekend, I'll find someone to pick it up next week.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js#3523
[2] https://dxr.mozilla.org/mozilla-central/source/dom/wifi/DOMWifiManager.js#194
[3] https://dxr.mozilla.org/mozilla-central/source/dom/wifi/DOMWifiManager.js#200

Comment 28

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

There's certificate importing test in marionette test[1], and it doesn't show test fail in these days[2].
So I don't have much clue on what and why it goes wrong.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/wifi/test/marionette/test_wifi_manage_pkcs12_certificate.js
[2] https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&filter-searchStr=MNW&exclusion_profile=false

Comment 29

[Jenny Lee]

Attachment: [Settings] Wi-Fi v1.41.pdf

Hi all,
See attached for updated spec. Per discussion with Arthur, I understand that part of the flow cannot be supported at this time as a certain APIs are missing, in order to provide better UX, I suggest we fill up the technical gap first. Thanks!

Comment 30

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: test_12345678.p12

Hi Vincent,
  I still can't get emulator up running to test import certificate manually, but marionette test for importing certificate shows no error.
  Based on log in comment 22 and comment 23, I think the issue happens while WifiWorker returning result to DOMWifiManager.
  Can you help find someone available to test issue as comment 18 on real device, with Authur's Gaia patch?
  Thanks!

Comment 31

[:gerard-majax]

No wonder why the test succeeds: plugging into WifiWorker.getImportedCerts() I do see my certificate being pulled from NSS DB. So it would looks like the WifiWorker/DOMWifiManager communication might be at fault here.

Comment 32

[:gerard-majax]

https://github.com/mozilla-b2g/gaia/blob/eb3ee5e2b9bf14e133319be605e255073bf870e4/apps/settings/js/panels/wifi_manage_certificates/panel.js#L36
https://github.com/mozilla-b2g/gaia/blob/41d07c112d6e9408887129aa8f0f8f68feaf4ef5/apps/settings/js/modules/wifi_utils.js

Why are we searching for ServerCert instead of UserCert ?

Comment 33

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Alexandre LISSY :gerard-majax from comment #31)
> No wonder why the test succeeds: plugging into WifiWorker.getImportedCerts()
> I do see my certificate being pulled from NSS DB. So it would looks like the
> WifiWorker/DOMWifiManager communication might be at fault here.

The test case also calls WifiManager.importCert(), and it needs DOMRequest.onsuccess()[1] to complete the test, or the test will timeout and fail.
This is a contradiction to the assumption of WifiWorker/DOMWifiManager communication failure.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/wifi/test/marionette/head.js#465

Comment 34

[:gerard-majax]

Indeed, Chuck, check the comment just above: everything is fine on Gecko side. Gaia is just looking at the wrong set of certificates ... Looks like bug 998226 needs some work too.

Comment 35

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Alexandre LISSY :gerard-majax from comment #34)
> Indeed, Chuck, check the comment just above: everything is fine on Gecko
> side. Gaia is just looking at the wrong set of certificates ... Looks like
> bug 998226 needs some work too.

ServerCert is list of imported server certificate, and UserCert is list of imported user certificates.
Since now we support both certificates, I think we need to show both lists.

Thank you for pointing this out.

Comment 36

[Vincent Chang[:vchang][changyihsin]]

(In reply to Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests] from comment #27)
> I think Bug 867899 is irrelevant to this.
> The log shows the execution result is already sent back to IPC, but fails to
> trigger DOMRequest, which I didn't expect could go wrong.
> Since both success and error event can't be triggered, the issue might be
> WifiWorker failed to send message to DOMWifiManager through IPC [1], or
> DOMWifiManager can't find corresponding request object of message [2][3].
> If I still can't get my emulator back to work this weekend, I'll find
> someone to pick it up next week.
> 
> [1]
> https://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js#3523
> [2]
> https://dxr.mozilla.org/mozilla-central/source/dom/wifi/DOMWifiManager.js#194
> [3]
> https://dxr.mozilla.org/mozilla-central/source/dom/wifi/DOMWifiManager.js#200

You can rollback gaia commit to 2 weeks ago to make emulator-kk work.

Comment 37

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

It works, thanks Vincent!
After test, Gecko part works as expected.

I found a typo in Gaia that causes no response to import error event, please refer to the comment in the pull request.
And the reason of no response while import success is as Alexandre pointed out in comment 32.

And I'll try if it's possible to provide more detailed error message, at least on password error.

Comment 38

[Arthur Chen [:arthurcc][inactive after 6/30]]

We only support importing server certificates for now.

Comment 39

[[e-mail]]

take for track

Comment 40

[[e-mail]]

de-assign myself, will deal with it when get priority

Comment 41

[BMO Automation]

Firefox OS is not being worked on