Project

General

Profile

Bug #3210

Missing SADB_X_EXT_ in kernel_pfkey plugin for FreeBSD

Added by Jean-François Hren about 1 month ago. Updated 18 days ago.

Status:
Closed
Priority:
Normal
Category:
kernel-interface
Target version:
Start date:
Due date:
Estimated time:
Affected version:
5.8.1
Resolution:
Fixed

Description

Hello,

There is some SADB_X_EXT_ missing in the kernel_pfkey plugin when under FreeBSD. It's not really an issue since everything is working correctly.
However when the verbose is enabled and we receive a SADB_X_EXT_SA_REPLAY extension, a crash can occur.
We patched kernel_pfkey_kernel.c with the missing extensions and it fixed our issue.

Thank you

patch-fix-verbose-pfkey (1.41 KB) patch-fix-verbose-pfkey Jean-François Hren, 16.10.2019 16:36

Associated revisions

Revision a0a03c25
Added by Tobias Brunner 18 days ago

Merge branch 'enum-strings'

Adds a compile check the number of enum strings and updates several of
these lists, in particular, the one in the pfkey-kernel plugin, where
strings for several new extensions on FreeBSD were missing.

Fixes #3210.

History

#1 Updated by Tobias Brunner about 1 month ago

  • Tracker changed from Issue to Bug
  • Status changed from New to Feedback
  • Target version set to 5.8.2

The dependency on constants and structs defined in a platform-specific header file is definitely not ideal. However, we don't use most of the structs (and those we do are portable or inside ifdefs), so this is mainly an issue with the strings for the debug messages. The crash comes from the fact that SADB_EXT_MAX is used in the ENUM() call, which might be larger than the last constant we actually have a string for. Using a different constant is tricky as compilation fails if the header doesn't define it. But I think we can at least fix the crash and just print the numeric identifier for unknown identifiers.

Regarding adding additional structs/strings, I had a look at the different headers. Linux, FreeBSD and macOS are the same up to SADB_X_EXT_SA2 (19), then macOS diverges. FreeBSD and Linux do so after SADB_X_EXT_NAT_T_DPORT (22), as you already noticed. FreeBSD and macOS might share 27/28 again (address migration), although it looks like macOS might use its own message and not SADB_UPDATE for this, so that is probably without consequence. OpenBSD uses a completely different set of custom extensions, but strongSwan currently doesn't run on it, so I guess we can ignore that for now.

Anyway, adding the strings should be OK, however, adding structs could be tricky when compiling on older versions of a platform whose headers don't yet include that struct, so that requires some ifdefs.

I pushed two commits to the 3210-pfkey-ext branch.

#2 Updated by Jean-François Hren 29 days ago

I tested the 3210-pfkey-ext branch and it worked fine. Thank you.

#3 Updated by Tobias Brunner 21 days ago

I tested the 3210-pfkey-ext branch and it worked fine. Thank you.

Thanks for testing. I've now replaced the runtime check with one that should be evaluated during compilation (which already revealed several missing strings in our codebase). Does that still work for you (the FreeBSD build on Cirrus-CI was successful, so I'm hopeful :)?

#4 Updated by Jean-François Hren 21 days ago

It still works for us, thank you.

#5 Updated by Tobias Brunner 18 days ago

  • Status changed from Feedback to Closed
  • Assignee set to Tobias Brunner
  • Resolution set to Fixed

Thanks for testing, merged to master.

Also available in: Atom PDF