Merge pull request #3235 from dkg/hwaddr-cleanup

minor improvements for dealing with MAC Addresses

7 files changed, 134 insertions, 11 deletions

diff --git a/man/systemd.network.xml b/man/systemd.network.xml
index d917fe2c12..821e22aff8 100644
--- a/man/systemd.network.xml
+++ b/man/systemd.network.xml
@@ -105,7 +105,8 @@
         <varlistentry>
           <term><varname>MACAddress=</varname></term>
           <listitem>
-            <para>The hardware address.</para>
+            <para>The hardware address of the interface (use full colon-delimited hexadecimal, e.g.,
+            01:23:45:67:89:ab).</para>
           </listitem>
         </varlistentry>
         <varlistentry>
@@ -198,7 +199,7 @@
       <varlistentry>
         <term><varname>MACAddress=</varname></term>
         <listitem>
-          <para>The hardware address.</para>
+          <para>The hardware address to set for the device.</para>
         </listitem>
       </varlistentry>
       <varlistentry>
diff --git a/src/basic/ether-addr-util.c b/src/basic/ether-addr-util.c
index a4d8d656da..5697e8d132 100644
--- a/src/basic/ether-addr-util.c
+++ b/src/basic/ether-addr-util.c
@@ -23,6 +23,7 @@
 
 #include "ether-addr-util.h"
 #include "macro.h"
