pluto: add ALLOW_MICROSOFT_BAD_PROPOSAL for self-proposals #447
Conversation
Earlier commit, introduced checking for self-proposals. Yet the commit did not add the ALLOW_MICROSOFT_BAD_PROPOSAL workaround in said case. Add the workaround, since it's required in a number of cases: https://bbs.archlinux.org/viewtopic.php?id=253434 Cc: Michael Richardson <mcr@credil.org> Fixes: 3a9afb0 ("wo#6211 . the check on the peers reply should also use localaddr when checking") Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
| @@ -602,7 +602,12 @@ check_net_id(struct isakmp_ipsec_id *id | |||
| } else if(!end->has_client && !subnetisaddr(&net_temp, endip)) { | |||
| loglog(RC_LOG_SERIOUS, "%s subnet returned does not match my self-proposal - us:%s vs them:%s", | |||
| which,subxmt,subrec); | |||
| bad_proposal = TRUE; | |||
| #ifdef ALLOW_MICROSOFT_BAD_PROPOSAL | |||
There was a problem hiding this comment.
Since both sides of this if()/else if(), result in the same exception being applied, the overall affect of ALLOW_MICROSOFT_BAD_PROPOSAL is to do nothing, and log the same thing, please just surround the entire if/else of an #ifdef, and don't even do the test.
Please document what versions of which Microsoft products do this.
This should be set on a per-conn basis, I think. Or perhaps, enabled by a vendor-based impairment.
I would have removed this #ifdef had I noticed it before.
There was a problem hiding this comment.
There's a subtle difference between the messages in the if/elseif branches, which clearly highlights which is the offending section. Grouping them together would make that harder, so if anything it's better to keep it as-is.
The device is Ubiquiti/UniFi (don't know further details sorry), plus a search through the webs shows many other vendors which need the workaround. So yes, the name is misleading, yet considering it has been enabled by default for years, removing it sounds like a terrible move.
|
Cherry picked into the master branch. |
Earlier commit, introduced checking for self-proposals. Yet the commit
did not add the ALLOW_MICROSOFT_BAD_PROPOSAL workaround in said case.
Add the workaround, since it's required in a number of cases:
https://bbs.archlinux.org/viewtopic.php?id=253434
Fixes: 3a9afb0 ("wo#6211 . the check on the peers reply should also use localaddr when checking")
/cc @mcr @shussain