Project

General

Profile

Bug #2954

The inactivity parameter changes the start_action

Added by Alexander Vasiliev 18 days ago. Updated 16 days ago.

Status:
Closed
Priority:
Normal
Category:
vici
Target version:
Start date:
06.03.2019
Due date:
Estimated time:
Affected version:
5.7.2
Resolution:
Fixed

Description

Hi. I dont know if this is a bug but when i've got this behavior i was confused. Also i didnt find any info in wiki or man pages.
When i run strongswan-swanctl.service with this in swanctl.conf:

inactivity = 300s
close_action = trap
start_action = trap

it ends up with this in logs (and this is what i expect to see):

...
Wed, 2019-03-06 18:26 09[CFG]  conn to-881:
Wed, 2019-03-06 18:26 09[CFG]   child to-881:
...
Wed, 2019-03-06 18:26 09[CFG]    start_action = hold
Wed, 2019-03-06 18:26 09[CFG]    close_action = hold
...
Wed, 2019-03-06 18:26 09[CFG]    inactivity = 300
...

But if i will put inactivity after actions:

close_action = trap
start_action = trap
inactivity = 300s

...
Wed, 2019-03-06 18:32 08[CFG]  conn to-881:
Wed, 2019-03-06 18:32 08[CFG]   child to-881:
...
Wed, 2019-03-06 18:32 08[CFG]    start_action = clear
Wed, 2019-03-06 18:32 08[CFG]    close_action = hold
...
Wed, 2019-03-06 18:32 08[CFG]    inactivity = 300
...

as a result there is no installed routes and policies.
May be this is "by design" but there are no any notes on a strict order of options.

Thanks, Alex
PS. I am sorry for my english

Associated revisions

Revision 090e2cf5 (diff)
Added by Tobias Brunner 16 days ago

vici: Correctly parse inactivity timeout as uint32_t

Using parse_time() directly actually overwrites the next member in the
child_cfg_create_t struct, which is start_action, which can cause
incorrect configs if inactivity is parsed after start_action.

Fixes #2954.

History

#1 Updated by Tobias Brunner 17 days ago

  • Category changed from swanctl to vici
  • Status changed from New to Feedback
  • Assignee set to Tobias Brunner
  • Target version set to 5.8.0
  • Resolution set to Fixed

The order of the options obviously should not matter. So it's definitely a bug.

The vici plugin parses the value of the inactivity option using the parse_time() utility function, which expects a pointer to a uint64_t. However, the struct member for inactivity, whose pointer is passed, is only a uint32_t. So writing to it also affects the struct member that follows it, which is, you can probably guess, start_action. So only if start_action is parsed after inactivity will it work as that corrects this side-effect.

I pushed a fix for this to the 2954-vici-inactivity branch.

By the way, close_action=trap is not needed if you already have start_action=trap.

#2 Updated by Alexander Vasiliev 17 days ago

I pushed a fix for this to the 2954-vici-inactivity branch.

Great. Thank you

By the way, close_action=trap is not needed if you already have start_action=trap.

Ok. I am new in this IPSec and StrongSwan world. So was trying things and got stucked with this.

Thanks again.

#3 Updated by Tobias Brunner 16 days ago

  • Status changed from Feedback to Closed

Also available in: Atom PDF