Changeset View
Standalone View
net/pfkeyv2.h
Show First 20 Lines • Show All 91 Lines • ▼ Show 20 Lines | struct sadb_ext { | ||||
u_int16_t sadb_ext_len; | u_int16_t sadb_ext_len; | ||||
u_int16_t sadb_ext_type; | u_int16_t sadb_ext_type; | ||||
}; | }; | ||||
struct sadb_sa { | struct sadb_sa { | ||||
u_int16_t sadb_sa_len; | u_int16_t sadb_sa_len; | ||||
u_int16_t sadb_sa_exttype; | u_int16_t sadb_sa_exttype; | ||||
u_int32_t sadb_sa_spi; | u_int32_t sadb_sa_spi; | ||||
u_int8_t sadb_sa_replay; | u_int32_t sadb_sa_replay; | ||||
ae: From my point of view, such changes aren't allowed. This structure is defined in RFC2367 and… | |||||
emeric.poupon_stormshield.euAuthorUnsubmitted Not Done Inline ActionsI understand your point of view. But this raises other questions:
The drawback is that we have to make changes in strongSwan. emeric.poupon_stormshield.eu: I understand your point of view.
But this raises other questions:
- can we extend non… | |||||
aeUnsubmitted Not Done Inline Actions
From my research it looks like only Linux and FreeBSD/NetBSD supports SADB_EXT_SA2 message type. But since Linux defines it in its header, I think its changing is undesirable.
First of we should use value from SADB_EXT_SA. Then if new extension type is present, we override replay value.
I think we can just modify setkey in the base. Then this code can be used as a reference for strongswan/ipsec-tools. ae: > can we extend non standardized messages to add a new field? I am thinking about the non… | |||||
emeric.poupon_stormshield.euAuthorUnsubmitted Not Done Inline Actions
I guess we should disable the replay window if the regular field is non zero and the extension field is set to 0? Ok for your proposal, what about something like that? In net/pfkeyv2.h: /* Additional large replay window support */ struct sadb_x_sa_replay { u_int32_t sadb_x_sa_replay; u_int32_t sadb_x_sa_replay_reserved; }; _Static_assert(sizeof(struct sadb_x_sa_replay) == 8, "struct size mismatch"); I think it may be better to specify the replay window in packet unit instead of byte unit. It would make the code easier to read and maintain. Here is what the RFC says on the field: "The size of the replay window, if not zero. If zero, then no replay window is in use". What do you think? emeric.poupon_stormshield.eu: > First of we should use value from SADB_EXT_SA. Then if new extension type is present, we… | |||||
aeUnsubmitted Not Done Inline Actions
I think so. Since application knows about our extension header, we expect its value has highest priority. But probably, when the kernel returns back the reply, it should reset replay value in SADB_EXT_SA to zero and omit SADB_EXT_SA_REPLAY header.
Looks good for me.
You are the first who implementing this, so I think this is at your discretion. ae: > I guess we should disable the replay window if the regular field is non zero and the… | |||||
u_int8_t sadb_sa_state; | u_int8_t sadb_sa_state; | ||||
u_int8_t sadb_sa_auth; | u_int8_t sadb_sa_auth; | ||||
u_int8_t sadb_sa_encrypt; | u_int8_t sadb_sa_encrypt; | ||||
u_int8_t sadb_sa_reserved[5]; | |||||
u_int32_t sadb_sa_flags; | u_int32_t sadb_sa_flags; | ||||
}; | }; | ||||
struct sadb_lifetime { | struct sadb_lifetime { | ||||
u_int16_t sadb_lifetime_len; | u_int16_t sadb_lifetime_len; | ||||
u_int16_t sadb_lifetime_exttype; | u_int16_t sadb_lifetime_exttype; | ||||
u_int32_t sadb_lifetime_allocations; | u_int32_t sadb_lifetime_allocations; | ||||
u_int64_t sadb_lifetime_bytes; | u_int64_t sadb_lifetime_bytes; | ||||
Show All 33 Lines | struct sadb_sens { | ||||
u_int8_t sadb_sens_integ_level; | u_int8_t sadb_sens_integ_level; | ||||
u_int8_t sadb_sens_integ_len; | u_int8_t sadb_sens_integ_len; | ||||
u_int32_t sadb_sens_reserved; | u_int32_t sadb_sens_reserved; | ||||
}; | }; | ||||
struct sadb_prop { | struct sadb_prop { | ||||
u_int16_t sadb_prop_len; | u_int16_t sadb_prop_len; | ||||
u_int16_t sadb_prop_exttype; | u_int16_t sadb_prop_exttype; | ||||
u_int8_t sadb_prop_replay; | u_int32_t sadb_prop_replay; | ||||
aeUnsubmitted Not Done Inline ActionsThis also is objectionable change. RFC2367 says that mostly all fields of structures are in host byte order. ae: This also is objectionable change. RFC2367 says that mostly all fields of structures are in… | |||||
u_int8_t sadb_prop_reserved[3]; | |||||
}; | }; | ||||
struct sadb_comb { | struct sadb_comb { | ||||
u_int8_t sadb_comb_auth; | u_int8_t sadb_comb_auth; | ||||
u_int8_t sadb_comb_encrypt; | u_int8_t sadb_comb_encrypt; | ||||
u_int16_t sadb_comb_flags; | u_int16_t sadb_comb_flags; | ||||
u_int16_t sadb_comb_auth_minbits; | u_int16_t sadb_comb_auth_minbits; | ||||
u_int16_t sadb_comb_auth_maxbits; | u_int16_t sadb_comb_auth_maxbits; | ||||
▲ Show 20 Lines • Show All 279 Lines • Show Last 20 Lines |
From my point of view, such changes aren't allowed. This structure is defined in RFC2367 and changing the size of fields and even the size of structure will lead to ABI breakage.
I think the proper way is introduce some new structure like sadb_sa2 was added.