Project

General

Profile

Issue #1157

Message ID overflow RFC 5996 2.2

Added by Hendrik Donner about 5 years ago. Updated about 5 years ago.

Status:
Feedback
Priority:
Normal
Assignee:
-
Category:
-
Affected version:
5.3.3
Resolution:

Description

According to RFC 5996 2.2 (https://tools.ietf.org/html/rfc5996#section-2.2) Message ID overflow must be handled by rekeying or closing the SA:

"In the unlikely event that Message IDs grow too large to fit in 32 bits, the IKE SA MUST be closed or rekeyed."

I think that should happen in src/libcharon/sa/ikev2/task_manager_v2.c but _incr_mid() is missing any overflow handling.
The message ID is incremented without overflow checks in other places, too (e.g. this->initiating.mid++ in process_response()).


Related issues

Is duplicate of Issue #2559: Whether support rekey when sequence number overflow?Closed

History

#1 Updated by Tobias Brunner about 5 years ago

  • Status changed from New to Feedback

According to RFC 5996 2.2 (https://tools.ietf.org/html/rfc5996#section-2.2) Message ID overflow must be handled by rekeying or closing the SA:

"In the unlikely event that Message IDs grow too large to fit in 32 bits, the IKE SA MUST be closed or rekeyed."

I think that should happen in src/libcharon/sa/ikev2/task_manager_v2.c but _incr_mid() is missing any overflow handling.
The message ID is incremented without overflow checks in other places, too (e.g. this->initiating.mid++ in process_response()).

Not that you are wrong, but as the RFC already states it is very unlikely that this would ever happen. Message IDs are 32-bit unsigned numbers, individually increased in each direction. So each peer can initiate over 4 billion exchanges before there will be an overflow. I don't see how this could ever become a problem.

#2 Updated by Hendrik Donner about 5 years ago

Tobias Brunner wrote:

According to RFC 5996 2.2 (https://tools.ietf.org/html/rfc5996#section-2.2) Message ID overflow must be handled by rekeying or closing the SA:

"In the unlikely event that Message IDs grow too large to fit in 32 bits, the IKE SA MUST be closed or rekeyed."

I think that should happen in src/libcharon/sa/ikev2/task_manager_v2.c but _incr_mid() is missing any overflow handling.
The message ID is incremented without overflow checks in other places, too (e.g. this->initiating.mid++ in process_response()).

Not that you are wrong, but as the RFC already states it is very unlikely that this would ever happen. Message IDs are 32-bit unsigned numbers, individually increased in each direction. So each peer can initiate over 4 billion exchanges before there will be an overflow. I don't see how this could ever become a problem.

The RFC also states that "..., the IKE SA MUST be closed or rekeyed." So StrongSwan is currently not implementing everything from RFC 5996. I agree that the event might be unlikely, but overflow handling would make the implementation comply to the RFC and make it more robust.

#3 Updated by Tobias Brunner about 5 years ago

According to RFC 5996 2.2 (https://tools.ietf.org/html/rfc5996#section-2.2) Message ID overflow must be handled by rekeying or closing the SA:

"In the unlikely event that Message IDs grow too large to fit in 32 bits, the IKE SA MUST be closed or rekeyed."

I think that should happen in src/libcharon/sa/ikev2/task_manager_v2.c but _incr_mid() is missing any overflow handling.
The message ID is incremented without overflow checks in other places, too (e.g. this->initiating.mid++ in process_response()).

Not that you are wrong, but as the RFC already states it is very unlikely that this would ever happen. Message IDs are 32-bit unsigned numbers, individually increased in each direction. So each peer can initiate over 4 billion exchanges before there will be an overflow. I don't see how this could ever become a problem.

The RFC also states that "..., the IKE SA MUST be closed or rekeyed." So StrongSwan is currently not implementing everything from RFC 5996. I agree that the event might be unlikely, but overflow handling would make the implementation comply to the RFC and make it more robust.

So? There are lots of things in that RFC we don't implement currently.

#4 Updated by Tobias Brunner almost 3 years ago

  • Is duplicate of Issue #2559: Whether support rekey when sequence number overflow? added

Also available in: Atom PDF