Move snmp packaging and unpackaging to their own function.

In preparation for SNMPv3/USM support.

Feedback and OK jmatthew@

3 files changed, 130 insertions, 54 deletions

diff --git a/usr.bin/snmp/snmp.c b/usr.bin/snmp/snmp.c
index 7fac77794a7..af2e66d82f7 100644
--- a/usr.bin/snmp/snmp.c
+++ b/usr.bin/snmp/snmp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: snmp.c,v 1.2 2019/08/27 06:14:28 martijn Exp $	*/
+/*	$OpenBSD: snmp.c,v 1.3 2019/09/18 09:44:38 martijn Exp $	*/
 
 /*
  * Copyright (c) 2019 Martijn van Duren <martijn@openbsd.org>
@@ -32,6 +32,10 @@
 
 static struct ber_element *
     snmp_resolve(struct snmp_agent *, struct ber_element *, int);
+static char *
+    snmp_package(struct snmp_agent *, struct ber_element *, size_t *);
+static struct ber_element *
+    snmp_unpackage(struct snmp_agent *, char *, size_t);
 
 struct snmp_agent *
 snmp_connect_v12(int fd, enum snmp_version version, const char *community)
@@ -171,19 +175,16 @@ fail:
 static struct ber_element *
 snmp_resolve(struct snmp_agent *agent, struct ber_element *pdu, int reply)
 {
-	struct ber_element *message, *varbind;
+	struct ber_element *varbind;
 	struct ber_oid oid;
 	struct timespec start, now;
 	struct pollfd pfd;
-	struct ber ber;
+	char *message;
 	ssize_t len;
 	long long reqid, rreqid;
-	long long version;
-	char *community;
 	short direction;
 	int to, nfds, ret;
 	int tries;
-	void *ptr;
 	char buf[READ_BUF_SIZE];
 
 	if (ber_scanf_elements(pdu, "{i", &reqid) != 0) {
@@ -192,23 +193,8 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element *pdu, int reply)
 		return NULL;
 	}
 
-	if ((message = ber_add_sequence(NULL)) == NULL) {
-		ber_free_elements(pdu);
+	if ((message = snmp_package(agent, pdu, &len)) == NULL)
 		return NULL;
-	}
-	if (ber_printf_elements(message, "dse", agent->version,
-	    agent->community, pdu) == NULL) {
-		ber_free_elements(pdu);
-		ber_free_elements(message);
-		return NULL;
-	}
-	memset(&ber, 0, sizeof(ber));
-	ber_set_application(&ber, smi_application);
-	len = ber_write_elements(&ber, message);
-	ber_free_elements(message);
-	message = NULL;
-	if (ber_get_writebuf(&ber, &ptr) < 1)
-		goto fail;
 
 	clock_gettime(CLOCK_MONOTONIC, &start);
 	memcpy(&now, &start, sizeof(now));
@@ -236,7 +222,7 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element *pdu, int reply)
 				goto fail;
 		}
 		if (direction == POLLOUT) {
-			ret = send(agent->fd, ptr, len, MSG_DONTWAIT);
+			ret = send(agent->fd, message, len, MSG_DONTWAIT);
 			if (ret == -1)
 				goto fail;
 			if (ret < len) {
@@ -253,25 +239,10 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element *pdu, int reply)
 			errno = ECONNRESET;
 		if (ret <= 0)
 			goto fail;
-		ber_set_readbuf(&ber, buf, ret);
-		if ((message = ber_read_elements(&ber, NULL)) == NULL) {
-			direction = POLLOUT;
+		if ((pdu = snmp_unpackage(agent, buf, ret)) == NULL) {
 			tries--;
-			continue;
-		}
-		if (ber_scanf_elements(message, "{ise", &version, &community,
-		    &pdu) != 0) {
-			errno = EPROTO;
 			direction = POLLOUT;
-			tries--;
-			continue;
-		}
-		/* Skip invalid packets; should not happen */
-		if (version != agent->version ||
-		    strcmp(community, agent->community) != 0) {
 			errno = EPROTO;
-			direction = POLLOUT;
-			tries--;
 			continue;
 		}
 		/* Validate pdu format and check request id */
@@ -297,17 +268,112 @@ snmp_resolve(struct snmp_agent *agent, struct ber_element *pdu, int reply)
 				break;
 			}
 		}
-		if (varbind != NULL)
-			continue;
 
-		ber_unlink_elements(message->be_sub->be_next);
-		ber_free_elements(message);
-		ber_free(&ber);
+		free(message);
 		return pdu;
 	}
 
 fail:
