Closed Bug 1759162 Opened 2 years ago Closed 2 years ago

PKCS#11 loading on Linux through the browser.pkcs11 API is broken since Bug 1745352 landed

Categories

(WebExtensions :: General, defect, P1)

Firefox 98
defect

Tracking

(firefox-esr91 unaffected, firefox98+ verified, firefox99+ verified, firefox100+ verified)

VERIFIED FIXED
100 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox98 + verified
firefox99 + verified
firefox100 + verified

People

(Reporter: wouter, Assigned: rpl)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [addons-jira])

Attachments

(2 files)

Given a file /usr/lib/mozilla/pkcs11-modules/beidpkcs11.json with the following contents:

{
"name": "beidpkcs11",
"description": "Belgium eID PKCS#11 module",
"path": "libbeidpkcs11.so.0",
"type": "pkcs11",
"allowed_extensions": ["belgiumeid@eid.belgium.be"]
}

then the following call:

browser.pkcs11.isModuleInstalled("beidpkcs11")

will throw an error rather than returning a promise resolving to "true". The browser console says:

OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH

This is in ext-pkcs11.js:54:37.

That line was modified recently through bug 1745352, so I suspect an error in the PathUtils.filename method.

(I can't currently test whether the bug occurs on other platforms; if necessary I can do this next Monday)

Hello,

I’m from QA and attempting to reproduce the issue in order to confirm it, however I’m having some trouble with this. I've attempted to reproduce the issue on the latest Release (98.0/20220304153049) so far, under Ubuntu 16.04 LTS.

Following the information you have provided thus far, I’ve made a “beidpkcs11.json” file with the content you provided and proceeded with putting it in /usr/lib/mozilla/pkcs11-modules/ (the pkcs11-modules folder was not present so I had to create it).

Next I’ve tried to call browser.pkcs11.isModuleInstalled("beidpkcs11"), but the console returns an error stating that browser.pkcs11 is not defined.

Would you mind providing some more detailed steps to reproduce and any extensions you used when you encountered the issue? Thank you !

Flags: needinfo?(w)

You are aware that this is an add-on specific API, right?

You can't run it from the web console, you'd need an add-on console (and you'll need to update the beidpkcs11.json file to allow your add-on to access it).

Flags: needinfo?(w) → needinfo?(acornestean)

@Wouter Verhelst

I’m not very familiar with either PKCS#11 or the used API.

That is why I was asking for some more detailed steps to reproduce the issue as well as the extension you used when you encountered the problem.

Thank you !

Flags: needinfo?(acornestean)

Hi Alex,
I took a look if I could help you to figure out how to reproduce this issue, and if I'm not mistaken the extension mentioned in comment 0 is ulisted but it should be possible to download it manually from https://eid.belgium.be/en

Let me know if this additional detail didn't help yet to confirm the STR to be used to reproduce this issue.

Flags: needinfo?(acornestean)

Hello Luca and thank you for the info !

Based on what you mentioned I managed to find an extension on AMO which matches "allowed_extensions": ["belgiumeid@eid.belgium.be"] from the beidpkcs11.json contents and is designed to use the Belgian eID in Firefox: https://addons.mozilla.org/en-US/firefox/addon/belgium-eid/ .

Now, with the provided “beidpkcs11.json” placed at “/usr/lib/mozilla/pkcs11-modules/”, and the above extension installed, I accessed the add-on console and called browser.pkcs11.isModuleInstalled("beidpkcs11").

As described in the initial description, the browser console logs the following error:
“ OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37 “

For more details, see the attached screenshot.

Reproduced the issue on the latest Nightly (100.0a1/20220314214902), Beta (99.0b3/20220313185831) and Release ( 98.0.1/20220313140707) under Ubuntu 16.04 LTS.

Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(acornestean)

Thanks.

Please note that this issue is causing our add-on to be completely non-functional. We're getting a number of one-star reviews as a result, when it is not really our fault. Can this please be given the appropriate priority?

PathUtils.filename throws if the path is not an absolute path, which was likely
not the case with the OS.Path.basename call used previously.

This internal change of behavior shouldn't be triggering any issue on Windows,
where the hostInfo.manifest.path seems to be always normalized into an absolute
path (by computing it as relative to the hostInfo.manifest if it wasn't already
an absolute path), but it makes browser.pkcs11.isModuleInstalled to regress
on Linux (and maybe also on MacOS if the pkcs11 manifest files include only
the library name and not its full path, as it seems to be the case for
the Belgium eID pkcs11 manifest packaged for Linux).

The result of PathUtils.filename is expected to only include the basename of
the file (without the full dir path and the file extension) and so to fix
the regression being triggered on non-windows platform we could use a fake
absolute url to get the expected result using PathUtils.filename as is.

Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Regressed by: 1745352
Severity: -- → S2
Priority: -- → P1

I'll look into adding a test case to explicitly cover the behavior expected when the pkcs11 module path in the pkcs11 manifest.json file is not an absolute path (based on a quick look to test_ext_pkcs11_management.js it seems that it is using using full paths to the module because that makes it possible to test mock pcks11 modules that are not in directory already part of the default system libraries paths).

Attachment #9267841 - Attachment description: Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand! → WIP: Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand!
Attachment #9267841 - Attachment description: WIP: Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand! → Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand!

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #9)

I'll look into adding a test case to explicitly cover the behavior expected when the pkcs11 module path in the pkcs11 manifest.json file is not an absolute path (based on a quick look to test_ext_pkcs11_management.js it seems that it is using using full paths to the module because that makes it possible to test mock pcks11 modules that are not in directory already part of the default system libraries paths).

I added a basic test assertion to cover the fix for this regression to the existing test cases, and pushed the patch to try:

Set release status flags based on info from the regressing bug 1745352

Pushed by luca.greco@alcacoop.it:
https://hg.mozilla.org/integration/autoland/rev/9907f16a1b8a
Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r=barret,willdurand
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 100 Branch

Verified the fix on the latest Nightly (100.0a1/20220316095231) under Ubuntu 16.04 LTS.

Following the STR used to confirm the issue (Comment 5), calling browser.pkcs11.isModuleInstalled("beidpkcs11") in the add-on console will no longer cause the “ OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37 “ error to be logged to the browser console.

Furthermore, the promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11") is now “fulfilled” as opposed to “rejected” as it was before the fix.

Status: RESOLVED → VERIFIED

Please nominate this for Beta and Release approval when you get a chance.

Flags: needinfo?(lgreco)

Confirmed, works for me in nightly.

Hi Alex, would you mind to also verify the same STR on MacOS and Windows?

The fix shouldn't have changed anything related to the behaviors expected on Windows, but it is theoretically covering both Linux and MacOS.

I'd like to double-check that the STR works fine also on the other two platforms before proceeding to request uplifts for Beta and Release.

Flags: needinfo?(lgreco) → needinfo?(acornestean)

Verified the fix on the latest Nightly (100.0a1/20220316214937) under Windows 10 x64 and macOS 11.3.1.

  • Windows:

I was having trouble identifying the location of where to put the “beidpkcs11.json” on Windows in order to check that the fix hasn’t caused any unwanted effects so I chose, based on Luca’s suggestion, to check the real user flow starting with installing the appropriate software from https://eid.belgium.be/en and the Belgium eID extension from AMO.

I’ve then proceeded to access the extension console and call browser.pkcs11.isModuleInstalled("beidpkcs11_64") (beidpkcs11_64 instead of beidpkcs11, as this seems to be the pkcs11 manifest installed with the software above).

No “OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37“ error was logged to browser console and the promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11_64") is “fulfilled”.

So there is no negative impact on Windows as a result of the fix.

  • MacOS:

I’ve placed the provided “beidpkcs11.json” file at /Library/Application Support/Mozilla/PKCS11Modules/
and calling browser.pkcs11.isModuleInstalled("beidpkcs11") in the add-on console did not throw any “OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37“ error in the browser console.

The promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11") is also “fulfilled”.

This should confirm that the issue is also fixed on macOS.

Flags: needinfo?(acornestean)

Comment on attachment 9267841 [details]
Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand!

Beta/Release Uplift Approval Request

  • User impact if declined: This issue may prevent to Linux (and likely also MacOS) users to log in on eGovernment websites (like Belgium eID case reported in this issue) that are using pkcs11 modules.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: same STR used to verify the fix on Nightly.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The risk should be low:
  • the fix is small, covered by automated tests (with additions to trigger the scenario that was not covered before and prevented us from catching the regressions as soon as introduced)
  • the fix is also limited to the part of the ext-pkcs11.js WebExtensions API script that is looking up the pkcs11 manifest files and it is not expected to affect any other behavior besides the one regressed by Bug 1759162 in Firefox 98 (and no change in behavior on the Windows platform, where the issues was not reproducible).
  • String changes made/needed:
Attachment #9267841 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9267841 [details]
Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand!

Beta/Release Uplift Approval Request

(If accepted for release too, it may also be worth to let QA to verify the uplift to beta first before uplifting it also to release, but it is worth to mention that the module didn't change in Firefox 99 and so we don't expect any difference between the fix applied to beta and the same fix applied to release)

  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Same as Bug 1759162 comment 19
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Same as Bug 1759162 comment 19
  • String changes made/needed:
Attachment #9267841 - Flags: approval-mozilla-release?
Whiteboard: [addons-jira]

Comment on attachment 9267841 [details]
Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand!

Approved for 99.0b5. Thanks.

Attachment #9267841 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

wfm in beta.

Verified the fix on the latest Beta (99.0b6/20220320185956) under Windows 10 x64, Ubuntu 16.04 LTS and macOS 11.3.1.

Ubuntu:
Following the STR from Comment 5, calling browser.pkcs11.isModuleInstalled("beidpkcs11") in the add-on console will not throw the “ OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37 “ error in the browser console.
Furthermore, the promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11") is also “fulfilled”.

MacOS:
Following the STR used to verify the fix on Nightly in Comment 18, calling browser.pkcs11.isModuleInstalled("beidpkcs11") in the add-on console did not throw any “OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37“ error in the browser console.
The promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11") is also “fulfilled”.

Windows:
Following the STR used to verify the fix on Nightly in Comment 18, no “OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37“ error was logged to browser console and the promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11_64") is “fulfilled”.

In conclusion, the fix is confirmed on Beta, on Linux and macOS. Furthermore, no negative impact was observed on Windows as a result of the fix.

Comment on attachment 9267841 [details]
Bug 1759162 - Fix pkcs11.isModuleInstalled regressions triggered on non-windows builds. r?barret!,willdurand!

Regression, P1/S2, verified by QA on nightly and Beta 6 => approved for 98.0.2, thanks.

Attachment #9267841 - Flags: approval-mozilla-release? → approval-mozilla-release+

Verified the fix on the latest Release (98.0.2/20220322144853) under Windows 10 x64, Ubuntu 16.04 LTS and macOS 11.3.1.

Following the steps from Comment 24, the same results have been obtained as on Beta i.e.

On macOS and Ubuntu:

  • calling browser.pkcs11.isModuleInstalled("beidpkcs11") in the add-on console will not throw the “ OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37 “ error in the browser console.
  • the promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11") is “fulfilled”.

On Windows:

  • calling browser.pkcs11.isModuleInstalled("beidpkcs11_64") in the add-on console will not throw the “ OperationError: PathUtils.filename: Could not initialize path: NS_ERROR_FILE_UNRECOGNIZED_PATH ext-pkcs11.js:54:37 “ error in the browser console.
  • the promise resulting from calling browser.pkcs11.isModuleInstalled("beidpkcs11_64") is “fulfilled”.

In conclusion, the fix is confirmed on Firefox Release, on Linux and macOS, and no negative impact has been observed on Windows.

QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: