Bug 234648 - security/strongswan: start/stop/reload modern vici-based configurations
Summary: security/strongswan: start/stop/reload modern vici-based configurations
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Kurt Jaeger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-06 00:59 UTC by Jose Luis Duran
Modified: 2019-04-22 05:08 UTC (History)
3 users (show)

See Also:
strongswan: maintainer-feedback+


Attachments
Ugly patch that loads swanctl.conf (811 bytes, patch)
2019-01-06 00:59 UTC, Jose Luis Duran
no flags Details | Diff
Patch set #2 for security/strongswan (2.64 KB, patch)
2019-01-09 07:58 UTC, Sam Chen
no flags Details | Diff
strongswan service test output for patch #2 (4.10 KB, text/plain)
2019-01-09 08:07 UTC, Sam Chen
no flags Details
Use a separate rc.d script (535 bytes, text/plain)
2019-01-12 00:05 UTC, Jose Luis Duran
no flags Details
Use a separate rc.d script (721 bytes, text/plain)
2019-01-15 20:34 UTC, Jose Luis Duran
no flags Details
Patch set type #2 for security/strongswan (rev 1) (4.11 KB, patch)
2019-01-17 13:53 UTC, Sam Chen
no flags Details | Diff
strongswan w/ strongswan_swanctl test output for patch set type #2 (2.78 KB, text/plain)
2019-01-17 13:56 UTC, Sam Chen
no flags Details
Updated startup script (2.84 KB, patch)
2019-03-01 11:29 UTC, strongswan
strongswan: maintainer-approval-
Details | Diff
Updated startup script (2.96 KB, patch)
2019-03-05 09:22 UTC, strongswan
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jose Luis Duran 2019-01-06 00:59:33 UTC
Created attachment 200820 [details]
Ugly patch that loads swanctl.conf

Migrating from a legacy, stroke-based configuration to a modern, vici-based (from ipsec.conf to swanctl.conf) I realized that the current rc script does not load the swanctl.conf file, instead one has to manually invoke `swanctl --load-all` to start it.

I am attaching a very ugly patch to the strongswan.in file, however I am confident a more elegant solution will be implemented.

Thanks.
Comment 1 Sam Chen 2019-01-09 07:57:06 UTC
Like Jose, I too recently noticed that rc.d/strongswan had to be modified to load swanctl.conf, and made local changes to the service script.  My version of the ugly patch and a test output is attached.  It mostly passes rclint(8), except the "one-line functions discouraged" errors that I haven't found a good solution.  Please check the code carefully, since I'm new to FreeBSD ports.  Thanks.
Comment 2 Sam Chen 2019-01-09 07:58:24 UTC
Created attachment 200947 [details]
Patch set #2 for security/strongswan
Comment 3 Sam Chen 2019-01-09 08:07:14 UTC
Created attachment 200948 [details]
strongswan service test output for patch #2
Comment 4 Jose Luis Duran 2019-01-12 00:05:19 UTC
Created attachment 201047 [details]
Use a separate rc.d script

I guess it is better to use a separate rc.d script (like upstream is doing it with systemd). The script is still ugly, but the idea is to load charon and then load the configuration using swanctl, avoiding the use of starter.
Comment 5 Jose Luis Duran 2019-01-15 20:34:56 UTC
Created attachment 201171 [details]
Use a separate rc.d script

