Subject: Re: [bug#48803] [PATCH] strongswan: provide a service definition and configuration interface.

From: Domagoj Stolfa <[email-masked]>
Date: Sun, 13 Jun 2021 14:04:00 +0100

Tobias,
> > and we do not put the config
> > file in a Guile string (to avoid indentation issues).
> 
> Not using a string is fine by me, but I don't understand this particular
> argument for it.

ipsec.conf is pythonistic in the sense that it's sensitive to
indentation. This is just to avoid copy-paste errors in a config file
that results in cryptic error messages because the user missed a space
while copy-pasting something. It's easier to just transmit the file as
given by the "higher-ups" out of bounds than have it as a configuration
string, as ipsec.secrets kind of has to be transmitted that way anyway.

> What does the daemon do now when USE-IPSEC? is #f?  Anything useful?

It doesn't do anything other than use the default configuration that is
provided by strongswan as it is shipped (basically, whatever is in the
build directory by default). This is what it has done up until this
point already, the user would have start strongswan by setting an
environment variable to some local `strongswan.conf`. It is also what
strongswan does on a fresh installation in any other distribution I've
tried it on.

> Could we drop USE-IPSEC? and allow IPSEC-CONF/IPSEC-SECRETS to be #f to
> signal the same thing (enforcing only sane combinations)? Or would that make
> things more confusing?

We could, the plan I had for `strongswan` as a service is to support
both ipsec.conf/ipsec.secrets and swanctl, hence the `use-ipsec?` as a
separate thing. I can refactor it without that flag and have no real
strong opinion on it.

> Is all this legacy enough to mark as such in the field name
> (LEGACY-IPSEC-CONF, etc.) or is it one of those things that will never ever
> go away and VPN providers will still hand out ipsecs.conf in 2038?

Unclear at this point, I don't see how strongswan could drop support for
ipsec.conf and ipsec.secrets without making a lot of users angry at this
point. The VPN that I'm using is configured and documented by people who
are quite familiar with strongswan, and even there the documentation is
referring to ipsec.conf and ipsec.secrets rather than swanctl at the
moment.

> Nitpick: ;;-comments are full sentences ending in a full stop.

ACK. Will fix.

> …you had to choose between two ifs and two #$strongswan-dirs, and chose two
> #$strongswan-dirs?  I prefer two ifs.

I think the reasoning for this was that if we're not using
ipsec.conf/ipsec.secrets, we would be writing swanctl-specific
configuration. Right now, that is just including strongswan.d, but it
might do other things, so I've kept it in a more traditional if-else
format.

> I guess.  I have no idea how ‘generic’ StrongSwan is and whether this makes
> more sense than (provision '(ipsec)) or not.

That's a good question. I think it could probably provision ipsec, but I
haven't really verified it so I didn't want to risk doing that. I assume
that it can, though.

> "StrongSwan's charon IKE keying daemon for IPsec VPN."

ACK.

> For this to be merged, we're still missing some documentation in
> doc/guix.text.  Would you be willing to write some?

Will include the docs with the next patch.

Thanks for the detailed feedback!

-- 
Domagoj