Project

General

Profile

Feature #2212

RFC 2367 says: Only SADB_SASTATE_MATURE SAs may be submitted in an SADB_ADD message.

Added by Andrey Elsukov almost 4 years ago. Updated almost 4 years ago.

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

Description

RFC2367 requires that SADB_SASTATE_MATURE should be used in SADB_ADD message. The same should be done for SADB_UPDATE.

--- a/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c
+++ b/src/libcharon/plugins/kernel_pfkey/kernel_pfkey_ipsec.c
@@ -1717,6 +1717,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t,
        sa->sadb_sa_exttype = SADB_EXT_SA;
        sa->sadb_sa_len = PFKEY_LEN(len);
        sa->sadb_sa_spi = id->spi;
+       sa->sadb_sa_state = SADB_SASTATE_MATURE;
        if (id->proto == IPPROTO_COMP)
        {
                sa->sadb_sa_encrypt = lookup_algorithm(COMPRESSION_ALGORITHM,

Associated revisions

Revision 4ae2209e (diff)
Added by Tobias Brunner almost 4 years ago

kernel-pfkey: Set state to SADB_SASTATE_MATURE when adding/updating SAs

Picky kernels might otherwise reject our messages as RFC 2367 explicitly
mandates this.

Fixes #2212.

History

#1 Updated by Tobias Brunner almost 4 years ago

  • Tracker changed from Issue to Feature
  • Description updated (diff)
  • Status changed from New to Feedback
  • Target version set to 5.5.2

I see that the RFC explicitly says this. And I guess with a picky kernel this is problematic (Therefore, the sadb_sa_state field of all submitted SAs MUST be SADB_SASTATE_MATURE and the kernel MUST return an error if this is not true.). But I wonder, does it really matter? What's the point of adding/updating an SA in any other state (in particular because the RFC only allows this one state)? So couldn't the kernel just ignore the state of the passed SA and assume SADB_SASTATE_MATURE?

Anyway, I pushed a fix to the 2212-pfkey-sastate branch.

#2 Updated by Andrey Elsukov almost 4 years ago

Tobias Brunner wrote:

I see that the RFC explicitly says this. And I guess with a picky kernel this is problematic (Therefore, the sadb_sa_state field of all submitted SAs MUST be SADB_SASTATE_MATURE and the kernel MUST return an error if this is not true.).

Yes, for SADB_ADD and SADB_UPDATE all SAs MUST have SADB_SASTATE_MATURE. And for SADB_GETSPI SAs MUST have SADB_SASTATE_LARVAL state. I just added such checks and have discovered that nobody takes care of this, so I had to disable it to not break anything. ;)

But I wonder, does it really matter? What's the point of adding/updating an SA in any other state (in particular because the RFC only allows this one state)? So couldn't the kernel just ignore the state of the passed SA and assume SADB_SASTATE_MATURE?

Anyway, I pushed a fix to the 2212-pfkey-sastate branch.

Thanks.
Tobias, while you are here, can you take a look at this proposal and say what you think? https://forums.freebsd.org/threads/40671/#post-339942
Will this useful for strongswan?

#3 Updated by Tobias Brunner almost 4 years ago

I see that the RFC explicitly says this. And I guess with a picky kernel this is problematic (Therefore, the sadb_sa_state field of all submitted SAs MUST be SADB_SASTATE_MATURE and the kernel MUST return an error if this is not true.).

Yes, for SADB_ADD and SADB_UPDATE all SAs MUST have SADB_SASTATE_MATURE. And for SADB_GETSPI SAs MUST have SADB_SASTATE_LARVAL state. I just added such checks and have discovered that nobody takes care of this, so I had to disable it to not break anything. ;)

Yeah, I'd imagine that's the case :)

Tobias, while you are here, can you take a look at this proposal and say what you think? https://forums.freebsd.org/threads/40671/#post-339942
Will this useful for strongswan?

Thanks, I've replied to the thread.

#4 Updated by Tobias Brunner almost 4 years ago

  • Category set to kernel-interface
  • Status changed from Feedback to Closed
  • Assignee set to Tobias Brunner
  • Resolution set to Fixed

Also available in: Atom PDF