Merge ~lucaskanashiro/ubuntu/+source/strongswan:re-enable-eap-plugins into ubuntu/+source/strongswan:ubuntu/devel

[Lucas Kanashiro]

The proposed changes came from this bug:

https://bugs.launchpad.net/ubuntu/+source/strongswan/+bug/1878887

They are:

- Re-enable the eap-dynamic and eap-peap libcharon plugins
- Remove some old conf files of some plugins removed from libcharon-extra-plugins in version 5.8.0-2

I submitted a MR to Debian to also re-enable those plugins there:

https://salsa.debian.org/debian/strongswan/-/merge_requests/9

Here is a PPA with the proposed package:

https://launchpad.net/~lucaskanashiro/+archive/ubuntu/groovy-strongswan-lp1878887/+packages

autopkgtest is still happy:

autopkgtest [17:10:53]: @@@@@@@@@@@@@@@@@@@@ summary
admin-strongswan-charon PASS
admin-strongswan-starter PASS
daemon PASS
plugins PASS

[Christian Ehrhardt  (paelzer)]

Re-Addition of plugin - yes
Thanks for adding the commit with some details for the reasoning as we discussed it - that will help down the road even if Yves won't take it into Debian - at least we will remember the reasoning this time.

For the conffile handling I'm glad this was spotted and will now be fixed, but I'm not 100% +1 on the way to do it. I've myself learned the hard way that coding that up reads nicely but then next year someone spots there needs to be an action on e.g. abort-install and so on.
Example of such issues - this MP misses libcharon-extra-plugins.postrm :-)

Instead of the <pkg>.postinst/prerm what usually works much much better is dropping d/<pgk>.maintscript with lines like
rm_conffile /etc/strongswan.d/charon/xauth-noauth.conf 5.8.4-1ubuntu2~

I'd have other complains at the scripts, but since I'm suggesting to replace them anyway I don't need to go into that.

What do you think, any reason not to do this with the more reliable (just one place not getting out of sync, auto-covers all places needed, ...) .maintscript approach?

review: Needs Fixing

[Lucas Kanashiro (lucaskanashiro)]

Thanks for the review Christian!

About the way I am handling conf files removals I tried to follow what is described here:

https://wiki.debian.org/DpkgConffileHandling

