WireGuard Archive on lore.kernel.org
 help / color / Atom feed
* android: handling ipv6 literals
@ 2018-08-06 15:38 Peter Gervai
  2018-08-13 15:12 ` [PATCH android] config: fix wrong Peer endpoint string format Zhao Gang
  2018-08-16  9:04 ` [PATCH android v2] " Zhao Gang
  0 siblings, 2 replies; 4+ messages in thread
From: Peter Gervai @ 2018-08-06 15:38 UTC (permalink / raw)
  To: wireguard

Hello,

Andoid client mess up ipv6 literals: input accept the - correct -
[1234:5678::1]:12345 format, but on the next edit the field gets
transformed to 1234:5678::1:12345 which obviously won't quite work.

A sidenote: pinging from the client could be useful (I have network
tools installed, but without it it's hard to tell what's going on).


Peter

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH android] config: fix wrong Peer endpoint string format
  2018-08-06 15:38 android: handling ipv6 literals Peter Gervai
@ 2018-08-13 15:12 ` Zhao Gang
  2018-08-16 19:09   ` Jason A. Donenfeld
  2018-08-16  9:04 ` [PATCH android v2] " Zhao Gang
  1 sibling, 1 reply; 4+ messages in thread
From: Zhao Gang @ 2018-08-13 15:12 UTC (permalink / raw)
  To: wireguard

When a tunnel is running, saving the tunnel's config with an IPv6 address endpoint like [::1]:42 would result in the wrong format ::1:42. This patch fixes it.

For endpoints with an IPv6 address(e.g. [::1]:42). Since the default endpoint InetSocketAddress is created unresolved, getEndpointString() returns "[::1]:42" (InetSocketAddress.getHostString() returns the literal hostname). After the endpoint is resolved, getEndpointString() returns "::1:42" (InetSocketAddress.getHostString() returns the IPv6 address without the square brackets). This inconsistent return values caused the above mentioned bug.

With this patch, function getEndpointString would return the right format string whether the endpoint is resolved or not. Also changed the function getResolvedEndpointString to call getEndpointString instead of making the string itself.

Signed-off-by: Zhao Gang <gang.zhao.42@gmail.com>
---
 app/src/main/java/com/wireguard/config/Peer.java | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/app/src/main/java/com/wireguard/config/Peer.java b/app/src/main/java/com/wireguard/config/Peer.java
index 49c8b70..4c8703c 100644
--- a/app/src/main/java/com/wireguard/config/Peer.java
+++ b/app/src/main/java/com/wireguard/config/Peer.java
@@ -71,7 +71,11 @@ public class Peer {
     private String getEndpointString() {
         if (endpoint == null)
             return null;
-        return String.format("%s:%d", endpoint.getHostString(), endpoint.getPort());
+
+        if (endpoint.getHostString().contains(":") && !endpoint.getHostString().contains("["))
+            return String.format("[%s]:%d", endpoint.getHostString(), endpoint.getPort());
+        else
+            return String.format("%s:%d", endpoint.getHostString(), endpoint.getPort());
     }
 
     public int getPersistentKeepalive() {
@@ -102,13 +106,8 @@ public class Peer {
             endpoint = new InetSocketAddress(endpoint.getHostString(), endpoint.getPort());
         if (endpoint.isUnresolved())
             throw new UnknownHostException(endpoint.getHostString());
-        if (endpoint.getAddress() instanceof Inet6Address)
-            return String.format("[%s]:%d",
-                    endpoint.getAddress().getHostAddress(),
-                    endpoint.getPort());
-        return String.format("%s:%d",
-                endpoint.getAddress().getHostAddress(),
-                endpoint.getPort());
+
+        return getEndpointString();
     }
 
     public void parse(final String line) {
-- 
2.18.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH android v2] config: fix wrong Peer endpoint string format
  2018-08-06 15:38 android: handling ipv6 literals Peter Gervai
  2018-08-13 15:12 ` [PATCH android] config: fix wrong Peer endpoint string format Zhao Gang
