[Settings] Add support for PKCS#12 files

RESOLVED WONTFIX

Status

Firefox OS
Gaia::Settings
RESOLVED WONTFIX
3 years ago
4 months ago

People

(Reporter: arthurcc, Unassigned)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(tracking-b2g:backlog)

Details

Attachments

(4 attachments)

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
I've tested and it's not enough: my certificates are seen by the Settings app, but then importing somehow fails.
So far I could debug in Gecko WifiCertService, I'm getting into the CryptoTask and ReadBlob() succeeded. It fails later.
As described in comment 1 there are errors when importing PKCS#12 files. Vincent, could you help confirm this? Thanks.
Flags: needinfo?(vchang)
Hi Chuck, not sure if you could provide some comments about comment 2.
Flags: needinfo?(vchang)
Flags: needinfo?(dkeeler)
Flags: needinfo?(chuckli0706)
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
Flags: needinfo?(chuckli0706) → needinfo?(lissyx+mozillians)
(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!
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.
Flags: needinfo?(lissyx+mozillians)
I'm not sure b2g implements the password-prompting machinery. Something like that would be necessary here.
Flags: needinfo?(dkeeler)
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?
Flags: needinfo?(chuckli0706)
I think the only way is to determine by the file extension, only pfx and p12 require password.
Flags: needinfo?(chuckli0706)
tracking-b2g: --- → +
Created attachment 8598439 [details] [review]
[gaia] crh0716:1156204 > mozilla-b2g:master
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!
Status: NEW → ASSIGNED
blocking-b2g: --- → 3.0?
Flags: needinfo?(lissyx+mozillians)
Flags: needinfo?(jelee)
Thanks, giving this WIP a try.
Flags: needinfo?(lissyx+mozillians)
(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.
Flags: needinfo?(arthur.chen)
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!
Flags: needinfo?(arthur.chen) → needinfo?(chuckli0706)
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.
Flags: needinfo?(chuckli0706) → needinfo?(arthur.chen)
Makes sense. I've updated the PR to use DOMRequest. Let's see if there is any error reported.
Flags: needinfo?(arthur.chen)
(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.
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.
Flags: needinfo?(lissyx+mozillians)
blocking-b2g: 3.0? → ---
(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.
Flags: needinfo?(lissyx+mozillians)
Isn't bug 875188 somehow related to this ?
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"]
Flags: needinfo?(chuckli0706)
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}
Flags: needinfo?(arthur.chen)
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
(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
If the API returns an error, an alert will pop up.
Flags: needinfo?(arthur.chen)
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
Flags: needinfo?(chuckli0706)

Comment 29

3 years ago
Created attachment 8600773 [details]
[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!
Flags: needinfo?(jelee)
Created attachment 8600876 [details]
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!
Flags: needinfo?(vchang)
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.
(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
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.
(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.
(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.
Flags: needinfo?(vchang)
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.
We only support importing server certificates for now.
Flags: needinfo?(ejchen)

Comment 39

3 years ago
take for track
Assignee: crh0716 → gasolin
Status: ASSIGNED → NEW

Comment 40

3 years ago
de-assign myself, will deal with it when get priority
Assignee: gasolin → nobody
tracking-b2g: + → backlog

Comment 41

4 months ago
Firefox OS is not being worked on
Status: NEW → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.