Merge pull request #352 from nuclearcat/stability-fixes

Stability fixes and function unification

12 files changed, 68 insertions, 29 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cbce9ec7..3f6bdeec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+### Deprecations
+- `log_pgsql` is deprecated and scheduled for removal. The `LOG_PGSQL` build
+  flag now fails the build; build with `LOG_PGSQL_DEPRECATED=TRUE` to keep it
+  for now, and the module logs a warning on startup. If you depend on it,
+  please object at https://github.com/accel-ppp/accel-ppp/issues, otherwise it
+  will be deleted.
+
 ### Features
 - New `metrics` module: HTTP endpoint exposing the same numbers as `accel-cmd show stat` at `/metrics`, in either Prometheus exposition or JSON format. Configurable listen address and optional IPv4 CIDR allow-list.
 
diff --git a/README.md b/README.md
index a78cb145..dd210758 100644
--- a/README.md
+++ b/README.md
@@ -48,7 +48,7 @@ monitoring kernel modules. Optional features require their corresponding
 development libraries:
 
 * Net-SNMP for NETSNMP=TRUE
-* PostgreSQL client libraries for LOG_PGSQL=TRUE
+* PostgreSQL client libraries for LOG_PGSQL_DEPRECATED=TRUE
 * Lua for LUA=TRUE or a specific Lua version such as LUA=5.3
 
 
@@ -74,7 +74,10 @@ Useful build options:
 * RADIUS=FALSE omits RADIUS support.
 * SHAPER=FALSE omits the traffic-shaping module.
 * NETSNMP=TRUE builds SNMP support.
-* LOG_PGSQL=TRUE builds PostgreSQL logging support.
+* LOG_PGSQL_DEPRECATED=TRUE builds PostgreSQL logging support. The module is
+  deprecated and scheduled for removal, the old LOG_PGSQL flag now fails the
+  build. If you depend on it, please say so at
+  https://github.com/accel-ppp/accel-ppp/issues.
 
 For example, to build the IPoE and VLAN monitoring modules for the running
 kernel:
diff --git a/accel-pppd/accel-ppp.conf b/accel-pppd/accel-ppp.conf
index c32e7612..fe124392 100644
--- a/accel-pppd/accel-ppp.conf
+++ b/accel-pppd/accel-ppp.conf
@@ -2,7 +2,7 @@
 log_file
 #log_syslog
 #log_tcp
-#log_pgsql
+#log_pgsql (deprecated, scheduled for removal)
 
 connlimit
 
diff --git a/accel-pppd/accel-ppp.conf.5 b/accel-pppd/accel-ppp.conf.5
index 7c042541..ad0858e8 100644
--- a/accel-pppd/accel-ppp.conf.5
+++ b/accel-pppd/accel-ppp.conf.5
@@ -36,6 +36,7 @@ This is logging target which logs messages over TCP/IP.
 .TP
 .BI log_pgsql
 This is logging target which logs messages to PostgreSQL.
+Deprecated and scheduled for removal, see the [log-pgsql] section.
 .TP
 .BI pptp
 .br
@@ -1343,6 +1344,10 @@ log all messages including debug messages
 .SH [log-pgsql]
 .br
 Configuration of log_pgsql module.
+.br
+This module is deprecated and scheduled for removal. It is built only when
+accel-ppp is configured with LOG_PGSQL_DEPRECATED=TRUE. If you depend on it,
+please object at https://github.com/accel-ppp/accel-ppp/issues.
 .TP
 .BI "conninfo=" conninfo
 Conninfo to connect to PostgreSQL server.
diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c
index 0cc23180..0e65168d 100644
--- a/accel-pppd/ctrl/pppoe/pppoe.c
+++ b/accel-pppd/ctrl/pppoe/pppoe.c
@@ -24,6 +24,7 @@
 #endif
 
 #include "iputils.h"
+#include "utils.h"
 #include "connlimit.h"
 #include "vlan_mon.h"
 
@@ -33,10 +34,6 @@
 
 #define SID_MAX 65536
 
