Skip to content

Conversation

@provokateurin
Copy link
Member

Replaces #405873.

I tested this on my homelab test VM and the self-signed certificates are now happily accepted when set through security.pki.certificateFiles.
I also want to add a test for this, but don't have the time to do that right now, therefore I want to keep the PR in draft until that is done.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels May 11, 2025
@provokateurin provokateurin requested a review from Ma27 May 11, 2025 19:44
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 11, 2025
@Ma27
Copy link
Member

Ma27 commented May 12, 2025

I also want to add a test for this, but don't have the time to do that right now, therefore I want to keep the PR in draft until that is done.

Hmm, maybe it's sufficient to expose the minio (or whatever we use) in the object-store test behind TLS and use the test CA we have in nixos/tests? If this works with verification enabled and the CA added to the system-wide bundle of the Nextcloud server, that's it I think.

Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an almost-working test btw ;-)


Paging cacert maintainers @fpletz @lukegb

@provokateurin
Copy link
Member Author

I have an almost-working test

Feel free to push :)

I will do the other adjustments on the weekend or next week.

@Ma27
Copy link
Member

Ma27 commented May 13, 2025

Done!

I will do the other adjustments on the weekend or next week.

No worries.
We may miss the official branch-off, but IMHO this is perfectly reasonable to backport.

@provokateurin
Copy link
Member Author

@Ma27 If you don't mind you can also do the changes from the review, I agree with all of them. Maybe we can merge it before the branch-off then :)

@Ma27 Ma27 force-pushed the nixos-nextcloud-use-cacert branch from 8ae675d to 553c4a6 Compare May 13, 2025 14:25
@Ma27 Ma27 marked this pull request as ready for review May 13, 2025 14:26
@nix-owners nix-owners bot requested review from bachp, britter and dotlambda May 13, 2025 14:32
@Ma27 Ma27 force-pushed the nixos-nextcloud-use-cacert branch from 553c4a6 to c90e4f3 Compare May 14, 2025 10:53
@provokateurin
Copy link
Member Author

@Ma27 can you fix the formatting?

@Ma27 Ma27 force-pushed the nixos-nextcloud-use-cacert branch from c90e4f3 to 36a240f Compare May 15, 2025 13:12
@Ma27 Ma27 force-pushed the nixos-nextcloud-use-cacert branch from 36a240f to 9368e14 Compare May 15, 2025 15:47
@Ma27
Copy link
Member

Ma27 commented May 15, 2025

@provokateurin I just realized that we can directly use the caBundle from the CA module by making this a parameter for the nextcloud package. We also need cacert, but this doesn't even get evaluated when using the module because laziness.

I like this even more because the option feels a little pointless given that the relevant path is already exposed. Wdyt?

@provokateurin
Copy link
Member Author

Ah this solution is a lot cleaner, but the commit message was not adjusted and still mentions security.pki.cacertPackage.

provokateurin and others added 2 commits May 15, 2025 18:19
Nextcloud manages the CA bundle on its own by default, but we patch this
out and replace it with the system-wide bundle.

Since this was originally designed for the objectstore feature, this
test ensures that an S3 behind a reverse proxy with TLS and its own CA
works fine.
@Ma27 Ma27 force-pushed the nixos-nextcloud-use-cacert branch from 9368e14 to e9f70c9 Compare May 15, 2025 16:20
@Ma27
Copy link
Member

Ma27 commented May 15, 2025

Good catch, fixed!

@provokateurin
Copy link
Member Author

Cool, looks ready to merge to me!

@Ma27 Ma27 merged commit 26126c7 into NixOS:master May 15, 2025
18 of 21 checks passed
@provokateurin provokateurin deleted the nixos-nextcloud-use-cacert branch May 15, 2025 20:28
@SuperSandro2000
Copy link
Member

Someone knows how to use compressDrvWeb now?

The following fails since this PR:

services.nextcoud.package = pkgs.compressDrvWeb pkgs.nextcloud31 {
  extraFindOperands = ''-not -iregex ".*(\/apps\/.*\/l10n\/).*"'';
};
       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: attribute 'override' missing
       at /nix/store/aj454c0f7biyfxzpinmw28y90377x9jv-source/nixos/modules/services/web-apps/nextcloud.nix:13:21:
           12|
           13|   overridePackage = cfg.package.override {
             |                     ^
           14|     inherit (config.security.pki) caBundle;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants