Project

General

Profile

Bug #2582

Typo patch prevents test-suites to be built

Added by CJ Tjhai over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Category:
testing
Target version:
Start date:
Due date:
Estimated time:
Affected version:
5.6.2
Resolution:
Fixed

Description

Hi,

I have been playing around with StrongSwan test-suites from a clean machine and I came across an issue.

In commit 2db6d5b8b378a1f460a073b71e1c178bc997861b of StrongSwan, there is a fix to a typo in freeradius (testing/scripts/recipes/patches/freeradius-tnc-fhh), see https://github.com/strongswan/strongswan/commit/2db6d5b8b378a1f460a073b71e1c178bc997861b#diff-ac54cdbd5dd654d18cdcb9c631a15dd8. The original line reads:

- * EAP - MD5 doesnot specify code, id & length but chap specifies them

and the patch corrected the typo to 'does not'. Because the typo is in original freeradius-2.2.8 source code, the patch in the commit will fail and therefore, I cannot build the test images.

There is also another issue, which does not prevent the test images to be built, but still causes the patch to fail. From https://download.strongswan.org/uml/ha-4.14-abicompat.patch.bz2, we have:

@@ -819,6 +1075,11 @@ static void clusterip_net_exit(struct net *net)
        cn->procdir = NULL;
 #endif
        nf_unregister_net_hook(net, &cip_arp_ops);
+#ifdef CONFIG_XFRM
+       nf_unregister_net_hook(net, &cip_pre_routing_ops);
+       nf_unregister_net_hook(net, &cip_xfrm_in_ops);
+       nf_unregister_net_hook(net, &cip_xfrm_out_ops);
+#endif /* CONFIG_XFRM */
 }

 static struct pernet_operations clusterip_net_ops = {

But looking at the kernel 4.15 source code at net/ipv4/netfilter/ipt_CLUSTERIP.c, there is a missing line which causes the above patch to fail. I believe the patch is trying to patch the following lines in ipt_CLUSTERIP.c:
static void clusterip_net_exit(struct net *net)
{
    struct clusterip_net *cn = net_generic(net, clusterip_net_id);
#ifdef CONFIG_PROC_FS
    proc_remove(cn->procdir);
    cn->procdir = NULL;
#endif
    nf_unregister_net_hook(net, &cip_arp_ops);
    WARN_ON_ONCE(!list_empty(&cn->configs));
}

Cheers,
CJ

carol.daemon.log (30.5 KB) carol.daemon.log Daemon log of carol CJ Tjhai, 19.03.2018 19:28
console.log (35.7 KB) console.log Console log CJ Tjhai, 19.03.2018 19:28
dave.daemon.log (30.2 KB) dave.daemon.log Daemon log of dave CJ Tjhai, 19.03.2018 19:28
carol.auth.log (18 KB) carol.auth.log Carol's auth log CJ Tjhai, 20.03.2018 13:36
console.log (22.7 KB) console.log Console log CJ Tjhai, 20.03.2018 13:36
moon.auth.log (166 Bytes) moon.auth.log Moon's auth log CJ Tjhai, 20.03.2018 13:36
alice.daemon.log (2.28 MB) alice.daemon.log Alice's daemon log CJ Tjhai, 20.03.2018 13:36

Associated revisions

Revision 0f785f6b (diff)
Added by Tobias Brunner over 2 years ago

testing: Revert typo fix in FreeRADIUS patch

Fixes: 2db6d5b8b378 ("Fixed some typos, courtesy of codespell")
Fixes #2582.

Revision d3b9db68 (diff)
Added by Tobias Brunner over 2 years ago

libimcv: Add Debian 8.10 to IMV database

References #2582.

History

#1 Updated by Tobias Brunner over 2 years ago

  • Category set to testing
  • Status changed from New to Feedback

In commit 2db6d5b8b378a1f460a073b71e1c178bc997861b of StrongSwan, there is a fix to a typo in freeradius (testing/scripts/recipes/patches/freeradius-tnc-fhh), see https://github.com/strongswan/strongswan/commit/2db6d5b8b378a1f460a073b71e1c178bc997861b#diff-ac54cdbd5dd654d18cdcb9c631a15dd8. The original line reads:
[...]
and the patch corrected the typo to 'does not'. Because the typo is in original freeradius-2.2.8 source code, the patch in the commit will fail and therefore, I cannot build the test images.

Oh, shoot! That was definitely not intended. Fixed in master.

There is also another issue, which does not prevent the test images to be built, but still causes the patch to fail. From https://download.strongswan.org/uml/ha-4.14-abicompat.patch.bz2, we have:
[...]
But looking at the kernel 4.15 source code at net/ipv4/netfilter/ipt_CLUSTERIP.c, there is a missing line which causes the above patch to fail. I believe the patch is trying to patch the following lines in ipt_CLUSTERIP.c:

Well, the patch was made for 4.14, so it's entirely possible that it doesn't fully work with 4.15. However, that part of the patch is not strictly necessary as it is cleanup code. But I'll try to update the patch to 4.15 ASAP.

#2 Updated by Tobias Brunner over 2 years ago

I've updated the patch, but noticed that since this commit (was added with 4.15.6) the ha/active-passive scenario doesn't work anymore because the ClusterIP firewall rules can't be installed anymore with --local-node 0.

#3 Updated by CJ Tjhai over 2 years ago

Hi Tobias,

Thanks for your prompt response. Is the plan then to stick to kernel 4.14 for testing?

Cheers,
CJ

#4 Updated by Tobias Brunner over 2 years ago

I've uploaded another patch that is compatible with 4.15.6+.

#5 Updated by CJ Tjhai over 2 years ago

Hi Tobias,

Not sure I should start a new thread, but let me know if it has to be.

Reading on the description of both ha test suites, I believe the description may be incorrect:
  • The description says that alice and moon as the real gateways, perhaps it should say alice and venus instead? Or perhaps the diagram is incorrect?
  • Is it coincidence that there is no configuration/log files for venus?

Cheers,
CJ

#6 Updated by Tobias Brunner over 2 years ago

Reading on the description of both ha test suites, I believe the description may be incorrect:
  • The description says that alice and moon as the real gateways, perhaps it should say alice and venus instead? Or perhaps the diagram is incorrect?

See #2465.

  • Is it coincidence that there is no configuration/log files for venus?

There is no configuration or log. It is only used as ICMP echo target.

#7 Updated by CJ Tjhai over 2 years ago

Hi Tobias,

Many thanks for the link to #2465.

I can now confirmed that there is now no issue in building the test suites. :-) Well done.

