nixos/molly-brown: init #92220

ehmry · 2020-07-03T19:59:20Z

Motivation for this change

Time for our first Gemini server module.

https://tildegit.org/solderpunk/molly-brown

The TLS certificate stuff is not ideal, clients complain about self-signed certificates. It would be nice to arrange an ACME certificate automatically, but ~~I'm not sure how best to do that without enabling an HTTP server~~ borrowing certs from HTTP servers can be reasonably done.

Things done

aanderse

I took a look at the module and provided some feedback. I hope you find it useful. If you need any further details please don't hesitate to ask.

nixos/modules/misc/ids.nix

nixos/modules/services/web-servers/molly-brown.nix

aanderse · 2020-07-05T00:52:06Z

nixos/modules/services/web-servers/molly-brown.nix

+          rm /tmp/request.pem
+        fi
+      '');
+      serviceConfig = {


Any hardening options we could apply?

ehmry · 2020-07-07T11:45:34Z

@aanderse thank you for the thorough review, I think I've fixed everything you found except for the systemd hardening. Since the server is running as an unprivileged user I hope its not an issue. I'm not comfortable throwing a systemd security salad at it, unless there were a set of re-usable rules that I could apply.

pkgs/servers/gemini/molly-brown/default.nix

fgaz

Just some style stuff on the package

fgaz · 2020-07-08T13:43:43Z

pkgs/servers/gemini/molly-brown/default.nix

+  goPackagePath = "tildegit.org/solderpunk/molly-brown";
+
+  src = fetchgit {
+    inherit rev;


I'd put the revision directly here and remove rec since it seems to be used here only

The go2nix tool placed rec here and I don't know why. If it is harmless I would keep it as.

pkgs/servers/gemini/molly-brown/default.nix

aanderse · 2020-07-09T11:30:32Z

@ehmry I didn't initially realize this is a web server... in this case DynamicUser may not be appropriate (as a default). Sorry for potentially steering you down the wrong path there 😞

I took a quick pass at the module to explain more along the lines of what I was thinking with settings. See my work here. Note that we still have some regular options, which can be used by NixOS to provide extra value (like say adding an openFirewall option), and then the settings option which more detailed configuration can go into. The settings option absorbs all the regular options, but still allows users to override those via services.molly-brown.settings.Port = lib.mkForce 4000;.

How does this look?

ehmry · 2020-07-12T20:31:09Z

Looks better to me. What is the problem with using DynamicUser here? Can we get away with DynamicUser for now?

aanderse · 2020-07-12T21:00:43Z

@ehmry the issue in using DynamicUser here is that you have a logDir option. If you hard-coded the logging directory to /var/log/molly-brown and create it with systemd via LogsDirectory then this wouldn't be an issue.

ehmry · 2020-07-14T12:19:17Z

Then I think hardcoding the log directory is reasonable.

aanderse · 2020-07-15T10:38:24Z

@ehmry I'm that case feel free to remove the users.user and users.group declarations and and DynamicUser back 👍

ehmry · 2020-07-15T16:33:10Z

Ok, rebased and squashed it all against master.

aanderse · 2020-07-16T10:53:05Z

I had never even heard of this protocol until this PR and now we have this too: https://github.com/NixOS/nixpkgs/pull/93209/files
I'll try this out in the next couple days and approve the module part of this PR, but we still need someone to review the package.

nixos/modules/services/web-servers/molly-brown.nix

sikmir · 2020-07-16T15:39:13Z

I had never even heard of this protocol until this PR and now we have this too: https://github.com/NixOS/nixpkgs/pull/93209/files

Another one Gemini client: #91453

aanderse

I did some testing and aside from the two minor outstanding comments I have I approve the module part of this PR. Great job 🎉

nixos/modules/services/web-servers/molly-brown.nix

aanderse · 2020-07-22T22:30:27Z

@ehmry are we good to merge this?
@sikmir and @fgaz did you both review and approve the package portion of this PR? I only looked at the module.

ehmry · 2020-07-23T09:34:50Z

I'd say its ready to merge, I just tested the SupplimentaryGroups change and its working - gemini://depot.1337.cx/init.gmi

sikmir · 2020-07-23T13:18:16Z

LGTM, approved.

ofborg bot added 6.topic: nixos 8.has: module (update) 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Jul 3, 2020

aanderse requested changes Jul 5, 2020

View reviewed changes

ehmry force-pushed the molly-brown branch from 87a965c to 0e40fb7 Compare July 7, 2020 11:36

ehmry marked this pull request as ready for review July 7, 2020 11:45

ehmry requested review from fgaz and sikmir July 7, 2020 11:57

ehmry force-pushed the molly-brown branch from 0e40fb7 to 2f245b4 Compare July 7, 2020 12:02

sikmir reviewed Jul 8, 2020

View reviewed changes

pkgs/servers/gemini/molly-brown/default.nix Outdated Show resolved Hide resolved

ehmry force-pushed the molly-brown branch from 2f245b4 to fef7b98 Compare July 8, 2020 07:54

fgaz reviewed Jul 8, 2020

View reviewed changes

ehmry force-pushed the molly-brown branch from fef7b98 to d3d1e80 Compare July 9, 2020 07:46

ehmry force-pushed the molly-brown branch from 8cd9326 to 2b97ed0 Compare July 15, 2020 16:52

aanderse reviewed Jul 16, 2020

View reviewed changes

nixos/modules/services/web-servers/molly-brown.nix Outdated Show resolved Hide resolved

sikmir approved these changes Jul 16, 2020

View reviewed changes

aanderse requested changes Jul 18, 2020

View reviewed changes

nixos/modules/services/web-servers/molly-brown.nix Outdated Show resolved Hide resolved

ehmry force-pushed the molly-brown branch 2 times, most recently from 0aa1e2e to 74539f8 Compare July 21, 2020 21:34

ehmry added 2 commits July 23, 2020 11:31

molly-brown: init at 2020-07-06

2ba2b3e

nixos/molly-brown: init

9687fcb

ehmry force-pushed the molly-brown branch from 74539f8 to 9687fcb Compare July 23, 2020 09:35

ehmry merged commit 76d60b0 into NixOS:master Jul 24, 2020

ehmry deleted the molly-brown branch July 24, 2020 09:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/molly-brown: init #92220

nixos/molly-brown: init #92220

ehmry commented Jul 3, 2020 •

edited

aanderse left a comment

aanderse Jul 5, 2020

ehmry commented Jul 7, 2020

fgaz left a comment

fgaz Jul 8, 2020

ehmry Jul 9, 2020

aanderse commented Jul 9, 2020

ehmry commented Jul 12, 2020

aanderse commented Jul 12, 2020

ehmry commented Jul 14, 2020

aanderse commented Jul 15, 2020

ehmry commented Jul 15, 2020

aanderse commented Jul 16, 2020

sikmir commented Jul 16, 2020

aanderse left a comment

aanderse commented Jul 22, 2020

ehmry commented Jul 23, 2020

sikmir commented Jul 23, 2020

nixos/molly-brown: init #92220

nixos/molly-brown: init #92220

Conversation

ehmry commented Jul 3, 2020 • edited

Motivation for this change

Things done

aanderse left a comment

Choose a reason for hiding this comment

aanderse Jul 5, 2020

Choose a reason for hiding this comment

ehmry commented Jul 7, 2020

fgaz left a comment

Choose a reason for hiding this comment

fgaz Jul 8, 2020

Choose a reason for hiding this comment

ehmry Jul 9, 2020

Choose a reason for hiding this comment

aanderse commented Jul 9, 2020

ehmry commented Jul 12, 2020

aanderse commented Jul 12, 2020

ehmry commented Jul 14, 2020

aanderse commented Jul 15, 2020

ehmry commented Jul 15, 2020

aanderse commented Jul 16, 2020

sikmir commented Jul 16, 2020

aanderse left a comment

Choose a reason for hiding this comment

aanderse commented Jul 22, 2020

ehmry commented Jul 23, 2020

sikmir commented Jul 23, 2020

ehmry commented Jul 3, 2020 •

edited