-#ifndef min
-#define min(x,y) ((x)<(y)?(x):(y))
-#endif
-
 struct pppoe_conn_t {
 	struct list_head entry;
 	struct triton_context_t ctx;
@@ -792,7 +789,7 @@ static int add_tag2(uint8_t *pack, size_t pack_size, const struct pppoe_tag *t)
 {
 	struct pppoe_hdr *hdr = (struct pppoe_hdr *)(pack + ETH_HLEN);
 	struct pppoe_tag *tag = (struct pppoe_tag *)(pack + ETH_HLEN + sizeof(*hdr) + ntohs(hdr->length));
-	if (pack_size <= ETH_HLEN + sizeof(*hdr) + ntohs(hdr->length) + ntohs(t->tag_len) || ntohs(t->tag_len) < 0)
+	if (pack_size <= ETH_HLEN + sizeof(*hdr) + ntohs(hdr->length) + sizeof(*t) + ntohs(t->tag_len))
 		return -1;
 
 	memcpy(tag, t, sizeof(*t) + ntohs(t->tag_len));
@@ -1043,6 +1040,8 @@ static void pppoe_recv_PADI(struct pppoe_serv_t *serv, uint8_t *pack, int size)
 	len = ntohs(hdr->length);
 	for (n = 0; n < len; n += sizeof(*tag) + ntohs(tag->tag_len)) {
 		tag = (struct pppoe_tag *)(pack + ETH_HLEN + sizeof(*hdr) + n);
+		if (n + sizeof(*tag) > len)
+			return;
 		if (n + sizeof(*tag) + ntohs(tag->tag_len) > len)
 			return;
 		switch (ntohs(tag->tag_type)) {
diff --git a/accel-pppd/ctrl/sstp/sstp.c b/accel-pppd/ctrl/sstp/sstp.c
index db08ae93..c3cd111d 100644
--- a/accel-pppd/ctrl/sstp/sstp.c
+++ b/accel-pppd/ctrl/sstp/sstp.c
@@ -50,9 +50,6 @@
 #include "sstp_prot.h"
 #include "if_ppposeq.h"
 
-#ifndef min
-#define min(x,y) ((x) < (y) ? (x) : (y))
-#endif
 #ifndef max
 #define max(x,y) ((x) > (y) ? (x) : (y))
 #endif
diff --git a/accel-pppd/log.c b/accel-pppd/log.c
index e4a1e6ed..7fac66df 100644
--- a/accel-pppd/log.c
+++ b/accel-pppd/log.c
@@ -11,15 +11,12 @@
 #include "triton/mempool.h"
 #include "events.h"
 #include "ppp.h"
+#include "utils.h"
 
 #include "log.h"
 
 #include "memdebug.h"
 
-#ifndef min
-#define min(x,y) ((x)<(y)?(x):(y))
-#endif
-
 #define LOG_MSG   0
 #define LOG_ERROR 1
 #define LOG_WARN  2
diff --git a/accel-pppd/logs/CMakeLists.txt b/accel-pppd/logs/CMakeLists.txt
index 4d8a631a..0052e1ae 100644
--- a/accel-pppd/logs/CMakeLists.txt
+++ b/accel-pppd/logs/CMakeLists.txt
@@ -33,7 +33,15 @@ IF(LOG_SYSLOG)
 	)
 ENDIF(LOG_SYSLOG)
 
+# log_pgsql is deprecated and scheduled for removal, the build flag was
+# renamed so that nobody keeps building it without noticing
 IF(LOG_PGSQL)
+	message(FATAL_ERROR "${Esc}[31mLOG_PGSQL is deprecated.\n The log_pgsql module is scheduled for removal in a future release.\n Build with -DLOG_PGSQL_DEPRECATED=TRUE if you still need it, and please say so at https://github.com/accel-ppp/accel-ppp/issues so that it is kept${Esc}[m")
+ENDIF(LOG_PGSQL)
+
+IF(LOG_PGSQL_DEPRECATED)
+	message(WARNING "${Esc}[33mBuilding deprecated module log_pgsql, it is scheduled for removal in a future release.\n Please say so at https://github.com/accel-ppp/accel-ppp/issues if you need it kept${Esc}[m")
+
 	find_library(PGSQL_LIBRARY pq
 					HINTS ${CMAKE_SOURCE_DIR}/lib/linux/gcc/${BIT}/lib
 					PATHS ${CMAKE_SOURCE_DIR}/lib/linux/gcc/${BIT}/lib
@@ -47,5 +55,5 @@ IF(LOG_PGSQL)
 	INSTALL(TARGETS log_pgsql
 		LIBRARY DESTINATION lib${LIB_SUFFIX}/accel-ppp
 	)
-ENDIF(LOG_PGSQL)
+ENDIF(LOG_PGSQL_DEPRECATED)
 
diff --git a/accel-pppd/logs/log_pgsql.c b/accel-pppd/logs/log_pgsql.c
index e4b5a9e9..6589da0e 100644
--- a/accel-pppd/logs/log_pgsql.c
+++ b/accel-pppd/logs/log_pgsql.c
@@ -1,3 +1,13 @@
+/*
+ * DEPRECATED
+ *
+ * This module is scheduled for removal in a future release. It is built
+ * only when the deprecated LOG_PGSQL_DEPRECATED build flag is given.
+ * If you depend on it, please object at
+ * https://github.com/accel-ppp/accel-ppp/issues, otherwise it will be
+ * deleted.
+ */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -9,6 +19,7 @@
 #include "log.h"
 #include "list.h"
 #include "ap_session.h"
+#include "utils.h"
 
 #include "memdebug.h"
 
@@ -57,20 +68,28 @@ static void unpack_msg(struct log_msg_t *msg)
 
 static void set_hdr(struct log_msg_t *msg, struct ap_session *ses)
 {
+	const char *username = ses && ses->username ? ses->username : "";
+	const char *sessionid = ses && ses->username ? ses->sessionid : "";
 	struct tm tm;
+	int pos, len, avail;
 
 	localtime_r(&msg->timestamp.tv_sec, &tm);
 
 	strftime(msg->hdr->msg, LOG_CHUNK_SIZE, "%Y-%m-%d %H:%M:%S", &tm);
-	msg->hdr->len = strlen(msg->hdr->msg) + 1;
-	if (ses && ses->username) {
-		strcpy(msg->hdr->msg + msg->hdr->len, ses->username);
-		msg->hdr->len += strlen(ses->username) + 1;
-		strcpy(msg->hdr->msg + msg->hdr->len, ses->sessionid);
-		msg->hdr->len += strlen(ses->sessionid) + 1;
-	} else
-		memset(msg->hdr->msg + msg->hdr->len, 0, 2);
+	pos = strlen(msg->hdr->msg) + 1;
+
+	/* username is peer supplied and may be up to 255 bytes long,
+	 * truncate it to what is left of the chunk, keeping one byte
+	 * for the terminator of the sessionid */
+	avail = LOG_CHUNK_SIZE - pos - 1;
+	len = snprintf(msg->hdr->msg + pos, avail, "%s", username);
+	pos += min(len, avail - 1) + 1;
 
+	avail = LOG_CHUNK_SIZE - pos;
+	len = snprintf(msg->hdr->msg + pos, avail, "%s", sessionid);
+	pos += min(len, avail - 1) + 1;
+
+	msg->hdr->len = pos;
 }
 
 static void write_next_msg(void)
@@ -284,6 +303,9 @@ static void init(void)
 {
 	char *opt;
 
+	log_warn("log_pgsql: this module is deprecated and is scheduled for removal,"
+		 " please object at https://github.com/accel-ppp/accel-ppp/issues if you need it\n");
+
 	spinlock_init(&queue_lock);
 
 	opt = conf_get_opt("log-pgsql", "conninfo");
diff --git a/accel-pppd/ppp/ppp_lcp.c b/accel-pppd/ppp/ppp_lcp.c
index 05b6c4a6..ed085b3b 100644
--- a/accel-pppd/ppp/ppp_lcp.c
+++ b/accel-pppd/ppp/ppp_lcp.c
@@ -13,13 +13,10 @@
 #include "ppp_lcp.h"
 #include "events.h"
 #include "iputils.h"
+#include "utils.h"
 
 #include "memdebug.h"
 
-#ifndef min
-#define min(x,y) ((x)<(y)?(x):(y))
-#endif
-
 struct recv_opt_t
 {
 	struct list_head entry;
diff --git a/accel-pppd/utils.h b/accel-pppd/utils.h
index aad4025f..63c1db0d 100644
--- a/accel-pppd/utils.h
+++ b/accel-pppd/utils.h
@@ -4,6 +4,10 @@
 #include <netinet/in.h>
 #include <stdint.h>
 
+#ifndef min
+#define min(x, y) ((x) < (y) ? (x) : (y))
+#endif
+
 char *u_ip6str(const struct in6_addr *addr, char *buf);
 char *u_ip4str(const struct in_addr *addr, char *buf);
 
diff --git a/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild b/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild
index 02bf282f..3edee26f 100644
--- a/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild
+++ b/contrib/gentoo/net-dialup/accel-ppp/accel-ppp-9999.ebuild
@@ -48,7 +48,7 @@ src_configure() {
 	fi
 
 	if  use postgres; then
-		mycmakeargs+=( "-DLOG_PGSQL=TRUE" )
+		mycmakeargs+=( "-DLOG_PGSQL_DEPRECATED=TRUE" )
 	fi
 	
 	if use shaper; then