+#include "string-util.h"
 
 char* ether_addr_to_string(const struct ether_addr *addr, char buffer[ETHER_ADDR_TO_STRING_MAX]) {
         assert(addr);
@@ -54,3 +55,71 @@ bool ether_addr_equal(const struct ether_addr *a, const struct ether_addr *b) {
                 a->ether_addr_octet[4] == b->ether_addr_octet[4] &&
                 a->ether_addr_octet[5] == b->ether_addr_octet[5];
 }
+
+int ether_addr_from_string(const char *s, struct ether_addr *ret, size_t *offset) {
+        size_t pos = 0, n, field;
+        char sep = '\0';
+        const char *hex = HEXDIGITS, *hexoff;
+        size_t x;
+        bool touched;
+
+#define parse_fields(v)                                         \
+        for (field = 0; field < ELEMENTSOF(v); field++) {       \
+                touched = false;                                \
+                for (n = 0; n < (2 * sizeof(v[0])); n++) {      \
+                        if (s[pos] == '\0')                     \
+                                break;                          \
+                        hexoff = strchr(hex, s[pos]);           \
+                        if (hexoff == NULL)                     \
+                                break;                          \
+                        assert(hexoff >= hex);                  \
+                        x = hexoff - hex;                       \
+                        if (x >= 16)                            \
+                                x -= 6; /* A-F */               \
+                        assert(x < 16);                         \
+                        touched = true;                         \
+                        v[field] <<= 4;                         \
+                        v[field] += x;                          \
+                        pos++;                                  \
+                }                                               \
+                if (!touched)                                   \
+                        return -EINVAL;                         \
+                if (field < (ELEMENTSOF(v)-1)) {                \
+                        if (s[pos] != sep)                      \
+                                return -EINVAL;                 \
+                        else                                    \
+                                pos++;                          \
+                }                                               \
+        }
+
+        assert(s);
+        assert(ret);
+
+        sep = s[strspn(s, hex)];
+        if (sep == '\n')
+                return -EINVAL;
+        if (strchr(":.-", sep) == NULL)
+                return -EINVAL;
+
+        if (sep == '.') {
+                uint16_t shorts[3] = { 0 };
+
+                parse_fields(shorts);
+
+                for (n = 0; n < ELEMENTSOF(shorts); n++) {
+                        ret->ether_addr_octet[2*n] = ((shorts[n] & (uint16_t)0xff00) >> 8);
+                        ret->ether_addr_octet[2*n + 1] = (shorts[n] & (uint16_t)0x00ff);
+                }
+        } else {
+                struct ether_addr out = { .ether_addr_octet = { 0 } };
+
+                parse_fields(out.ether_addr_octet);
+
+                for (n = 0; n < ELEMENTSOF(out.ether_addr_octet); n++)
+                        ret->ether_addr_octet[n] = out.ether_addr_octet[n];
+        }
+
+        if (offset)
+                *offset = pos;
+        return 0;
+}
diff --git a/src/basic/ether-addr-util.h b/src/basic/ether-addr-util.h
index 074363793e..74e125a95f 100644
--- a/src/basic/ether-addr-util.h
+++ b/src/basic/ether-addr-util.h
@@ -35,3 +35,5 @@ bool ether_addr_equal(const struct ether_addr *a, const struct ether_addr *b);
 static inline bool ether_addr_is_null(const struct ether_addr *addr) {
         return ether_addr_equal(addr, &ETHER_ADDR_NULL);
 }
+
+int ether_addr_from_string(const char *s, struct ether_addr *ret, size_t *offset);
diff --git a/src/basic/string-util.h b/src/basic/string-util.h
index ad0c813761..139cc8c91b 100644
--- a/src/basic/string-util.h
+++ b/src/basic/string-util.h
@@ -37,6 +37,7 @@
 #define UPPERCASE_LETTERS "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 #define LETTERS           LOWERCASE_LETTERS UPPERCASE_LETTERS
 #define ALPHANUMERICAL    LETTERS DIGITS
+#define HEXDIGITS         DIGITS "abcdefABCDEF"
 
 #define streq(a,b) (strcmp((a),(b)) == 0)
 #define strneq(a, b, n) (strncmp((a), (b), (n)) == 0)
diff --git a/src/libsystemd-network/network-internal.c b/src/libsystemd-network/network-internal.c
index 2badcdff58..046b0f9393 100644
--- a/src/libsystemd-network/network-internal.c
+++ b/src/libsystemd-network/network-internal.c
@@ -27,6 +27,7 @@
 #include "condition.h"
 #include "conf-parser.h"
 #include "dhcp-lease-internal.h"
+#include "ether-addr-util.c"
 #include "hexdecoct.h"
 #include "log.h"
 #include "network-internal.h"
@@ -272,6 +273,8 @@ int config_parse_hwaddr(const char *unit,
                         void *userdata) {
         struct ether_addr **hwaddr = data;
         struct ether_addr *n;
+        const char *start;
+        size_t offset;
         int r;
 
         assert(filename);
@@ -283,14 +286,10 @@ int config_parse_hwaddr(const char *unit,
         if (!n)
                 return log_oom();
 
-        r = sscanf(rvalue, "%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
-                   &n->ether_addr_octet[0],
-                   &n->ether_addr_octet[1],
-                   &n->ether_addr_octet[2],
-                   &n->ether_addr_octet[3],
-                   &n->ether_addr_octet[4],
-                   &n->ether_addr_octet[5]);
-        if (r != 6) {
+        start = rvalue + strspn(rvalue, WHITESPACE);
+        r = ether_addr_from_string(start, n, &offset);
+
+        if (r || (start[offset + strspn(start + offset, WHITESPACE)] != '\0')) {
                 log_syntax(unit, LOG_ERR, filename, line, 0, "Not a valid MAC address, ignoring assignment: %s", rvalue);
                 free(n);
                 return 0;
diff --git a/src/network/test-networkd-conf.c b/src/network/test-networkd-conf.c
index 9bd30b82c6..0e1a18457d 100644
--- a/src/network/test-networkd-conf.c
+++ b/src/network/test-networkd-conf.c
@@ -21,9 +21,11 @@
 #include "log.h"
 #include "macro.h"
 #include "string-util.h"
+#include "ether-addr-util.h"
 
 #include "networkd-conf.h"
 #include "networkd-network.h"
+#include "network-internal.h"
 
 static void test_config_parse_duid_type_one(const char *rvalue, int ret, DUIDType expected) {
         DUIDType actual = 0;
@@ -60,6 +62,21 @@ static void test_config_parse_duid_rawdata_one(const char *rvalue, int ret, cons
         }
 }
 
+static void test_config_parse_hwaddr_one(const char *rvalue, int ret, const struct ether_addr* expected) {
+        struct ether_addr *actual = NULL;
+        int r;
+
+        r = config_parse_hwaddr("network", "filename", 1, "section", 1, "lvalue", 0, rvalue, &actual, NULL);
+        assert_se(ret == r);
+        if (expected) {
+                assert_se(actual);
+                assert(ether_addr_equal(expected, actual));
+        } else {
+                assert_se(actual == NULL);
+        }
+        free(actual);
+}
+
 #define BYTES_0_128 "0:1:2:3:4:5:6:7:8:9:a:b:c:d:e:f:10:11:12:13:14:15:16:17:18:19:1a:1b:1c:1d:1e:1f:20:21:22:23:24:25:26:27:28:29:2a:2b:2c:2d:2e:2f:30:31:32:33:34:35:36:37:38:39:3a:3b:3c:3d:3e:3f:40:41:42:43:44:45:46:47:48:49:4a:4b:4c:4d:4e:4f:50:51:52:53:54:55:56:57:58:59:5a:5b:5c:5d:5e:5f:60:61:62:63:64:65:66:67:68:69:6a:6b:6c:6d:6e:6f:70:71:72:73:74:75:76:77:78:79:7a:7b:7c:7d:7e:7f:80"
 
 #define BYTES_1_128 {0x1,0x2,0x3,0x4,0x5,0x6,0x7,0x8,0x9,0xa,0xb,0xc,0xd,0xe,0xf,0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17,0x18,0x19,0x1a,0x1b,0x1c,0x1d,0x1e,0x1f,0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27,0x28,0x29,0x2a,0x2b,0x2c,0x2d,0x2e,0x2f,0x30,0x31,0x32,0x33,0x34,0x35,0x36,0x37,0x38,0x39,0x3a,0x3b,0x3c,0x3d,0x3e,0x3f,0x40,0x41,0x42,0x43,0x44,0x45,0x46,0x47,0x48,0x49,0x4a,0x4b,0x4c,0x4d,0x4e,0x4f,0x50,0x51,0x52,0x53,0x54,0x55,0x56,0x57,0x58,0x59,0x5a,0x5b,0x5c,0x5d,0x5e,0x5f,0x60,0x61,0x62,0x63,0x64,0x65,0x66,0x67,0x68,0x69,0x6a,0x6b,0x6c,0x6d,0x6e,0x6f,0x70,0x71,0x72,0x73,0x74,0x75,0x76,0x77,0x78,0x79,0x7a,0x7b,0x7c,0x7d,0x7e,0x7f,0x80}
@@ -80,12 +97,46 @@ static void test_config_parse_duid_rawdata(void) {
         test_config_parse_duid_rawdata_one(BYTES_0_128 + 2, 0, &(DUID){0, 128, BYTES_1_128});
 }
 
+static void test_config_parse_hwaddr(void) {
+        const struct ether_addr t[] = {
+                { .ether_addr_octet = { 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff } },
+                { .ether_addr_octet = { 0x01, 0x23, 0x45, 0x67, 0x89, 0xab } },
+        };
+        test_config_parse_hwaddr_one("", 0, NULL);
+        test_config_parse_hwaddr_one("no:ta:ma:ca:dd:re", 0, NULL);
+        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:fx", 0, NULL);
+        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff", 0, &t[0]);
+        test_config_parse_hwaddr_one(" aa:bb:cc:dd:ee:ff", 0, &t[0]);
+        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff \t\n", 0, &t[0]);
+        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff \t\nxxx", 0, NULL);
+        test_config_parse_hwaddr_one("aa:bb:cc: dd:ee:ff", 0, NULL);
+        test_config_parse_hwaddr_one("aa:bb:cc:d d:ee:ff", 0, NULL);
+        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee", 0, NULL);
+        test_config_parse_hwaddr_one("9:aa:bb:cc:dd:ee:ff", 0, NULL);
+        test_config_parse_hwaddr_one("aa:bb:cc:dd:ee:ff:gg", 0, NULL);
+        test_config_parse_hwaddr_one("aa:Bb:CC:dd:ee:ff", 0, &t[0]);
+        test_config_parse_hwaddr_one("01:23:45:67:89:aB", 0, &t[1]);
+        test_config_parse_hwaddr_one("1:23:45:67:89:aB", 0, &t[1]);
+        test_config_parse_hwaddr_one("aa-bb-cc-dd-ee-ff", 0, &t[0]);
+        test_config_parse_hwaddr_one("AA-BB-CC-DD-EE-FF", 0, &t[0]);
+        test_config_parse_hwaddr_one("01-23-45-67-89-ab", 0, &t[1]);
+        test_config_parse_hwaddr_one("aabb.ccdd.eeff", 0, &t[0]);
+        test_config_parse_hwaddr_one("0123.4567.89ab", 0, &t[1]);
+        test_config_parse_hwaddr_one("123.4567.89ab.", 0, NULL);
+        test_config_parse_hwaddr_one("aabbcc.ddeeff", 0, NULL);
+        test_config_parse_hwaddr_one("aabbccddeeff", 0, NULL);
+        test_config_parse_hwaddr_one("aabbccddee:ff", 0, NULL);
+        test_config_parse_hwaddr_one("012345.6789ab", 0, NULL);
+        test_config_parse_hwaddr_one("123.4567.89ab", 0, &t[1]);
+}
+
 int main(int argc, char **argv) {
         log_parse_environment();
         log_open();
 
         test_config_parse_duid_type();
         test_config_parse_duid_rawdata();
+        test_config_parse_hwaddr();
 
         return 0;
 }
diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c
index ec9f173da2..79609765e0 100644
--- a/src/test/test-fileio.c
+++ b/src/test/test-fileio.c
@@ -289,7 +289,7 @@ static void test_capeff(void) {
 
                 assert_se(r == 0);
                 assert_se(*capeff);
-                p = capeff[strspn(capeff, DIGITS "abcdefABCDEF")];
+                p = capeff[strspn(capeff, HEXDIGITS)];
                 assert_se(!p || isspace(p));
         }
 }