This version uses daemon(8) to handle charon.
Comment 6 strongswan 2019-01-17 11:37:55 UTC
(In reply to Jose Luis Duran from comment #5)
Thanks Jose.
I will have a look at the script.
Comment 7 Sam Chen 2019-01-17 13:51:47 UTC
Nice work, Jose.  I agree it's a step forward to manage charon under the BSD rc.d framework.  Let me remove my hacked script from Attachments.

Now I think backwards compatibility is important for ipsec config migration.  I've expanded on your earlier rc.d script and added support for enabling both rc.d/strongswan and rc.d/strongswan_swanctl simultaneously.  And added code to extra_commands for "reload statusall".  rc.d/strongswan will start BEFORE (rclist(8)) rc.d/strongswan_swanctl for reason noted in the code--also changed the former to pass rclint.

One code digression is mine removes the command_args "-r" to daemon(8).  Upstream's systemd strongswan-swanctl does not auto-restart charon, nor do almost all BSD ports that use daemon(8).  There could be an issue where ipsec starter.c's 5 sec auto-restart of charon affects BSD daemon(8)'s 1 sec auto-restart interval.

Also between charon invocation and swanctl run I introduced an up-to 5 sec wait loop for charon.pid file.  A fixed 1 sec wait could be just on the edge for that overloaded cloud VM.

Please find the revised "Patch set #2" and test output, attached.  Thanks.
Comment 8 Sam Chen 2019-01-17 13:53:48 UTC
Created attachment 201208 [details]
Patch set type #2 for security/strongswan (rev 1)
Comment 9 Sam Chen 2019-01-17 13:56:40 UTC
Created attachment 201209 [details]
strongswan w/ strongswan_swanctl test output for patch set type #2
Comment 10 Jose Luis Duran 2019-01-17 18:26:44 UTC
(In reply to Sam Chen from comment #7)

Thank you Sam! I agree with most of your suggestions.  I think the maintainer will move this conversation over to Phabricator and we'll take it from there.

Thank you all!
Comment 11 strongswan 2019-02-26 19:54:29 UTC
I created a combined script using the suggestions provided along with the existing script which allows to configure which interface to use (vici or stroke).

Have a look at 
https://reviews.freebsd.org/D19367
Comment 12 Jose Luis Duran 2019-02-26 19:58:29 UTC
(In reply to strongswan from comment #11)

Thank you! Looks good. I'll give it a try.
Comment 13 strongswan 2019-03-01 11:29:01 UTC
Created attachment 202474 [details]
Updated startup script

Updated startup script according to review D19367.
Comment 14 strongswan 2019-03-01 11:31:58 UTC
The latest patch
Can be committed along with Bug 235340
Comment 15 Sam Chen 2019-03-01 17:51:10 UTC
(In reply to strongswan from comment #11)

Thank you.  Let me give it try as well.
Comment 16 Sam Chen 2019-03-04 21:50:55 UTC
Confirmed the updated start script, with review D19367 patch, successfully loads the chosen ipsec.conf or swanctl.conf on system boot.

Thanks for your work.
Comment 17 strongswan 2019-03-05 09:21:49 UTC
Comment on attachment 202474 [details]
Updated startup script

Superseeded by latest updated
Comment 18 strongswan 2019-03-05 09:22:37 UTC
Created attachment 202573 [details]
Updated startup script

Updated the patch with the default case.
Comment 19 Jose Luis Duran 2019-03-08 05:28:56 UTC
(In reply to strongswan from comment #18)
There seems to be a typo in the warning message.
Comment 20 Kurt Jaeger freebsd_committer 2019-03-09 10:37:17 UTC
Committed, thanks!
Comment 21 commit-hook freebsd_committer 2019-03-09 10:37:28 UTC
A commit references this bug:

Author: pi
Date: Sat Mar  9 10:37:14 UTC 2019
New revision: 495117
URL: https://svnweb.freebsd.org/changeset/ports/495117

Log:
  security/strongswan: add vici-based configuration for the rc script

  The rc script is modified to allow both a legacy (ipsec.conf-based)
  startup or a new (swanctl.conf-based) config. Default is the legacy.

  The new setup is based on vici, the Versatile IKE Configuration Interface.

  For more details, see:

  https://wiki.strongswan.org/projects/strongswan/wiki/Vici

  PR:		234648
  Submitted by:	Jose Luis Duran <jlduran@gmail.com>
  Reviewed by:	Sam Chen <sc.gear@one.caeon.com>
  Approved by:	strongswan@Nanoteq.com (maintainer)
  Differential Revision:	D19367

Changes:
  head/security/strongswan/Makefile
  head/security/strongswan/files/strongswan.in