Try solve upstream conflict #2
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If we don't do this, get_plain() will fail after generating the message fragmented unless it was generated non-fragmented previously.
The message ID of the first IKE_AUTH exchange is a safe-guard against potential truncation attacks if IKE_INTERMEDIATE exchanges are not used for multiple key exchanges but some other future use where the number of exchanges might not depend on the selected proposal.
Initially, this is handled with a key derivation for each IKE_INTERMEDIATE exchange. When rekeying, the keys are derived only once all IKE_FOLLOWUP_KE exchanges are done.
It also changes that payloads are built before installing the CHILD_SA on the responder, that is, the KE payload is generated before keys are derived, so that key_exchange_t::get_public_key() is called before get_shared_secret(), or its internal equivalent, which could be relevant for KE implementations that want to ensure that the key can't be accessed again after the key derivation.
…g changed The responder doesn't create a CHILD_SA and allocate an SPI anymore when responding with an INVALID_KE_PAYLOAD notify.
Co-authored-by: Tobias Brunner <[email protected]>
All additional (and the initial) key exchanges must use a different method.
As the winner of a rekey collision, we previously always triggered the child_rekey() event once when creating the redundant SA on behalf of the peer in the passive child-rekey task and then a second time when creating the winning SA in the active task. However, both calls passed the replaced CHILD_SA as "old". This made tracking CHILD_SAs impossible because there was no transition from the redundant, "new" SA of the first event to the "new", winning SA of the second. Of course, when the second event was triggered, the redundant SA might not have existed anymore because the peer is expected to delete it, which could happen before the CREATE_CHILD_SA response arrives at the initiator. This refactoring ensures that the child_rekey() event is triggered in a way that makes the CHILD_SAs trackable in all reasonable (and even some unreasonable) scenarios. The event is generally only triggered once after installing the outbound SA for the new/winning CHILD_SA. This can be when processing the CREATE_CHILD_SA in the active child-rekey task, or when processing the DELETE for the old SA in a passive child-delete task. There are some cases where the event is still triggered twice, but it is now ensured that listeners can properly transition to the winning SA. Some corner cases are now also handled correctly, e.g. if a responder's DELETE for the new CHILD_SA arrives before its CREATE_CHILD_SA response that actually creates it on the initiator. Also handled properly are responders of rekeyings that incorrectly send a DELETE for the old CHILD_SA (previously this caused both, the new and the old SA, to get deleted).
Previously, it could happen that child_rekey() was triggered twice for the same "old" SA. For listeners that would mean they'd loose track as they'd be tracking a new SA that wasn't relevant anymore and for which no updown event would ever get triggered (it was the redundant SA in a collision). This new assert ensures that events are triggered in a predictable way and listeners can track SAs properly.
Not really building it out-of-tree for now, though.
Can be useful when using the script locally.
This doesn't really seem useful (perhaps it was before we started to configure the outbound interface on our routes). And it can actually cause the route installation to fail e.g. for routes over point-to-point interfaces where we'd get "Error: Nexthop has invalid gateway" errors. Closes #2548
Synchronization for the additional transforms in the IKE and Child SA proposals is added. Details of the IKE_SA synchronization are changed to support IKE_INTERMEDIATE exchanges that cause multiple HA_IKE_ADD messages and key derivations. The cache has been extended to handle multiple such messages. Co-authored-by: Thomas Egerer <[email protected]>
The node that gets activated usually won't be able to complete the IKE_SA mainly because the IKE keys are now derived delayed, so the key material required to process a message often won't be available (only later IKE_AUTH messages and retransmits of earlier messages that the active node already received and synced the keys for may be decrypted). A second issue affects IKE_SAs with multiple key exchanges. Because the IntAuth value(s) are currently not synced, which are necessary to verify/create the AUTH payloads, the IKE_AUTH exchange couldn't be completed.
Adds support for multiple key exchanges to the ha plugin. Also, because of the delayed key derivation and the not synced IntAuth values, incomplete IKE_SAs are now destroyed during a failover. Closes #2550
Using a specific address can be useful in scenarios where dynamic routing could change the path to the RADIUS server and a changing source address is a problem for the server. Closes #2598
Without UDP-encapsulation, the IKE and ESP traffic is not directly related (other than via IPs), so firewalls might no keep the state for IKE traffic alive if there is no IKE traffic for a while and constant ESP traffic prevents DPDs from being exchanged because inbound ESP traffic is considered. Closes #1759
When using port 4500 for IKE_SA_INIT, Windows Server 2016, 2025 and possibly others send back all packets to the port initially used by the client, not the one floated to before sending IKE_AUTH. So if UDP encapsulation is used, no traffic can be received as the initial socket can't have UDP decapsulation enabled. tcpdump output: ``` IP <client-ip>.47547 > <server-ip>.4500: UDP-encap: ESP(spi=0xfd4e5fc2,seq=...) IP <server-ip>.4500 > <client-ip>.57962: UDP-encap: ESP(spi=0xccc5e213,seq=...) ``` Avoid this by floating early if a non-default destination port is used. This also ensures we don't send packets from port 500 (without non-ESP marker) if ephemeral source ports are not used. Closes #2664 Signed-off-by: Michael Braun <[email protected]> Co-authored-by: Tobias Brunner <[email protected]>
Fixes: a5e80cf ("libcharon: Enable make_before_break option by default")
…reauth Listeners can't track those IKE_SAs otherwise. For break-before-make reauthentications, these events are already triggered because that is implemented by calling reestablish() on the old IKE_SA.
In particular the one for charon-nm was missing. References #2683
These have specific values for charon-nm's use case but might have to be changed for special setups or because of conflicts. References #2683
If the client's network goes down for a while but the same IP address is assigned later, it won't be aware if the server killed the IKE_SA while it wasn't reachable. This way, a DPD is triggered and the client can reestablish the SA if necessary. When roaming to a different IP, a MOBIKE update is triggered with the same effect. References #2696
…quickly These are the same values we use for the Android app. References #2696
Fixes: 3babf1f ("vici: Update Python build")
Fixes: 3babf1f ("vici: Update Python build")
The copied target string was not freed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.