[bug#55912] [PATCH] home: Add OpenSSH service.

[Ludovic Courtès]

Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> Ludovic Courtès schreef op za 11-06-2022 om 18:49 [+0200]:
>> +  (port
>> +   (maybe-integer 'disabled)
>> +   "TCP port number to connect to.")
>
> TCP only allows natural numbers up to some bound, and in practice
> implementations only support non-zero natural numbers, so maybe the
> predicate can be refined a bit?

We could do that, though that’s more code for little in return…

>> +                             (formatted-message
>> +                              (G_ "~s: unsupported address family")
>
> Maybe a hint:
>
>   hint: AF_INET and AF_INET6 are supported.

Sure, that makes sense.

>> +  (proxy-command
>> +   (maybe-string 'disabled)
>
> Attila Lendvai has a patch series at 54674 that changes 'disabled' ->
> *unspecified* -- I think it would be better to apply that patch series
> first.

I’ll take a look.

> Wouldn't the value need to be escaped?  Or at least a check that it
> doesn't contain special characters like \n or whatever special
> charaters an OpenSSH configuration has.

Oh right, it needs to be somewhat escaped; I’ll do that.  I think
‘object->string’ will be a good-enough escaping mechanism, and it’ll
take care of newlines.  (Doing things The Right Way would require
detailed knowledge about the grammar that OpenSSH’s parser expects.)

>
>>+ (define* (file-join name files #:optional (delimiter " "))
>>+  "Return a file in the store called @var{name} that is the
>>+ concatenation
>>+ of all the file-like objects listed in @var{files}, with
> @var{delimited}
>>+ inserted after each of them."
>
> Does this work for files with non-ASCII characters and for file names
> that contain non-ASCII characters?

‘files’ is a list of “file-like objects”, which, by definition, have
names acceptable for the stores (so ASCII names).

That’s not a user-visible limitation since store file names are hints
more than anything else.  You could have a local file, say
“courtès.pub”, and you’d do:

  (local-file "courtès.pub" "that-guy.pub")

This service doesn’t change that.

>>+   (description "Configure the OpenSSH @acronym{SSH, secure shell}
>>+client and _add it to the user profile_.")
>
> (emphasis added).  Why is it automagically added to the user profile? 
> This is considered bad practice for system services.  Maybe the user
> keeps all their remote communication things in a single profile, maybe
> the user only uses openssh things via other tools like 'guix deploy' or
> 'gnome-shell-extension-gsconnect' and hence has no need for 'openssh'
> in their home profile.   Maybe the user never ssh's _from_ the computer
> that has the openssh home configuration and only connects _to_ the
> computer and hence the 'openssh' in the profile isn't necessary.
>
> Now there are two ways to add 'openssh' to the environment: the Guix
> Home equivalent of a 'packages' field and the openssh home service,
> with AFAICT no mechanism for deciding which one ‘wins’ and no mechanism
> for a proper error message like ‘only add the openssh package to the
> profile or use the openssh home service, not both!’, which doesn't seem
> ideal to me.

All good points!  I’m usually against magically extending the profile
with new packages.  In this case, my reasoning was: if you’re going to
set up OpenSSH config files, that’s probably because you’re going to
need OpenSSH, so why not bring it while we’re at it?  (This rationale
usually doesn’t hold for system services: just because I run ntpd
doesn’t mean I need to have it in the system profile.)

But you’re right here, so I guess I’ll just remove it.

v2 coming soon!

Thanks,
Ludo’.