On Tue, Feb 18, 2020 at 12:09:10PM +0100, Tobias Heider wrote:
> here is an update of the last diff rebased onto current with minor fixes.
> There
> were some problems when multiple transport and non-transport policies were
> configured, which should now be fixed.
> I also have a test case for the new regression test which runs successfully.
Thanks, as already discussed off-list this is working as advertised.
This is useful for scenarios such as GRE over IPsec where double
encapsulation is neither necessary nor desired.
OK kn,
nits inline
> index d99d52ff77c..7b4c2075810 100644
> --- a/sbin/iked/iked.h
> +++ b/sbin/iked/iked.h
> - /* compression */
> + /* compression, transport mode */
I'd drop these useless comments.
> if ((pol->pol_flags & IKED_POLICY_IPCOMP) &&
> (len = ikev2_add_ipcompnotify(env, e, &pld, len, sa, 1)) == -1)
> goto done;
> + if ((pol->pol_flags & IKED_POLICY_TRANSPORT) &&
> + (len = ikev2_add_transport_mode(env, e, &pld, len, sa)) == -1)
> + goto done;
>
> if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_SA) == -1)
> goto done;
> @@ -1786,6 +1791,13 @@ ikev2_add_sighashnotify(struct ibuf *e, struct
> ikev2_payload **pld,
> return (len);
> }
> diff --git a/sbin/iked/parse.y b/sbin/iked/parse.y
> index fe052068922..3ca68e2fd7a 100644
> --- a/sbin/iked/parse.y
> +++ b/sbin/iked/parse.y
> @@ -426,7 +426,7 @@ typedef struct {
> %type <v.id> id
> %type <v.transforms> transforms
> %type <v.filters> filters
> -%type <v.ikemode> ikeflags ikematch ikemode ipcomp
> +%type <v.ikemode> ikeflags ikematch ikemode ipcomp tmode
> %type <v.ikeauth> ikeauth
> %type <v.ikekey> keyspec
> %type <v.mode> ike_sas child_sas
> @@ -890,7 +890,7 @@ child_sa : CHILDSA {
> }
> ;
>
> -ikeflags : ikematch ikemode ipcomp { $$ = $1 | $2 | $3; }
> +ikeflags : ikematch ikemode ipcomp tmode { $$ = $1 | $2 | $3 | $4; }
> ;
>
> ikematch : /* empty */ { $$ = 0; }
> @@ -908,6 +908,11 @@ ipcomp : /* empty */ { $$ =
> 0; }
> | IPCOMP { $$ = IKED_POLICY_IPCOMP; }
> ;
>
> +tmode : /* empty */ { $$ = 0; }
> + | TUNNEL { $$ = 0; }
This should probably be explicit just like TRANSPORT.
Although unlikely, in the default for tmode changes this 0 for TUNNEL
would case an explicit `tunnel' in the config to change to the new
default as well.
> + | TRANSPORT { $$ = IKED_POLICY_TRANSPORT; }
> + ;
> +
> ikeauth : /* empty */ {
> $$.auth_method = IKEV2_AUTH_SIG_ANY; /* default */
> $$.auth_eap = 0;
> @@ -2465,6 +2470,9 @@ print_policy(struct iked_policy *pol)
> else
> print_verbose(" passive");
>
> + if (pol->pol_flags & IKED_POLICY_TRANSPORT)
> + print_verbose(" transport");
> +
What about printing "tunnel"? Omitting it makes existing config and
output stay the same with your diff, on the other hand we do have options
that are printed already even when being the default and/or omitted from
the config.
> print_verbose(" %s", print_xf(pol->pol_saproto, 0, saxfs));
>
> if (pol->pol_ipproto)
>
