Christophe,

> Make replay window configurable per connection.

Thanks for your patch, I finally could take a look at it. Some notes:

> +++ b/src/libcharon/config/child_cfg.c
> +             .replay_window = -1,

I think it would make sense to initialize the default value directly to
->get_int("charon.replay_window"), and remove that get_int() from the
kernel interface. Any kernel backend could make use of it, and...

> +++ b/src/libcharon/plugins/stroke/stroke_config.c
> +     child_cfg->set_replay_window(child_cfg, msg->add_conn.replay_window);

... you could have that -1 magic value check just specific to stroke,
and not across the whole code base. Just omit set_replay_window() in
that case to use the default.

> +++ b/src/libhydra/kernel/kernel_ipsec.h
> @@ -115,7 +116,8 @@ struct kernel_ipsec_t {
> -                                             ipsec_mode_t mode, u_int16_t 
> ipcomp, u_int16_t cpi,
> +                                             ipsec_mode_t mode, u_int16_t 
> ipcomp,
> +                                             u_int32_t replay_window, 
> u_int16_t cpi,

There are some additional implementations of kernel_ipsec_t that need to
get updated to the new method signature, namely:

> $ git grep -l "^[[:space:]]*kernel_ipsec_t \w*;" -- '*.h'
> src/charon-tkm/src/tkm/tkm_kernel_ipsec.h
> src/frontends/android/jni/libandroidbridge/kernel/android_ipsec.h
> src/libcharon/plugins/kernel_libipsec/kernel_libipsec_ipsec.h
> src/libcharon/plugins/load_tester/load_tester_ipsec.h
> src/libhydra/plugins/kernel_klips/kernel_klips_ipsec.h
> src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.h
> src/libhydra/plugins/kernel_pfkey/kernel_pfkey_ipsec.h

> +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c
> +     if (replay_window == UINT32_C(-1) ||
> +             replay_window == this->replay_window)
> +     {
> +             /* default replay_window parameters */
> +             replay_window = this->replay_window;
> +             replay_bmp = this->replay_bmp;
> +     }
> +     else
> +     {
> +             /* sa-specific replay_window parameters */
> +             replay_bmp = replay_bmp_len(replay_window);
> +     }

this->replay_* gets obsolete, and we'd just need a single code path
here.

All other changes look fine.

Best regards
Martin

_______________________________________________
Dev mailing list
[email protected]
https://lists.strongswan.org/mailman/listinfo/dev

Reply via email to