@ 2018-08-16  9:04 ` " Zhao Gang
  1 sibling, 0 replies; 4+ messages in thread
From: Zhao Gang @ 2018-08-16  9:04 UTC (permalink / raw)
  To: wireguard

When a tunnel is running, saving the tunnel's config with an IPv6 address endpoint like [::1234]:42 would result in the wrong format ::1234:42. This patch fixes it.

For endpoints with an IPv6 address(e.g. [::1234]:42). Since the default endpoint InetSocketAddress is created unresolved, getEndpointString() returns "[::1234]:42" (InetSocketAddress.getHostString() returns the literal hostname). After the endpoint is resolved, getEndpointString() returns "::1234:42" (InetSocketAddress.getHostString() returns the IPv6 address without the square brackets). This inconsistent return values caused the above mentioned bug.

With this patch, function getEndpointString would return the right format string whether the endpoint is resolved or not. Also changed the function getResolvedEndpointString to call getEndpointString instead of making the string itself.

Reported-by: Peter Gervai <grin@grin.hu>
Signed-off-by: Zhao Gang <gang.zhao.42@gmail.com>
---
 app/src/main/java/com/wireguard/config/Peer.java | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/app/src/main/java/com/wireguard/config/Peer.java b/app/src/main/java/com/wireguard/config/Peer.java
index 49c8b70..40b77ff 100644
--- a/app/src/main/java/com/wireguard/config/Peer.java
+++ b/app/src/main/java/com/wireguard/config/Peer.java
@@ -15,7 +15,6 @@ import android.support.annotation.Nullable;
 import com.android.databinding.library.baseAdapters.BR;
 import com.wireguard.crypto.KeyEncoding;
 
-import java.net.Inet6Address;
 import java.net.InetSocketAddress;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -71,7 +70,11 @@ public class Peer {
     private String getEndpointString() {
         if (endpoint == null)
             return null;
-        return String.format("%s:%d", endpoint.getHostString(), endpoint.getPort());
+
+        if (endpoint.getHostString().contains(":") && !endpoint.getHostString().contains("["))
+            return String.format("[%s]:%d", endpoint.getHostString(), endpoint.getPort());
+        else
+            return String.format("%s:%d", endpoint.getHostString(), endpoint.getPort());
     }
 
     public int getPersistentKeepalive() {
@@ -102,13 +105,8 @@ public class Peer {
             endpoint = new InetSocketAddress(endpoint.getHostString(), endpoint.getPort());
         if (endpoint.isUnresolved())
             throw new UnknownHostException(endpoint.getHostString());
-        if (endpoint.getAddress() instanceof Inet6Address)
-            return String.format("[%s]:%d",
-                    endpoint.getAddress().getHostAddress(),
-                    endpoint.getPort());
-        return String.format("%s:%d",
-                endpoint.getAddress().getHostAddress(),
-                endpoint.getPort());
+
+        return getEndpointString();
     }
 
     public void parse(final String line) {
-- 
2.18.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH android] config: fix wrong Peer endpoint string format
  2018-08-13 15:12 ` [PATCH android] config: fix wrong Peer endpoint string format Zhao Gang
@ 2018-08-16 19:09   ` Jason A. Donenfeld
  0 siblings, 0 replies; 4+ messages in thread
From: Jason A. Donenfeld @ 2018-08-16 19:09 UTC (permalink / raw)
  To: gang.zhao.42; +Cc: WireGuard mailing list

Hi Gang,

Thanks for the patch, I'll merge the first hunk of this but not the
second, because:

On Thu, Aug 16, 2018 at 11:58 AM Zhao Gang <gang.zhao.42@gmail.com> wrote:
> -        return String.format("%s:%d", endpoint.getHostString(), endpoint.getPort());
> +
> +        if (endpoint.getHostString().contains(":") && !endpoint.getHostString().contains("["))
> +            return String.format("[%s]:%d", endpoint.getHostString(), endpoint.getPort());
> +        else
> +            return String.format("%s:%d", endpoint.getHostString(), endpoint.getPort());

This seems like a fix for the bug. But:

> -        if (endpoint.getAddress() instanceof Inet6Address)
> -            return String.format("[%s]:%d",
> -                    endpoint.getAddress().getHostAddress(),
> -                    endpoint.getPort());
> -        return String.format("%s:%d",
> -                endpoint.getAddress().getHostAddress(),
> -                endpoint.getPort());

The purpose of the code here is to resolve the hostname to an IP
address, and then return ip+port, with no hostnames in it. Replacing
it with a call to the prior fixed function isn't the intended
behavior.

I've committed your fix here as:
https://git.zx2c4.com/wireguard-android/commit/?id=ace2a77bc9019d050112bed5a5d1eb2a3363d971

Thanks again,
Jason

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-06 15:38 android: handling ipv6 literals Peter Gervai
2018-08-13 15:12 ` [PATCH android] config: fix wrong Peer endpoint string format Zhao Gang
2018-08-16 19:09   ` Jason A. Donenfeld
2018-08-16  9:04 ` [PATCH android v2] " Zhao Gang

WireGuard Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/wireguard/0 wireguard/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 wireguard wireguard/ https://lore.kernel.org/wireguard \
		wireguard@lists.zx2c4.com zx2c4-wireguard@archiver.kernel.org
	public-inbox-index wireguard


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/com.zx2c4.lists.wireguard


AGPL code for this site: git clone https://public-inbox.org/ public-inbox