sshd: add custom options #41792
sshd: add custom options #41792
Conversation
Izorkin
force-pushed
the
sshd-custom
branch
2 times, most recently
from
302dbc6
to
02b644f
Compare
| @@ -290,13 +290,71 @@ in | |||
| type = types.bool; | |||
| default = false; | |||
| description = '' | |||
| Specifies whether sshd(8) should look up the remote host name, and to check that the resolved host name for | |||
| Specifies whether sshd should look up the remote host name, and to check that the resolved host name for | |||
There was a problem hiding this comment.
That (8) is desirable there and can just stay there.
| the remote IP address maps back to the very same IP address. | ||
| If this option is set to no (the default) then only addresses and not host names may be used in | ||
| ~/.ssh/authorized_keys from and sshd_config Match Host directives. | ||
| ''; | ||
| }; | ||
|
|
||
| useTCPKeepAlive = mkOption { |
There was a problem hiding this comment.
I think this is problematic from a maintenance point of view. I'd expect this to be auto-generated from some sources, which you don't seem to provide, which means that in two years it will be out of date and broken for someone.
There was a problem hiding this comment.
Me use official documentation - https://man.openbsd.org/sshd_config#TCPKeepAlive Renamed to tcpKeepAlive
There was a problem hiding this comment.
The concern is that you did a manual copy/paste of documentation, which means that if the source ever updates their data, that our module is out of date (and won't be updated unless someone does manual work).
Renaming it doesn't address my raised concern.
You might be lucky and it will still be merged regardless, but I'd count it as an increase in technical debt for the project.
| '' else '' | ||
| X11Forwarding no | ||
| ''} | ||
| ${if cfg.forwardX11 |
There was a problem hiding this comment.
You are now just deleting newlines, which might mean the subsequent configuration is concatenated resulting in invalid configuration (didn't check, but that's why I believe it was done like that).
There was a problem hiding this comment.
Me tested to local VM - the configuration is generated normally
There was a problem hiding this comment.
OK, thanks for the confirmation.
|
#41798 =/
|
|
NixOS options are only added when they add value, not when they just shadow the options provided by the tool. Providing all options is just code bloat and we will always be behind upstream. |
Motivation for this change
Add custom options - TCPKeepAlive, clientAliveCountMax and clientAliveInterval.
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)