<div dir="ltr"><div>This comment is salient:</div><div>       /*<br>         * .has_client means that .client contains a hardwired value,<br>         * if it doesn't then it is filled in later (for instance by<br>         * instantiate() calling default_end() after host_addr is<br>         * known).<br>         */<br></div><div>the ddns code isn't calling default_end() after updating .host_addr.</div><div><br></div><div>from:</div><a href="https://testing.libreswan.org/v3.30-1728-ga28312f6bd-main/ikev2-ddns-03/OUTPUT/west.pluto.log.gz">https://testing.libreswan.org/v3.30-1728-ga28312f6bd-main/ikev2-ddns-03/OUTPUT/west.pluto.log.gz</a><div><br></div><div>extract_end(right) logging a warning:</div><div><pre style="color:rgb(0,0,0);white-space:pre-wrap">connection "named": failed to convert '<a href="http://right.libreswan.org">right.libreswan.org</a>' at load time: not a numeric IPv4 address and name lookup failed (no validation performed)</pre><pre style="color:rgb(0,0,0);white-space:pre-wrap">followed by default_end(), left succeeds but default_end(right) returns immediately and silently (default_end() returns an error code that is ignored :-):</pre><pre style="color:rgb(0,0,0);white-space:pre-wrap">| left host_port 500</pre><pre style="color:rgb(0,0,0);white-space:pre-wrap">which, correctly leads to:</pre><pre style="color:rgb(0,0,0);white-space:pre-wrap">| connect_to_host_pair: <a href="http://192.1.2.45:500">192.1.2.45:500</a> <unspecified>:0 -> hp@(nil): none
| new hp@0x7f52d4e8af88
added IKEv2 connection "named"</pre></div><div>somewhere here:</div><div><pre style="color:rgb(0,0,0);white-space:pre-wrap">updating pending dns lookups
| FOR_EACH_CONNECTION_... in connection_check_ddns
| pending ddns: changing connection "named" to CK_PERMANENT
| pending ddns: updating IP address for <a href="http://right.libreswan.org">right.libreswan.org</a> from <unspecified> to 192.1.2.23
| connect_to_host_pair: <a href="http://192.1.2.45:500">192.1.2.45:500</a> <a href="http://192.1.2.23:0">192.1.2.23:0</a> -> hp@(nil): none</pre></div><div>default_end() should have been re-called.</div><div><br></div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 21 Sep 2020 at 14:34, Andrew Cagney <<a href="mailto:andrew.cagney@gmail.com">andrew.cagney@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 21 Sep 2020 at 13:53, Paul Wouters <<a href="mailto:paul@nohats.ca" target="_blank">paul@nohats.ca</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sun, 20 Sep 2020, Paul Wouters wrote:<br>
<br>
> Subject: [Swan-dev] nat: ikeport commit broke DDNS tests<br>
<br>
Fixed with commit 82ffa122d2500bb7a4<br>
<br>
That's probably a bandaid and not the best solution, but it can be<br>
cleaned up later when we cleanup the port variables more.<br>
<br></blockquote><div><br></div><div>There's more to this.   The original commit moved:<br></div><div><br></div><div> @@ -818,14 +833,6 @@ static int extract_end(struct fd *whackfd,<br>                     leftright, src->host_ikeport);<br>                dst->raw.host.ikeport = 0;<br>        }<br>-    /*<br>-    * XXX: When DST is the peer setting .host_port to PLUTO_PORT<br>-         * (our port) is wrong.  IKE_UDP_PORT is the next best thing.<br>-        *<br>-    * But what if DST is THIS?  .host_port gets ignored?<br>-        */<br>-  dst->host_port = (dst->raw.host.ikeport ? dst->raw.host.ikeport : IKE_UDP_PORT);<br></div><div><br></div><div>to:<br></div><div><br></div><div>+       /*<br>+    * XXX: When DST is the peer setting .host_port to PLUTO_PORT<br>+         * (our port) is wrong.  IKE_UDP_PORT is the next best thing.<br>+        *<br>+    * But what if DST is THIS?  .host_port gets ignored?<br>+        *<br>+    * If one end has an ikeport, the other must use ikport or nat<br>+        * port.<br>+      */<br>+  e->host_port = (e->raw.host.ikeport ? e->raw.host.ikeport :<br>+                 remote_port ? NAT_IKE_UDP_PORT :<br>+                     IKE_UDP_PORT);<br>+       dbg("%s host_port %d", leftright, e->host_port);<br></div><div><br></div><div>where REMOTE_PORT is the other END's ikeport as specified in the config file.<br></div></div><div><br></div><div>Putting back the original assignment means there's code using a wrong value.<br></div><div class="gmail_quote"><div><br></div></div></div>
</blockquote></div></div></div>