+	free(message);
+	return NULL;
+}
+
+static char *
+snmp_package(struct snmp_agent *agent, struct ber_element *pdu, size_t *len)
+{
+	struct ber ber;
+	struct ber_element *message;
+	ssize_t ret;
+	char *packet = NULL;
+
+	bzero(&ber, sizeof(ber));
+	ber_set_application(&ber, smi_application);
+
+	if ((message = ber_add_sequence(NULL)) == NULL) {
+		ber_free_elements(pdu);
+		goto fail;
+	}
+
+	switch (agent->version) {
+	case SNMP_V1:
+	case SNMP_V2C:
+		if (ber_printf_elements(message, "dse", agent->version,
+		    agent->community, pdu) == NULL) {
+			ber_free_elements(pdu);
+			goto fail;
+		}
+		break;
+	case SNMP_V3:
+		break;
+	}
+
+	if (ber_write_elements(&ber, message) == -1)
+		goto fail;
+	ret = ber_copy_writebuf(&ber, (void **)&packet);
+
+	*len = (size_t) ret;
+	ber_free(&ber);
+
+fail:
 	ber_free_elements(message);
+	return packet;
+}
+
+static struct ber_element *
+snmp_unpackage(struct snmp_agent *agent, char *buf, size_t buflen)
+{
+	struct ber ber;
+	enum snmp_version version;
+	char *community;
+	struct ber_element *pdu;
+	struct ber_element *message = NULL, *payload;
+
+	bzero(&ber, sizeof(ber));
+	ber_set_application(&ber, smi_application);
+
+	ber_set_readbuf(&ber, buf, buflen);
+	if ((message = ber_read_elements(&ber, NULL)) == NULL)
+		return NULL;
 	ber_free(&ber);
+
+	if (ber_scanf_elements(message, "{de", &version, &payload) != 0)
+		goto fail;
+
+	if (version != agent->version)
+		goto fail;
+
+	switch (version)
+	{
+	case SNMP_V1:
+	case SNMP_V2C:
+		if (ber_scanf_elements(payload, "se", &community, &pdu) == -1)
+			goto fail;
+		if (strcmp(community, agent->community) != 0)
+			goto fail;
+		ber_unlink_elements(payload);
+		ber_free_elements(message);
+		return pdu;
+	case SNMP_V3:
+		break;
+	}
+	/* NOTREACHED */
+
+fail:
+	ber_free_elements(message);
 	return NULL;
 }
+
+ssize_t
+ber_copy_writebuf(struct ber *ber, void **buf)
+{
+	char *bbuf;
+	ssize_t ret;
+
+	*buf = NULL;
+	if ((ret = ber_get_writebuf(ber, (void **)&bbuf)) == -1)
+		return -1;
+	if ((*buf = malloc(ret)) == NULL)
+		return -1;
+	memcpy(*buf, bbuf, ret);
+	return  ret;
+}
diff --git a/usr.bin/snmp/snmp.h b/usr.bin/snmp/snmp.h
index 502aa75df6e..81034c95096 100644
--- a/usr.bin/snmp/snmp.h
+++ b/usr.bin/snmp/snmp.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: snmp.h,v 1.1 2019/08/09 06:17:59 martijn Exp $	*/
+/*	$OpenBSD: snmp.h,v 1.2 2019/09/18 09:44:38 martijn Exp $	*/
 
 /*
  * Copyright (c) 2019 Martijn van Duren <martijn@openbsd.org>
@@ -133,4 +133,6 @@ struct ber_element *
 int snmp_trap(struct snmp_agent *, struct timespec *, struct ber_oid *,
     struct ber_element *);
 
+ssize_t ber_copy_writebuf(struct ber *, void **);
+
 #endif /* SNMPD_SNMP_H */
diff --git a/usr.bin/snmp/snmpc.c b/usr.bin/snmp/snmpc.c
index dc48b10ce16..d49c272d6bc 100644
--- a/usr.bin/snmp/snmpc.c
+++ b/usr.bin/snmp/snmpc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: snmpc.c,v 1.7 2019/08/14 14:40:23 deraadt Exp $	*/
+/*	$OpenBSD: snmpc.c,v 1.8 2019/09/18 09:44:38 martijn Exp $	*/
 
 /*
  * Copyright (c) 2019 Martijn van Duren <martijn@openbsd.org>
@@ -45,6 +45,7 @@ int snmpc_get(int, char *[]);
 int snmpc_walk(int, char *[]);
 int snmpc_trap(int, char *[]);
 int snmpc_mibtree(int, char *[]);
+struct snmp_agent *snmpc_connect(char *, char *);
 int snmpc_parseagent(char *, char *);
 int snmpc_print(struct ber_element *);
 __dead void snmpc_printerror(enum snmp_error, char *);
@@ -304,9 +305,7 @@ snmpc_get(int argc, char *argv[])
 	if (argc < 2)
 		usage();
 
-	agent = snmp_connect_v12(snmpc_parseagent(argv[0], "161"), version,
-	    community);
-	if (agent == NULL)
+	if ((agent = snmpc_connect(argv[0], "161")) == NULL)
 		err(1, "%s", snmp_app->name);
 	agent->timeout = timeout;
 	agent->retries = retries;
@@ -372,8 +371,7 @@ snmpc_walk(int argc, char *argv[])
 		usage();
 	oids = argc == 1 ? mib : argv[1];
 
-	agent = snmp_connect_v12(snmpc_parseagent(argv[0], "161"), version, community);
-	if (agent == NULL)
+	if ((agent = snmpc_connect(argv[0], "161"))== NULL)
 		err(1, "%s", snmp_app->name);
 	agent->timeout = timeout;
 	agent->retries = retries;
@@ -495,9 +493,7 @@ snmpc_trap(int argc, char *argv[])
 	if (version == SNMP_V1)
 		errx(1, "trap is not supported for snmp v1");
 
-	agent = snmp_connect_v12(snmpc_parseagent(argv[0], "162"),
-	    version, community);
-	if (agent == NULL)
+	if ((agent = snmpc_connect(argv[0], "162")) == NULL)
 		err(1, "%s", snmp_app->name);
 
 	if (pledge("stdio", NULL) == -1)
@@ -693,6 +689,18 @@ snmpc_mibtree(int argc, char *argv[])
 	return 0;
 }
 
+struct snmp_agent *
+snmpc_connect(char *host, char *port)
+{
+	switch (version) {
+	case SNMP_V1:
+	case SNMP_V2C:
+		return snmp_connect_v12(snmpc_parseagent(host, port), version,
+		    community);
+	}
+	return NULL;
+}
+
 int
 snmpc_print(struct ber_element *elm)
 {