Conversation
RISK-alt
requested review from
jdauphant-dinum,
olebrouster-dinum,
plegrand-dinum and
rlahfa-dinum
as code owners
August 11, 2026 22:11
rlahfa-dinum
added
status: awaiting-maintainers
Contributor
|
@RISK-alt We review your contribution and we come back next week |
rlahfa-dinum
reviewed
Sep 2, 2026
rlahfa-dinum
reviewed
Sep 2, 2026
rlahfa-dinum
reviewed
Sep 2, 2026
rlahfa-dinum
requested changes
Sep 2, 2026
rlahfa-dinum
left a comment
rlahfa-dinum
left a comment
Contributor
There was a problem hiding this comment.
I performed a review on the new commits. There's a couple of design questions that are not answered at this point, please take a look. Thank you!
rlahfa-dinum
added
status: awaiting-author
This rework the abstraction module for Firefox into a generic abstraction for both major browsers: Chromium and Firefox. This enables developers to let user choose between Firefox and Chromium and configure them uniformly. Obviously, abstracting all options is impossible, so a developer should always consider using the fine-grained API of a browser if needed. Signed-off-by: Ryan Lahfa <ryan.lahfa.ext@numerique.gouv.fr>
The module as introduced did not evaluate:
- homepage-dashboard.nix read securix.homepage-dashboard.{enable,bookmarks}
without declaring either option;
- lockFlags was typed as an enum although it holds a list of flags;
- nullOr was applied to two arguments instead of to (submodule proxyConfig);
- firefox.nix and chromium.nix consumed a homepage option neither declared;
- chromium.nix dereferenced cfg.proxy unconditionally although it defaults to
null, and read autoConfigUrl where option-types.nix declares autoConfigURL;
- the per-browser extensions were typed loosely enough that each browser could
receive the other one's shape.
Type the extensions submodule so that each browser gets what it expects.
The Chromium proxy policy also used mkIf inside programs.chromium.extraOpts.
That option is a plain attribute set, so the module system never discharges the
property, and {_type = "if"; ...} would be serialised as-is into the policy
JSON. Build the conditional parts with optionalAttrs instead.
Signed-off-by: risk-alt <aldu6974@gmail.com>
securix.browser.enable defaulted to false and modules/tools/default.nix only imported the module, so the browser abstraction removed Firefox from the system instead of configuring it. Enable it by default, and default the browser list to Firefox, which is what Sécurix shipped so far. The default lives on the option rather than in modules/tools/default.nix on purpose: list options concatenate on merge, so a definition there would force users to mkForce their way out of Firefox. Restore the Bitwarden extension the refactor dropped along the way, and install it on Chromium as well. programs.chromium only writes policy files, so install the package explicitly, otherwise picking Chromium configures a browser that is not there. Drop the bare firefox entry from environment.systemPackages: programs.firefox already installs the wrapped package, and shipping both puts two bin/firefox in collision inside the system profile. Enabling the module for real also surfaced one last type error: the browser-wide lockFlags were typed as listOf lockFlagEnum, where lockFlagEnum is the list of allowed flags rather than a type. Signed-off-by: risk-alt <aldu6974@gmail.com>
programs.chromium.initialPrefs feeds /etc/chromium/initial_preferences, whose schema is the initial preferences one, not the Chrome Enterprise policy one. Every policy written there was silently ignored. Move them to extraOpts, which lands in /etc/chromium/policies/managed/extra.json. Three keys do not exist as written, checked against the policy list in components/policy/resources/templates/policies.yaml: - IsolateOrigins is typed as a string holding a comma separated list of origins; the intent here, isolating every origin, is SitePerProcess; - GenAiSettings is GenAiDefaultSettings, an int-enum where 2 means "do not allow GenAI features"; - BuiltInAIAPIIsEnabled is BuiltInAIAPIsEnabled. Wire the home page through programs.chromium.homepageLocation, which was the remaining TODO on the Chromium side, and honour the extension lock flag the way the Firefox module already does. Pass null rather than an empty list when no extension is configured, so that ExtensionInstallForcelist stays out of the generated policy file. Signed-off-by: risk-alt <aldu6974@gmail.com>
RISK-alt
force-pushed
the
browsers-smartcard
branch
from
September 3, 2026 09:19
d0864d7 to
4481fdf
Compare
Contributor
Author
|
Force-pushed, rebased on top of the #239 series. The three items are addressed; the design question on |
The browser modules only produce configuration, so assert on what they emit: the Firefox policy file, both Chromium managed policy files, the presence of each browser in the system profile, and the local homepage dashboard answering on its port. This is what would have caught the policies landing in initial_preferences. Signed-off-by: risk-alt <aldu6974@gmail.com>
securix.firefox is now one browser behind the securix.browser abstraction. Signed-off-by: risk-alt <aldu6974@gmail.com>
…g host modules/tools/firefox.nix installed it before the browser abstraction moved the module, and the move dropped it. Without the native host, Tridactyl degrades silently: no :native commands, no editor integration. Signed-off-by: risk-alt <aldu6974@gmail.com>
ProxyPacMandatory keeps Chromium from falling back to a direct connection when the PAC script is unavailable or invalid. Leaving it false means a fetch failure silently takes the browser around the proxy, which is the opposite of what configuring one is for. The key is only read in pac_script mode, so move it next to ProxyPacUrl. Signed-off-by: risk-alt <aldu6974@gmail.com>
RISK-alt
force-pushed
the
browsers-smartcard
branch
from
September 3, 2026 09:24
4481fdf to
dd30c5c
Compare
Contributor
Author
|
CI note: That is Two things suggest it is a flake rather than a regression from this branch:
The extra VM this branch adds may make the race easier to lose by loading the runner further. I cannot re-run the job myself; a re-run should be enough to confirm. Happy to send a separate PR tightening that assertion into a |
…messaging host" This reverts commit 3369b1152a2e9a0bd90ecd8be76fd8f0be3d1216. Keep this pull request to the browser abstraction itself. Reverting this revert is all it takes to bring the native messaging host back. Signed-off-by: risk-alt <aldu6974@gmail.com>
Browsers on Sécurix have to be told about PKCS#11 modules; smartcards are not picked up on their own. Add a generic option holding the modules to register, and consume it on the Firefox side through the SecurityDevices policy. nixpkgs' Firefox wrapper offers wrapperConfig.smartcardSupport, but it writes the policy to $libDir/distribution/policies.json. Firefox returns the first policy file it finds without merging, and programs.firefox writes /etc/firefox/policies/policies.json, so the wrapper's copy would never be read. Set the policy here instead, with an absolute store path, which also makes the wrapper's library path irrelevant. Use the Add/Delete form of the policy rather than the deprecated flat one (Firefox 114, ESR 112.12). It is also what keeps the registration correct across upgrades: the policy handler only adds a module when no loaded module carries that library path, and the add then fails because the name is already taken, so a store path change would leave the entry pointing at a library the garbage collector has reclaimed. Deleting the name first re-registers it on every start. Signed-off-by: risk-alt <aldu6974@gmail.com>
…r NSS database Chromium has no enterprise policy to install a PKCS#11 module on Linux: it reads the NSS database of the user, in ~/.pki/nssdb. nixpkgs' NSS does not load p11-kit modules either -- useP11kit only swaps libnssckbi for p11-kit's trust module, it does not carry the patch that would load /etc/pkcs11/modules -- so there is no system-wide place to declare them and the database has to be populated per user. Add a user unit that creates the database when it is missing and registers each module with modutil. Each module is deleted before being added, so that the entry follows the store path across upgrades rather than pointing at a library the garbage collector has reclaimed. OpenSC ships pkcs11-register for this, but it also rewrites Firefox profiles, which the SecurityDevices policy already covers, and it registers a different provider for Chrome than for Firefox (OpenSC#1818). Signed-off-by: risk-alt <aldu6974@gmail.com>
Smartcards were not picked up by the browsers Sécurix ships, even though opensc is already in the system profile and pcscd is already enabled. Register the OpenSC PKCS#11 module in both browsers by default, and keep pcscd enabled from here as well, so the browser module does not depend on security-keys.nix being imported. Closes cloud-gouv#208 Signed-off-by: risk-alt <aldu6974@gmail.com>
Assert on both halves of the feature: the SecurityDevices policy Firefox reads, and the NSS database entry Chromium reads, checked through modutil for a lingering test user. Signed-off-by: risk-alt <aldu6974@gmail.com>
Contributor
|
The test failure is known and being taken care of in another PR. |
RISK-alt
force-pushed
the
browsers-smartcard
branch
from
September 3, 2026 09:56
dd30c5c to
e9caa8d
Compare
rlahfa-dinum
approved these changes
Sep 3, 2026
rlahfa-dinum
left a comment
rlahfa-dinum
left a comment
Contributor
There was a problem hiding this comment.
Looks good on the PKCS#11 bits. I will put status blocked until we merge its dependency.
rlahfa-dinum
added
status: blocked
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #208. Stacked on #239, review that one first — this branch starts from
it, so the diff only makes sense once #239 is in.
Smartcards were not picked up by the browsers Sécurix ships, even though opensc
is already in the system profile and pcscd is already enabled by
modules/security-keys.nix. This is only about telling the browsers where themodule lives.
Firefox gets it declaratively, through the
SecurityDevicespolicy. Worthknowing: nixpkgs' Firefox wrapper already offers
wrapperConfig.smartcardSupport, which sets that same policy — but it writes itto
$libDir/distribution/policies.json. Firefox returns the first policy fileit finds and does not merge, and
programs.firefoxwrites/etc/firefox/policies/policies.json, which wins. The wrapper's copy wouldnever be read, so the policy is set explicitly here, with an absolute store
path.
Chromium has no equivalent policy on Linux. It reads the NSS database of the
user, in
~/.pki/nssdb. nixpkgs' NSS does not load p11-kit modules either(
useP11kitonly swapslibnssckbifor p11-kit's trust module), so there is nosystem-wide shortcut: a small user unit creates the database when missing and
registers each module with
modutil. Each module is deleted before being added,so the entry follows the store path across upgrades instead of pointing at a
library the garbage collector has reclaimed.
I did consider OpenSC's own
pkcs11-register, and dropped it: it also rewritesFirefox profiles, which the policy already covers, and it registers a different
provider for Chrome than for Firefox (OpenSC#1818).
The option is generic —
securix.browser.securityDevicestakes any PKCS#11module — with
securix.browser.enableSmartcardsfilling it with OpenSC bydefault, which is what the issue asks for.
Tested in the VM test, both halves separately: the policy file carries the
module path, and
modutil -listshows the security device in the database of alingering test user.
One thing you will see in that test's log, so that nobody has to go looking for
it:
pcscdlogsRejected unauthorized PC/SC clientthere. That is themodutil -listcall loading the module and probing for a reader as a user thetest reaches through
su, with no active session. pcsc-lite's polkit actiongrants
access_pcscto active local sessions only (allow_active = yes,allow_inactive = no), which is what a real Sécurix user has. Registering themodule does not talk to pcscd at all.