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

Proposed by Lucas Kanashiro
Status: Merged
Approved by: Lucas Kanashiro
Approved revision: 71090bca2d877bcb79c2193923f41dcb67c8ed2e
Merged at revision: 71090bca2d877bcb79c2193923f41dcb67c8ed2e
Proposed branch: ~lucaskanashiro/ubuntu/+source/strongswan:re-enable-eap-plugins
Merge into: ubuntu/+source/strongswan:ubuntu/devel
Diff against target: 107 lines (+34/-0)
5 files modified
debian/changelog (+15/-0)
debian/control (+3/-0)
debian/libcharon-extra-plugins.install (+6/-0)
debian/libcharon-extra-plugins.maintscript (+8/-0)
debian/rules (+2/-0)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  (community) Approve
Canonical Server Pending
Review via email: mp+384385@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

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
Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

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!

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

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

Up for review again.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

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
Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

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

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

$ 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index aa2a2b3..1315154 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,18 @@
6+strongswan (5.8.4-1ubuntu2) groovy; urgency=medium
7+
8+ * Re-enable eap-{dynamic,peap} libcharon plugins (LP: #1878887)
9+ - d/control: update libcharon-extra-plugins description.
10+ - d/libcharon-extra-plugins.install: install .so and conf files.
11+ - d/rules: add plugins to the configuration arguments.
12+ * Remove conf files of plugins removed from libcharon-extra-plugins
13+ - The conf file of the following plugins were removed: eap-aka-3gpp2,
14+ eap-sim-file, eap-sim-pcsc, eap-sim, eap-simaka-pseudonym,
15+ eap-simaka-reauth, eap-simaka-sql, xauth-noauth.
16+ - Created d/libcharon-extra-plugins.maintscript to handle the removals
17+ properly.
18+
19+ -- Lucas Kanashiro <kanashiro@ubuntu.com> Thu, 21 May 2020 14:53:05 -0300
20+
21 strongswan (5.8.4-1ubuntu1) groovy; urgency=medium
22
23 * Merge with Debian unstable. Remaining changes:
24diff --git a/debian/control b/debian/control
25index 5ee5ad5..fbd59a0 100644
26--- a/debian/control
27+++ b/debian/control
28@@ -204,6 +204,9 @@ Description: strongSwan charon library (extra plugins)
29 - unity (Cisco Unity extensions for IKEv1)
30 - xauth-eap (XAuth backend that uses EAP methods to verify passwords)
31 - xauth-pam (XAuth backend that uses PAM modules to verify passwords)
32+ - eap-dynamic (EAP proxy plugin that dynamically selects an EAP method
33+ requested/supported by the client (since 5.0.1))
34+ - eap-peap (EAP-PEAP protocol handler, wraps other EAP methods securely)
35
36 Package: strongswan-starter
37 Architecture: any
38diff --git a/debian/libcharon-extra-plugins.install b/debian/libcharon-extra-plugins.install
39index 7765f20..cc0bf6f 100644
40--- a/debian/libcharon-extra-plugins.install
41+++ b/debian/libcharon-extra-plugins.install
42@@ -2,9 +2,11 @@
43 usr/lib/ipsec/plugins/libstrongswan-addrblock.so
44 usr/lib/ipsec/plugins/libstrongswan-certexpire.so
45 usr/lib/ipsec/plugins/libstrongswan-eap-aka.so
46+usr/lib/ipsec/plugins/libstrongswan-eap-dynamic.so
47 usr/lib/ipsec/plugins/libstrongswan-eap-gtc.so
48 usr/lib/ipsec/plugins/libstrongswan-eap-identity.so
49 usr/lib/ipsec/plugins/libstrongswan-eap-md5.so
50+usr/lib/ipsec/plugins/libstrongswan-eap-peap.so
51 usr/lib/ipsec/plugins/libstrongswan-eap-radius.so
52 usr/lib/ipsec/plugins/libstrongswan-eap-tls.so
53 usr/lib/ipsec/plugins/libstrongswan-eap-tnc.so
54@@ -24,9 +26,11 @@ usr/lib/ipsec/plugins/libstrongswan-xauth-pam.so
55 usr/share/strongswan/templates/config/plugins/addrblock.conf
56 usr/share/strongswan/templates/config/plugins/certexpire.conf
57 usr/share/strongswan/templates/config/plugins/eap-aka.conf
58+usr/share/strongswan/templates/config/plugins/eap-dynamic.conf
59 usr/share/strongswan/templates/config/plugins/eap-gtc.conf
60 usr/share/strongswan/templates/config/plugins/eap-identity.conf
61 usr/share/strongswan/templates/config/plugins/eap-md5.conf
62+usr/share/strongswan/templates/config/plugins/eap-peap.conf
63 usr/share/strongswan/templates/config/plugins/eap-radius.conf
64 usr/share/strongswan/templates/config/plugins/eap-tls.conf
65 usr/share/strongswan/templates/config/plugins/eap-tnc.conf
66@@ -47,9 +51,11 @@ etc/strongswan.d/tnc.conf
67 etc/strongswan.d/charon/addrblock.conf
68 etc/strongswan.d/charon/certexpire.conf
69 etc/strongswan.d/charon/eap-aka.conf
70+etc/strongswan.d/charon/eap-dynamic.conf
71 etc/strongswan.d/charon/eap-gtc.conf
72 etc/strongswan.d/charon/eap-identity.conf
73 etc/strongswan.d/charon/eap-md5.conf
74+etc/strongswan.d/charon/eap-peap.conf
75 etc/strongswan.d/charon/eap-radius.conf
76 etc/strongswan.d/charon/eap-tls.conf
77 etc/strongswan.d/charon/eap-tnc.conf
78diff --git a/debian/libcharon-extra-plugins.maintscript b/debian/libcharon-extra-plugins.maintscript
79new file mode 100644
80index 0000000..f6e7a3a
81--- /dev/null
82+++ b/debian/libcharon-extra-plugins.maintscript
83@@ -0,0 +1,8 @@
84+rm_conffile /etc/strongswan.d/charon/eap-aka-3gpp2.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
85+rm_conffile /etc/strongswan.d/charon/eap-sim-file.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
86+rm_conffile /etc/strongswan.d/charon/eap-sim-pcsc.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
87+rm_conffile /etc/strongswan.d/charon/eap-sim.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
88+rm_conffile /etc/strongswan.d/charon/eap-simaka-pseudonym.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
89+rm_conffile /etc/strongswan.d/charon/eap-simaka-reauth.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
90+rm_conffile /etc/strongswan.d/charon/eap-simaka-sql.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
91+rm_conffile /etc/strongswan.d/charon/xauth-noauth.conf 5.8.4-1ubuntu2~ libcharon-extra-plugins
92diff --git a/debian/rules b/debian/rules
93index 8f2d740..25cbede 100755
94--- a/debian/rules
95+++ b/debian/rules
96@@ -15,9 +15,11 @@ CONFIGUREARGS := --libdir=/usr/lib --libexecdir=/usr/lib \
97 --enable-curl \
98 --enable-eap-aka \
99 --enable-eap-gtc \
100+ --enable-eap-dynamic \
101 --enable-eap-identity \
102 --enable-eap-md5 \
103 --enable-eap-mschapv2 \
104+ --enable-eap-peap \
105 --enable-eap-radius \
106 --enable-eap-tls \
107 --enable-eap-tnc \

Subscribers

People subscribed via source and target branches