Move duid and ipv6 secret file reading code into a common function.

common.c

@@ -276,7 +276,7 @@ addvard(struct dhcpcd_ctx *ctx,
 }
 
 char *
-hwaddr_ntoa(const unsigned char *hwaddr, size_t hwlen, char *buf, size_t buflen)
+hwaddr_ntoa(const uint8_t *hwaddr, size_t hwlen, char *buf, size_t buflen)
 {
 	char *p;
 	size_t i;
@@ -301,16 +301,18 @@ hwaddr_ntoa(const unsigned char *hwaddr, size_t hwlen, char *buf, size_t buflen)
 }
 
 size_t
-hwaddr_aton(unsigned char *buffer, const char *addr)
+hwaddr_aton(uint8_t *buffer, const char *addr)
 {
 	char c[3];
 	const char *p = addr;
-	unsigned char *bp = buffer;
+	uint8_t *bp = buffer;
 	size_t len = 0;
 
 	c[2] = '\0';
 	while (*p) {
 		c[0] = *p++;
+		if (c[0] == '\n')
+			continue;
 		c[1] = *p++;
 		/* Ensure that digits are hex */
 		if (isxdigit((unsigned char)c[0]) == 0 ||
@@ -325,15 +327,44 @@ hwaddr_aton(unsigned char *buffer, const char *addr)
 			return 0;
 		}
 		/* Ensure that next data is EOL or a seperator with data */
-		if (!(*p == '\0' || (*p == ':' && *(p + 1) != '\0'))) {
+		if (!(*p == '\0' || *p == '\n' ||
+		    (*p == ':' && *(p + 1) != '\0')))
+		{
 			errno = EINVAL;
 			return 0;
 		}
 		if (*p)
 			p++;
 		if (bp)
-			*bp++ = (unsigned char)strtol(c, NULL, 16);
+			*bp++ = (uint8_t)strtol(c, NULL, 16);
 		len++;
 	}
 	return len;
 }
+
+size_t
+read_hwaddr_aton(uint8_t **data, const char *path)
+{
+	FILE *fp;
+	char *buf;
+	size_t buf_len, len;
+	ssize_t llen;
+
+	if ((fp = fopen(path, "r")) == NULL)
+		return 0;
+
+	buf = NULL;
+	buf_len = len = 0;
+	*data = NULL;
+	while ((llen = getline(&buf, &buf_len, fp)) != -1) {
+		if ((len = hwaddr_aton(NULL, buf)) != 0) {
+			if ((*data = malloc(len)) != NULL)
+				len = hwaddr_aton(*data, buf);
+			else
+				len = 0;
+			break;
+		}
+	}
+	fclose(fp);
+	return len;
+}

common.h

@@ -193,6 +193,7 @@ ssize_t addvar(struct dhcpcd_ctx *,
 ssize_t addvard(struct dhcpcd_ctx *,
     char ***, const char *, const char *, size_t);
 
-char *hwaddr_ntoa(const unsigned char *, size_t, char *, size_t);
-size_t hwaddr_aton(unsigned char *, const char *);
+char *hwaddr_ntoa(const uint8_t *, size_t, char *, size_t);
+size_t hwaddr_aton(uint8_t *, const char *);
+size_t read_hwaddr_aton(uint8_t **, const char *);
 #endif

dhcpcd.h

@@ -164,7 +164,7 @@ struct dhcpcd_ctx {
 	size_t opt_buffer_len;
 #endif
 #ifdef INET6
-	unsigned char secret[SECRET_LEN];
+	uint8_t *secret;
 	size_t secret_len;
 
 	struct dhcp_opt *nd_opts;

duid.c

@@ -54,9 +54,9 @@
 #include "duid.h"
 
 static size_t
-duid_make(unsigned char *d, const struct interface *ifp, uint16_t type)
+duid_make(uint8_t *d, const struct interface *ifp, uint16_t type)
 {
-	unsigned char *p;
+	uint8_t *p;
 	uint16_t u16;
 	time_t t;
 	uint32_t u32;
@@ -84,37 +84,33 @@ duid_make(unsigned char *d, const struct interface *ifp, uint16_t type)
 
 #define DUID_STRLEN DUID_LEN * 3
 static size_t
-duid_get(unsigned char *d, const struct interface *ifp)
+duid_get(uint8_t **d, const struct interface *ifp)
 {
 	FILE *fp;
+	uint8_t *data;
+	size_t len;
 	int x = 0;
-	size_t len = 0;
 	char line[DUID_STRLEN];
 	const struct interface *ifp2;
 
 	/* If we already have a DUID then use it as it's never supposed
 	 * to change once we have one even if the interfaces do */
-	if ((fp = fopen(DUID, "r"))) {
-		while (fgets(line, DUID_STRLEN, fp)) {
-			len = strlen(line);
-			if (len) {
-				if (line[len - 1] == '\n')
-					line[len - 1] = '\0';
-			}
-			len = hwaddr_aton(NULL, line);
-			if (len && len <= DUID_LEN) {
-				hwaddr_aton(d, line);
-				break;
-			}
-			len = 0;
-		}
-		fclose(fp);
-		if (len)
+	if ((len = read_hwaddr_aton(&data, DUID)) != 0) {
+		if (len <= DUID_LEN) {
+			*d = data;
 			return len;
+		}
+		logger(ifp->ctx, LOG_ERR,
+		    "DUID too big (max %u): %s", DUID_LEN, DUID);
+		/* Keep the buffer, will assign below. */
 	} else {
 		if (errno != ENOENT)
 			logger(ifp->ctx, LOG_ERR,
 			    "error reading DUID: %s: %m", DUID);
+		if ((data = malloc(DUID_LEN)) == NULL) {
+			logger(ifp->ctx, LOG_ERR, "%s: malloc: %m", __func__);
+			return 0;
+		}
 	}
 
 	/* No file? OK, lets make one based on our interface */
@@ -133,23 +129,24 @@ duid_get(unsigned char *d, const struct interface *ifp)
 		} else {
 			logger(ifp->ctx, LOG_WARNING,
 			    "no interfaces have a fixed hardware address");
-			return duid_make(d, ifp, DUID_LL);
+			return duid_make(data, ifp, DUID_LL);
 		}
 	}
 
+	*d = data;
 	if (!(fp = fopen(DUID, "w"))) {
 		logger(ifp->ctx, LOG_ERR, "error writing DUID: %s: %m", DUID);
-		return duid_make(d, ifp, DUID_LL);
+		return duid_make(data, ifp, DUID_LL);
 	}
-	len = duid_make(d, ifp, DUID_LLT);
-	x = fprintf(fp, "%s\n", hwaddr_ntoa(d, len, line, sizeof(line)));
+	len = duid_make(data, ifp, DUID_LLT);
+	x = fprintf(fp, "%s\n", hwaddr_ntoa(data, len, line, sizeof(line)));
 	if (fclose(fp) == EOF)
 		x = -1;
 	/* Failed to write the duid? scrub it, we cannot use it */
 	if (x < 1) {
 		logger(ifp->ctx, LOG_ERR, "error writing DUID: %s: %m", DUID);
 		unlink(DUID);
-		return duid_make(d, ifp, DUID_LL);
+		return duid_make(data, ifp, DUID_LL);
 	}
 	return len;
 }
@@ -157,13 +154,7 @@ duid_get(unsigned char *d, const struct interface *ifp)
 size_t duid_init(const struct interface *ifp)
 {
 
-	if (ifp->ctx->duid == NULL) {
-		ifp->ctx->duid = malloc(DUID_LEN);
-		if (ifp->ctx->duid == NULL) {
-			logger(ifp->ctx, LOG_ERR, "%s: %m", __func__);
-			return 0;
-		}
-		ifp->ctx->duid_len = duid_get(ifp->ctx->duid, ifp);
-	}
+	if (ifp->ctx->duid == NULL)
+		ifp->ctx->duid_len = duid_get(&ifp->ctx->duid, ifp);
 	return ifp->ctx->duid_len;
 }

ipv6.c

@@ -179,43 +179,28 @@ ipv6_readsecret(struct dhcpcd_ctx *ctx)
 	uint32_t r;
 	int x;
 
-	if ((fp = fopen(SECRET, "r"))) {
-		len = 0;
-		while (fgets(line, sizeof(line), fp)) {
-			len = strlen(line);
-			if (len) {
-				if (line[len - 1] == '\n')
-					line[len - 1] = '\0';
-			}
-			len = hwaddr_aton(NULL, line);
-			if (len) {
-				ctx->secret_len = hwaddr_aton(ctx->secret,
-				    line);
-				break;
-			}
-			len = 0;
-		}
-		fclose(fp);
-		if (len)
-			return (ssize_t)len;
-	} else {
-		if (errno != ENOENT)
-			logger(ctx, LOG_ERR,
-			    "error reading secret: %s: %m", SECRET);
-	}
+	if ((ctx->secret_len = read_hwaddr_aton(&ctx->secret, SECRET)) != 0)
+		return (ssize_t)ctx->secret_len;
+
+	if (errno != ENOENT)
+		logger(ctx, LOG_ERR, "error reading secret: %s: %m", SECRET);
 
 	/* Chaining arc4random should be good enough.
 	 * RFC7217 section 5.1 states the key SHOULD be at least 128 bits.
 	 * To attempt and future proof ourselves, we'll generate a key of
 	 * 512 bits (64 bytes). */
+	if (ctx->secret_len < 64) {
+		if ((ctx->secret = malloc(64)) == NULL) {
+			logger(ctx, LOG_ERR, "%s: malloc: %m", __func__);
+			return -1;
+		}
+		ctx->secret_len = 64;
+	}
 	p = ctx->secret;
-	ctx->secret_len = 0;
 	for (len = 0; len < 512 / NBBY; len += sizeof(r)) {
 		r = arc4random();
 		memcpy(p, &r, sizeof(r));
 		p += sizeof(r);
-		ctx->secret_len += sizeof(r);
-
 	}
 
 	/* Ensure that only the dhcpcd user can read the secret.
@@ -1514,6 +1499,7 @@ ipv6_ctxfree(struct dhcpcd_ctx *ctx)
 	if (ctx->ipv6 == NULL)
 		return;
 
+	free(ctx->secret);
 	ipv6_freerts(ctx->ipv6->routes);
 	free(ctx->ipv6->routes);
 	free(ctx->ipv6->ra_routers);