However, I had a number tests that fail :-( and they were all related tncc:
  • [FAIL] 384 tnc/tnccs-11-radius-pts: pre..test..post
  • [FAIL] 389 tnc/tnccs-20-ev-pt-tls: pre..test..post
  • [FAIL] 397 tnc/tnccs-20-nea-pt-tls: pre..test..post
  • [FAIL] 398 tnc/tnccs-20-os: pre..test..post
  • [FAIL] 399 tnc/tnccs-20-os-pts: pre..test..post
  • [FAIL] 400 tnc/tnccs-20-pdp-eap: pre..test..post
  • [FAIL] 401 tnc/tnccs-20-pdp-pt-tls: pre..test..post
  • [FAIL] 403 tnc/tnccs-20-pts-no-ecc: pre..test..post

I could not figure out why they failed though. As an example, test 384 (tnccs-11-radius-pts) failed, but the other two tncc-11-radius tests (382 and 383) passed. Attached are the console.log, and the daemon logs of both dave and carol. I would be grateful if you could give me a pointer on why they failed.

Is there any restrictions on the hardware where the tests should be run on. I ran them on a MacBook Pro running Ubuntu 16.04.

Many thanks,
CJ

#8 Updated by Tobias Brunner over 2 years ago

I could not figure out why they failed though. As an example, test 384 (tnccs-11-radius-pts) failed, but the other two tncc-11-radius tests (382 and 383) passed. Attached are the console.log, and the daemon logs of both dave and carol. I would be grateful if you could give me a pointer on why they failed.

Probably because Debian 8.10 is currently unknown and the clients are therefore not added to the correct groups. Should be fixed with d3b9db688a.

#9 Updated by CJ Tjhai over 2 years ago

Hi Tobias,

Many thanks for fixing that and after re-running the test suites from scratch, I had only one remaining failed test:

  • [FAIL] 389 tnc/tnccs-20-ev-pt-tls: pre..test..post

Please find attached the related log files. Strangely though, when I re-start the test from scratch (including rebuilding), it passed but a different test failed (more on this below). Is this normal?

On the test that failed on the second run, it failed on the following lines of console.log:

~~~~~~~ FAIL ~~~~~~~
moon# ipsec status 2> /dev/null | grep 'rw\[4]: ESTABLISHED.*moon.strongswan.org.*dave@strongswan.org' [YES]
~~~~~~~~~~~~~~~~~~~~

and when I inspected the content of moon.statusall, it made sense why it failed, see below.
          rw{2}:   10.1.0.0/16 === 192.168.0.200/32
          rw[2]: ESTABLISHED 2 seconds ago, 192.168.0.1[moon.strongswan.org]...192.168.0.100[carol@strongswan.org]
          rw[2]: IKEv2 SPIs: 8a6514fb34472556_i 54d4fab8bb4dfe9c_r*, public key reauthentication in 54 minutes

Perhaps this is due to timing issue?

Best regards,
CJ

#10 Updated by Tobias Brunner over 2 years ago

Please find attached the related log files. Strangely though, when I re-start the test from scratch (including rebuilding), it passed but a different test failed (more on this below). Is this normal?

Not sure about it being normal. But occasionally tests fail for some reason or other (timing, some other test not cleaning up properly, external dependencies). If you build from scratch, the TNC tests are especially prone to fail due to some external dependencies (version numbers of packages, DB updates etc. as seen above with the new Debian version). I think Andreas usually fixes those when he runs the test suite (not sure if he typically builds the complete environment from scratch, though, I rarely do that).

Perhaps this is due to timing issue?

Could be.

#11 Updated by CJ Tjhai over 2 years ago

Hi Tobias,

Thanks for your prompt response.

I have no more question and I'm happy for this issue to be closed.

Cheers,
CJ

#12 Updated by Tobias Brunner over 2 years ago

  • Tracker changed from Issue to Bug
  • Status changed from Feedback to Closed
  • Assignee set to Tobias Brunner
  • Target version set to 5.6.3
  • Resolution set to Fixed

Also available in: Atom PDF