For instance, it does not mention the need for the postrm script. But I was also thinking about a way to reduce the duplicated code (that's always a bad thing). I searched for your solution in other packages and it is indeed the way some packages handle conf files removal, more elegant btw :) Thanks for the suggestion!

[Lucas Kanashiro (lucaskanashiro)]

Removal of conf files was fixed. I uploaded a new version to the PPA fwiw.

Up for review again.

[Christian Ehrhardt  (paelzer)]

LGTM now if the testing of the conffile removal works I'm fine - thanks for reworking this for me.

There is one thing I wondered, which is the upgrade from a fixed Focal (that already has removed the conffiles) to a fixed groovy (that wants to remove them again).
IIRC the debhelper bits already handle that case gracefully - but if you could before upload give this an explicit try?

review: Approve

[Lucas Kanashiro (lucaskanashiro)]

I did the test upgrading from fixed Focal to fixed Groovy, and it worked fine, no failure. I am going to upload it.

[Lucas Kanashiro (lucaskanashiro)]

$ git push pkg upload/5.8.4-1ubuntu2
Enumerating objects: 21, done.
Counting objects: 100% (21/21), done.
Delta compression using up to 8 threads
Compressing objects: 100% (15/15), done.
Writing objects: 100% (15/15), 3.15 KiB | 1.05 MiB/s, done.
Total 15 (delta 10), reused 0 (delta 0)
remote: Checking connectivity: 15, done.
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/strongswan
 * [new tag] upload/5.8.4-1ubuntu2 -> upload/5.8.4-1ubuntu2

$ dput ubuntu ../strongswan_5.8.4-1ubuntu2_source.changes
Checking signature on .changes
gpg: ../strongswan_5.8.4-1ubuntu2_source.changes: Valid signature from F823A2729883C97C
Checking signature on .dsc
gpg: ../strongswan_5.8.4-1ubuntu2.dsc: Valid signature from F823A2729883C97C
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading strongswan_5.8.4-1ubuntu2.dsc: done.
  Uploading strongswan_5.8.4-1ubuntu2.debian.tar.xz: done.
  Uploading strongswan_5.8.4-1ubuntu2_source.buildinfo: done.
  Uploading strongswan_5.8.4-1ubuntu2_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index aa2a2b3..1315154 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,18 @@
+strongswan (5.8.4-1ubuntu2) groovy; urgency=medium
+
+  * Re-enable eap-{dynamic,peap} libcharon plugins (LP: #1878887)
+    - d/control: update libcharon-extra-plugins description.
+    - d/libcharon-extra-plugins.install: install .so and conf files.
+    - d/rules: add plugins to the configuration arguments.
+  * Remove conf files of plugins removed from libcharon-extra-plugins
+    - The conf file of the following plugins were removed: eap-aka-3gpp2,
+      eap-sim-file, eap-sim-pcsc, eap-sim, eap-simaka-pseudonym,
+      eap-simaka-reauth, eap-simaka-sql, xauth-noauth.
+    - Created d/libcharon-extra-plugins.maintscript to handle the removals
+      properly.
+
+ -- Lucas Kanashiro <[e-mail]>  Thu, 21 May 2020 14:53:05 -0300
+
 strongswan (5.8.4-1ubuntu1) groovy; urgency=medium
 
   * Merge with Debian unstable. Remaining changes:
diff --git a/debian/control b/debian/control
index 5ee5ad5..fbd59a0 100644
--- a/debian/control
+++ b/debian/control
@@ -204,6 +204,9 @@ Description: strongSwan charon library (extra plugins)
   - unity (Cisco Unity extensions for IKEv1)
   - xauth-eap (XAuth backend that uses EAP methods to verify passwords)
   - xauth-pam (XAuth backend that uses PAM modules to verify passwords)
+  - eap-dynamic (EAP proxy plugin that dynamically selects an EAP method
+    requested/supported by the client (since 5.0.1))
+  - eap-peap (EAP-PEAP protocol handler, wraps other EAP methods securely)
 
 Package: strongswan-starter
 Architecture: any
diff --git a/debian/libcharon-extra-plugins.install b/debian/libcharon-extra-plugins.install
index 7765f20..cc0bf6f 100644
--- a/debian/libcharon-extra-plugins.install
+++ b/debian/libcharon-extra-plugins.install
@@ -2,9 +2,11 @@
 usr/lib/ipsec/plugins/libstrongswan-addrblock.so
 usr/lib/ipsec/plugins/libstrongswan-certexpire.so
 usr/lib/ipsec/plugins/libstrongswan-eap-aka.so
+usr/lib/ipsec/plugins/libstrongswan-eap-dynamic.so
 usr/lib/ipsec/plugins/libstrongswan-eap-gtc.so
 usr/lib/ipsec/plugins/libstrongswan-eap-identity.so
 usr/lib/ipsec/plugins/libstrongswan-eap-md5.so
+usr/lib/ipsec/plugins/libstrongswan-eap-peap.so
 usr/lib/ipsec/plugins/libstrongswan-eap-radius.so
 usr/lib/ipsec/plugins/libstrongswan-eap-tls.so
 usr/lib/ipsec/plugins/libstrongswan-eap-tnc.so
@@ -24,9 +26,11 @@ usr/lib/ipsec/plugins/libstrongswan-xauth-pam.so
 usr/share/strongswan/templates/config/plugins/addrblock.conf
 usr/share/strongswan/templates/config/plugins/certexpire.conf
 usr/share/strongswan/templates/config/plugins/eap-aka.conf
+usr/share/strongswan/templates/config/plugins/eap-dynamic.conf
 usr/share/strongswan/templates/config/plugins/eap-gtc.conf
 usr/share/strongswan/templates/config/plugins/eap-identity.conf
 usr/share/strongswan/templates/config/plugins/eap-md5.conf
+usr/share/strongswan/templates/config/plugins/eap-peap.conf
 usr/share/strongswan/templates/config/plugins/eap-radius.conf
 usr/share/strongswan/templates/config/plugins/eap-tls.conf
 usr/share/strongswan/templates/config/plugins/eap-tnc.conf
@@ -47,9 +51,11 @@ etc/strongswan.d/tnc.conf
 etc/strongswan.d/charon/addrblock.conf
 etc/strongswan.d/charon/certexpire.conf
 etc/strongswan.d/charon/eap-aka.conf
+etc/strongswan.d/charon/eap-dynamic.conf
 etc/strongswan.d/charon/eap-gtc.conf
 etc/strongswan.d/charon/eap-identity.conf
 etc/strongswan.d/charon/eap-md5.conf
+etc/strongswan.d/charon/eap-peap.conf
 etc/strongswan.d/charon/eap-radius.conf
 etc/strongswan.d/charon/eap-tls.conf
 etc/strongswan.d/charon/eap-tnc.conf
diff --git a/debian/libcharon-extra-plugins.maintscript b/debian/libcharon-extra-plugins.maintscript
 new file mode 100644
index 0000000..f6e7a3a
--- /dev/null
+++ b/debian/libcharon-extra-plugins.maintscript
@@ -0,0 +1,8 @@
+rm_conffile /etc/strongswan.d/charon/eap-aka-3gpp2.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
+rm_conffile /etc/strongswan.d/charon/eap-sim-file.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
+rm_conffile /etc/strongswan.d/charon/eap-sim-pcsc.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
+rm_conffile /etc/strongswan.d/charon/eap-sim.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
+rm_conffile /etc/strongswan.d/charon/eap-simaka-pseudonym.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
+rm_conffile /etc/strongswan.d/charon/eap-simaka-reauth.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
+rm_conffile /etc/strongswan.d/charon/eap-simaka-sql.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
+rm_conffile /etc/strongswan.d/charon/xauth-noauth.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
diff --git a/debian/rules b/debian/rules
index 8f2d740..25cbede 100755
--- a/debian/rules
+++ b/debian/rules
@@ -15,9 +15,11 @@ CONFIGUREARGS := --libdir=/usr/lib --libexecdir=/usr/lib \
 		--enable-curl \
 		--enable-eap-aka \
 		--enable-eap-gtc \
+		--enable-eap-dynamic \
 		--enable-eap-identity \
 		--enable-eap-md5 \
 		--enable-eap-mschapv2 \
+		--enable-eap-peap \
 		--enable-eap-radius \
 		--enable-eap-tls \
 		--enable-eap-tnc \