ppp: rework host-uniq support to take hex encoded strings

The previous implementation of the "host-uniq" option used plain strings for
passing the value to pppd which made it impossible to specify binary data.

Switch the format to a hex encoded string to support binary data.

Signed-off-by: Jo-Philipp Wich <jow@openwrt.org>

SVN-Revision: 44094

3 files changed, 149 insertions, 49 deletions

diff --git a/package/network/services/ppp/Makefile b/package/network/services/ppp/Makefile
index efb0a76fbb..e9b806db34 100644
--- a/package/network/services/ppp/Makefile
+++ b/package/network/services/ppp/Makefile
@@ -1,5 +1,5 @@
 #
-# Copyright (C) 2006-2014 OpenWrt.org
+# Copyright (C) 2006-2015 OpenWrt.org
 #
 # This is free software, licensed under the GNU General Public License v2.
 # See /LICENSE for more information.
@@ -10,7 +10,7 @@ include $(INCLUDE_DIR)/kernel.mk
 
 PKG_NAME:=ppp
 PKG_VERSION:=2.4.7
-PKG_RELEASE:=4
+PKG_RELEASE:=5
 
 PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
 PKG_SOURCE_URL:=ftp://ftp.samba.org/pub/ppp/
diff --git a/package/network/services/ppp/files/ppp.sh b/package/network/services/ppp/files/ppp.sh
index df404dda39..ba3b412316 100755
--- a/package/network/services/ppp/files/ppp.sh
+++ b/package/network/services/ppp/files/ppp.sh
@@ -113,7 +113,7 @@ proto_pppoe_init_config() {
 	ppp_generic_init_config
 	proto_config_add_string "ac"
 	proto_config_add_string "service"
-	proto_config_add_string host_uniq
+	proto_config_add_string "host_uniq"
 }
 
 proto_pppoe_setup() {
@@ -135,8 +135,8 @@ proto_pppoe_setup() {
 		plugin rp-pppoe.so \
 		${ac:+rp_pppoe_ac "$ac"} \
 		${service:+rp_pppoe_service "$service"} \
-		"nic-$iface" \
-		${host_uniq:+host-uniq "$host_uniq"}
+		${host_uniq:+host-uniq "$host_uniq"} \
+		"nic-$iface"
 }
 
 proto_pppoe_teardown() {
diff --git a/package/network/services/ppp/patches/520-uniq.patch b/package/network/services/ppp/patches/520-uniq.patch
index cbafe4cd5f..54c0d62712 100644
--- a/package/network/services/ppp/patches/520-uniq.patch
+++ b/package/network/services/ppp/patches/520-uniq.patch
@@ -1,20 +1,21 @@
 --- a/pppd/plugins/rp-pppoe/common.c
 +++ b/pppd/plugins/rp-pppoe/common.c
-@@ -121,13 +121,13 @@ sendPADT(PPPoEConnection *conn, char con
+@@ -119,15 +119,11 @@ sendPADT(PPPoEConnection *conn, char con
+     conn->session = 0;
+ 
      /* If we're using Host-Uniq, copy it over */
-     if (conn->useHostUniq) {
- 	PPPoETag hostUniq;
+-    if (conn->useHostUniq) {
+-	PPPoETag hostUniq;
 -	pid_t pid = getpid();
-+	int len = strlen(conn->useHostUniq);
- 	hostUniq.type = htons(TAG_HOST_UNIQ);
+-	hostUniq.type = htons(TAG_HOST_UNIQ);
 -	hostUniq.length = htons(sizeof(pid));
 -	memcpy(hostUniq.payload, &pid, sizeof(pid));
 -	memcpy(cursor, &hostUniq, sizeof(pid) + TAG_HDR_SIZE);
 -	cursor += sizeof(pid) + TAG_HDR_SIZE;
 -	plen += sizeof(pid) + TAG_HDR_SIZE;
-+	hostUniq.length = htons(len);
-+	memcpy(hostUniq.payload, conn->useHostUniq, len);
-+	memcpy(cursor, &hostUniq, len + TAG_HDR_SIZE);
++    if (conn->hostUniq.length) {
++	int len = ntohs(conn->hostUniq.length);
++	memcpy(cursor, &conn->hostUniq, len + TAG_HDR_SIZE);
 +	cursor += len + TAG_HDR_SIZE;
 +	plen += len + TAG_HDR_SIZE;
      }
@@ -22,65 +23,86 @@
      /* Copy error message */
 --- a/pppd/plugins/rp-pppoe/discovery.c
 +++ b/pppd/plugins/rp-pppoe/discovery.c
-@@ -104,7 +104,7 @@ parseForHostUniq(UINT16_t type, UINT16_t
+@@ -80,13 +80,10 @@ static void
+ parseForHostUniq(UINT16_t type, UINT16_t len, unsigned char *data,
+ 		 void *extra)
+ {
+-    int *val = (int *) extra;
+-    if (type == TAG_HOST_UNIQ && len == sizeof(pid_t)) {
+-	pid_t tmp;
+-	memcpy(&tmp, data, len);
+-	if (tmp == getpid()) {
+-	    *val = 1;
+-	}
++    PPPoETag *tag = extra;
++
++    if (type == TAG_HOST_UNIQ && len == ntohs(tag->length)) {
++	tag->length = memcmp(data, tag->payload, len);
+     }
+ }
+ 
+@@ -104,16 +101,16 @@ parseForHostUniq(UINT16_t type, UINT16_t
  static int
  packetIsForMe(PPPoEConnection *conn, PPPoEPacket *packet)
  {
 -    int forMe = 0;
-+    char *uniq = conn->useHostUniq;
++    PPPoETag hostUniq = conn->hostUniq;
  
      /* If packet is not directed to our MAC address, forget it */
      if (memcmp(packet->ethHdr.h_dest, conn->myEth, ETH_ALEN)) return 0;
-@@ -112,8 +112,8 @@ packetIsForMe(PPPoEConnection *conn, PPP
+ 
      /* If we're not using the Host-Unique tag, then accept the packet */
-     if (!conn->useHostUniq) return 1;
+-    if (!conn->useHostUniq) return 1;
++    if (!conn->hostUniq.length) return 1;
  
 -    parsePacket(packet, parseForHostUniq, &forMe);
 -    return forMe;
-+    parsePacket(packet, parseForHostUniq, &uniq);
-+    return uniq != 0;
++    parsePacket(packet, parseForHostUniq, &hostUniq);
++    return (hostUniq.length == 0);
  }
  
  /**********************************************************************
-@@ -303,14 +303,14 @@ sendPADI(PPPoEConnection *conn)
+@@ -301,16 +298,12 @@ sendPADI(PPPoEConnection *conn)
+     }
+ 
      /* If we're using Host-Uniq, copy it over */
-     if (conn->useHostUniq) {
- 	PPPoETag hostUniq;
+-    if (conn->useHostUniq) {
+-	PPPoETag hostUniq;
 -	pid_t pid = getpid();
-+	int len = strlen(conn->useHostUniq);
- 	hostUniq.type = htons(TAG_HOST_UNIQ);
+-	hostUniq.type = htons(TAG_HOST_UNIQ);
 -	hostUniq.length = htons(sizeof(pid));
 -	memcpy(hostUniq.payload, &pid, sizeof(pid));
 -	CHECK_ROOM(cursor, packet.payload, sizeof(pid) + TAG_HDR_SIZE);
 -	memcpy(cursor, &hostUniq, sizeof(pid) + TAG_HDR_SIZE);
 -	cursor += sizeof(pid) + TAG_HDR_SIZE;
 -	plen += sizeof(pid) + TAG_HDR_SIZE;
-+	hostUniq.length = htons(len);
-+	memcpy(hostUniq.payload, conn->useHostUniq, len);
++    if (conn->hostUniq.length) {
++	int len = ntohs(conn->hostUniq.length);
 +	CHECK_ROOM(cursor, packet.payload, len + TAG_HDR_SIZE);
-+	memcpy(cursor, &hostUniq, len + TAG_HDR_SIZE);
++	memcpy(cursor, &conn->hostUniq, len + TAG_HDR_SIZE);
 +	cursor += len + TAG_HDR_SIZE;
 +	plen += len + TAG_HDR_SIZE;
      }
  
      /* Add our maximum MTU/MRU */
-@@ -480,14 +480,14 @@ sendPADR(PPPoEConnection *conn)
+@@ -478,16 +471,12 @@ sendPADR(PPPoEConnection *conn)
+     cursor += namelen + TAG_HDR_SIZE;
+ 
      /* If we're using Host-Uniq, copy it over */
-     if (conn->useHostUniq) {
- 	PPPoETag hostUniq;
+-    if (conn->useHostUniq) {
+-	PPPoETag hostUniq;
 -	pid_t pid = getpid();
-+	int len = strlen(conn->useHostUniq);
- 	hostUniq.type = htons(TAG_HOST_UNIQ);
+-	hostUniq.type = htons(TAG_HOST_UNIQ);
 -	hostUniq.length = htons(sizeof(pid));
 -	memcpy(hostUniq.payload, &pid, sizeof(pid));
 -	CHECK_ROOM(cursor, packet.payload, sizeof(pid)+TAG_HDR_SIZE);
 -	memcpy(cursor, &hostUniq, sizeof(pid) + TAG_HDR_SIZE);
 -	cursor += sizeof(pid) + TAG_HDR_SIZE;
 -	plen += sizeof(pid) + TAG_HDR_SIZE;
-+	hostUniq.length = htons(len);
-+	memcpy(hostUniq.payload, conn->useHostUniq, len);
++    if (conn->hostUniq.length) {
++	int len = ntohs(conn->hostUniq.length);
 +	CHECK_ROOM(cursor, packet.payload, len+TAG_HDR_SIZE);
-+	memcpy(cursor, &hostUniq, len + TAG_HDR_SIZE);
++	memcpy(cursor, &conn->hostUniq, len + TAG_HDR_SIZE);
 +	cursor += len + TAG_HDR_SIZE;
 +	plen += len + TAG_HDR_SIZE;
      }
@@ -105,28 +127,59 @@
      { NULL }
  };
  int (*OldDevnameHook)(char *cmd, char **argv, int doit) = NULL;
-@@ -107,7 +110,7 @@ PPPOEInitDevice(void)
+@@ -107,7 +110,6 @@ PPPOEInitDevice(void)
      conn->ifName = devnam;
      conn->discoverySocket = -1;
      conn->sessionSocket = -1;
 -    conn->useHostUniq = 1;
-+    conn->useHostUniq = NULL;
      conn->printACNames = printACNames;
      conn->discoveryTimeout = PADI_TIMEOUT;
      return 1;
-@@ -163,6 +166,9 @@ PPPOEConnectDevice(void)
+@@ -163,6 +165,9 @@ PPPOEConnectDevice(void)
      if (lcp_wantoptions[0].mru > ifr.ifr_mtu - TOTAL_OVERHEAD)
  	lcp_wantoptions[0].mru = ifr.ifr_mtu - TOTAL_OVERHEAD;
  
-+    if(host_uniq)
-+	conn->useHostUniq = host_uniq;
++    if (host_uniq && !parseHostUniq(host_uniq, &conn->hostUniq))
++	fatal("Illegal value for host-uniq option");
 +
      conn->acName = acName;
      conn->serviceName = pppd_pppoe_service;
      strlcpy(ppp_devnam, devnam, sizeof(ppp_devnam));
 --- a/pppd/plugins/rp-pppoe/pppoe-discovery.c
 +++ b/pppd/plugins/rp-pppoe/pppoe-discovery.c
-@@ -641,7 +641,7 @@ int main(int argc, char *argv[])
+@@ -344,7 +344,7 @@ packetIsForMe(PPPoEConnection *conn, PPP
+     if (memcmp(packet->ethHdr.h_dest, conn->myEth, ETH_ALEN)) return 0;
+ 
+     /* If we're not using the Host-Unique tag, then accept the packet */
+-    if (!conn->useHostUniq) return 1;
++    if (!conn->hostUniq.length) return 1;
+ 
+     parsePacket(packet, parseForHostUniq, &forMe);
+     return forMe;
+@@ -470,16 +470,12 @@ sendPADI(PPPoEConnection *conn)
+     cursor += namelen + TAG_HDR_SIZE;
+ 
+     /* If we're using Host-Uniq, copy it over */
+-    if (conn->useHostUniq) {
+-	PPPoETag hostUniq;
+-	pid_t pid = getpid();
+-	hostUniq.type = htons(TAG_HOST_UNIQ);
+-	hostUniq.length = htons(sizeof(pid));
+-	memcpy(hostUniq.payload, &pid, sizeof(pid));
+-	CHECK_ROOM(cursor, packet.payload, sizeof(pid) + TAG_HDR_SIZE);
+-	memcpy(cursor, &hostUniq, sizeof(pid) + TAG_HDR_SIZE);
+-	cursor += sizeof(pid) + TAG_HDR_SIZE;
+-	plen += sizeof(pid) + TAG_HDR_SIZE;
++    if (conn->hostUniq.length) {
++	int len = ntohs(conn->hostUniq.length);
++	CHECK_ROOM(cursor, packet.payload, len + TAG_HDR_SIZE);
++	memcpy(cursor, &conn->hostUniq, len + TAG_HDR_SIZE);
++	cursor += len + TAG_HDR_SIZE;
++	plen += len + TAG_HDR_SIZE;
+     }
+ 
+     packet.length = htons(plen);
+@@ -641,7 +637,7 @@ int main(int argc, char *argv[])
  
      memset(conn, 0, sizeof(PPPoEConnection));
  
@@ -135,35 +188,82 @@
  	switch(opt) {
  	case 'S':
  	    conn->serviceName = xstrdup(optarg);
-@@ -650,7 +650,19 @@ int main(int argc, char *argv[])
+@@ -650,7 +646,23 @@ int main(int argc, char *argv[])
  	    conn->acName = xstrdup(optarg);
  	    break;
  	case 'U':
 -	    conn->useHostUniq = 1;
-+	    if(conn->useHostUniq) {
++	    if(conn->hostUniq.length) {
 +		fprintf(stderr, "-U and -W are mutually exclusive\n");
 +		exit(EXIT_FAILURE);
 +	    }
-+	    conn->useHostUniq = malloc(12);
-+	    snprintf(conn->useHostUniq, 12, "%d", getpid());
++            char pidbuf[5];
++            snprintf(pidbuf, sizeof(pidbuf), "%04x", getpid());
++            parseHostUniq(pidbuf, &conn->hostUniq);
 +	    break;
 +	case 'W':
-+	    if(conn->useHostUniq) {
++	    if(conn->hostUniq.length) {
 +		fprintf(stderr, "-U and -W are mutually exclusive\n");
 +		exit(EXIT_FAILURE);
 +	    }
-+	    conn->useHostUniq = xstrdup(optarg);
++	    if (!parseHostUniq(optarg, &conn->hostUniq)) {
++                fprintf(stderr, "Invalid host-uniq argument: %s\n", optarg);
++                exit(EXIT_FAILURE);
++            }
  	    break;
  	case 'D':
  	    conn->debugFile = fopen(optarg, "w");
 --- a/pppd/plugins/rp-pppoe/pppoe.h
 +++ b/pppd/plugins/rp-pppoe/pppoe.h
-@@ -224,7 +224,7 @@ typedef struct PPPoEConnectionStruct {
+@@ -21,6 +21,8 @@
+ 
+ #include <stdio.h>		/* For FILE */
+ #include <sys/types.h>		/* For pid_t */
++#include <ctype.h>
++#include <string.h>
+ 
+ /* How do we access raw Ethernet devices? */
+ #undef USE_LINUX_PACKET
+@@ -224,7 +226,7 @@ typedef struct PPPoEConnectionStruct {
      char *serviceName;		/* Desired service name, if any */
      char *acName;		/* Desired AC name, if any */
      int synchronous;		/* Use synchronous PPP */
 -    int useHostUniq;		/* Use Host-Uniq tag */
-+    char *useHostUniq;		/* Use Host-Uniq tag */
++    PPPoETag hostUniq;		/* Use Host-Uniq tag */
      int printACNames;		/* Just print AC names */
      FILE *debugFile;		/* Debug file for dumping packets */
      int numPADOs;		/* Number of PADO packets received */
+@@ -280,6 +282,33 @@ void pppoe_printpkt(PPPoEPacket *packet,
+ 		    void (*printer)(void *, char *, ...), void *arg);
+ void pppoe_log_packet(const char *prefix, PPPoEPacket *packet);
+ 
++static inline int parseHostUniq(const char *uniq, PPPoETag *tag)
++{
++    int i, len = strlen(uniq);
++
++#define hex(x) \
++    (((x) <= '9') ? ((x) - '0') : \
++        (((x) <= 'F') ? ((x) - 'A' + 10) : \
++            ((x) - 'a' + 10)))
++
++    if (len % 2)
++        return 0;
++
++    for (i = 0; i < len; i += 2)
++    {
++        if (!isxdigit(uniq[i]) || !isxdigit(uniq[i+1]))
++            return 0;
++
++        tag->payload[i / 2] = (char)(16 * hex(uniq[i]) + hex(uniq[i+1]));
++    }
++
++#undef hex
++
++    tag->type = htons(TAG_HOST_UNIQ);
++    tag->length = htons(len / 2);
++    return 1;
++}
++
+ #define SET_STRING(var, val) do { if (var) free(var); var = strDup(val); } while(0);
+ 
+ #define CHECK_ROOM(cursor, start, len) \