Hi Paul, > [...] which I tracked down to the tsi of the Cisco peer not returning a > port number in its reply.
I see. > Using the patch below, I was able to accommodate this omission. Does > this seem like a reasonable change, perhaps behind a configuration > flag? Thanks for the patch, looks reasonable. I don't think a configuration option is necessary, as long as we install the more restrictive selector. Instead of just checking the port, I think we can handle this in a more generic way by selecting the subset of the proposed and the returned selector. This should work in any case, in is actually even simpler. Please try the attached patch, if that works, I can push it to master. Best regards Martin
>From 9d9042d6d95b0ecb292d77e7d8350fcd28e1aa27 Mon Sep 17 00:00:00 2001 From: Martin Willi <[email protected]> Date: Thu, 7 Mar 2013 09:50:43 +0100 Subject: [PATCH] As Quick Mode initiator, select a subset of the proposed and the returned TS Cisco 5505 firewalls don't return the port if we send a specific one, letting the is_contained_in() checks fail. Using get_subset() selection builds the Quick Mode correctly with the common subset of selectors. Based on an initial patch from Paul Stewart. --- src/libcharon/sa/ikev1/tasks/quick_mode.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libcharon/sa/ikev1/tasks/quick_mode.c b/src/libcharon/sa/ikev1/tasks/quick_mode.c index 16c4763..afdff8c 100644 --- a/src/libcharon/sa/ikev1/tasks/quick_mode.c +++ b/src/libcharon/sa/ikev1/tasks/quick_mode.c @@ -594,20 +594,27 @@ static bool get_ts(private_quick_mode_t *this, message_t *message) if (this->initiator) { + traffic_selector_t *tsisub, *tsrsub; + /* check if peer selection is valid */ - if (!tsr->is_contained_in(tsr, this->tsr) || - !tsi->is_contained_in(tsi, this->tsi)) + tsisub = this->tsi->get_subset(this->tsi, tsi); + tsrsub = this->tsr->get_subset(this->tsr, tsr); + if (!tsisub || !tsrsub) { DBG1(DBG_IKE, "peer selected invalid traffic selectors: " "%R for %R, %R for %R", tsi, this->tsi, tsr, this->tsr); + DESTROY_IF(tsisub); + DESTROY_IF(tsrsub); tsi->destroy(tsi); tsr->destroy(tsr); return FALSE; } + tsi->destroy(tsi); + tsr->destroy(tsr); this->tsi->destroy(this->tsi); this->tsr->destroy(this->tsr); - this->tsi = tsi; - this->tsr = tsr; + this->tsi = tsisub; + this->tsr = tsrsub; } else { -- 1.7.10.4
_______________________________________________ Dev mailing list [email protected] https://lists.strongswan.org/mailman/listinfo/dev
