Refactoring

Makefile

@@ -10,7 +10,7 @@ MANDIR     	:= $(DESTDIR)$(PREFIX)/share/man
 
 STAMP	:= configure-stamp
 ifeq ($(wildcard $(STAMP)),)
-_ := $(shell ./configure)
+	_ := $(shell ./configure)
 endif
 
 #
@@ -27,11 +27,11 @@ LDFLAGS		:= -lpthread -lm $(OSLDFLAGS)
 CYGWIN_REQS	:= cygwin1.dll cygrunsrv.exe
 
 ifeq ($(CC),gcc)
-GCC_VER := $(shell ${CC} -dumpfullversion | sed -e 's/\.\([0-9][0-9]\)/\1/g' -e 's/\.\([0-9]\)/0\1/g' -e 's/^[0-9]\{3,4\}$$/&00/')
-GCC_GTEQ_430 := $(shell expr ${GCC_VER} \>= 40300)
-GCC_GTEQ_450 := $(shell expr ${GCC_VER} \>= 40500)
-GCC_GTEQ_600 := $(shell expr ${GCC_VER} \>= 60000)
-GCC_GTEQ_700 := $(shell expr ${GCC_VER} \>= 70000)
+	GCC_VER := $(shell ${CC} -dumpfullversion | sed -e 's/\.\([0-9][0-9]\)/\1/g' -e 's/\.\([0-9]\)/0\1/g' -e 's/^[0-9]\{3,4\}$$/&00/')
+	GCC_GTEQ_430 := $(shell expr ${GCC_VER} \>= 40300)
+	GCC_GTEQ_450 := $(shell expr ${GCC_VER} \>= 40500)
+	GCC_GTEQ_600 := $(shell expr ${GCC_VER} \>= 60000)
+	GCC_GTEQ_700 := $(shell expr ${GCC_VER} \>= 70000)
 endif
 
 CFLAGS	+= -std=c99 -D__BSD_VISIBLE -D_ALL_SOURCE -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=200112 -D_ISOC99_SOURCE -D_REENTRANT -D_BSD_SOURCE -D_DEFAULT_SOURCE -D_DARWIN_C_SOURCE -DVERSION=\"'$(VER)'\"
@@ -41,24 +41,24 @@ CFLAGS	+= -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
 #CFLAGS  += -fsanitize=undefined -fsanitize-undefined-trap-on-error
 
 ifeq ($(CC),gcc)
-ifeq "$(GCC_GTEQ_430)" "1"
-	CFLAGS += -Wlogical-op
-endif
-ifeq "$(GCC_GTEQ_450)" "1"
-	CFLAGS += -Wjump-misses-init
-endif
-ifeq "$(GCC_GTEQ_600)" "1"
-	CFLAGS += -Wduplicated-cond
-	CFLAGS += -Wnull-dereference
-	CFLAGS += -Werror=uninitialized
-	CFLAGS += -Wformat=2
-	CFLAGS += -Wformat-overflow=2
-	CFLAGS += -Wformat-truncation=2
-	CFLAGS += -Wformat-security
-endif
-ifeq "$(GCC_GTEQ_700)" "1"
-	CFLAGS += -Wduplicated-branches
-endif
+	ifeq "$(GCC_GTEQ_430)" "1"
+		CFLAGS += -Wlogical-op
+	endif
+	ifeq "$(GCC_GTEQ_450)" "1"
+		CFLAGS += -Wjump-misses-init
+	endif
+	ifeq "$(GCC_GTEQ_600)" "1"
+		CFLAGS += -Wduplicated-cond
+		CFLAGS += -Wnull-dereference
+		CFLAGS += -Werror=uninitialized
+		CFLAGS += -Wformat=2
+		CFLAGS += -Wformat-overflow=2
+		CFLAGS += -Wformat-truncation=2
+		CFLAGS += -Wformat-security
+	endif
+	ifeq "$(GCC_GTEQ_700)" "1"
+		CFLAGS += -Wduplicated-branches
+	endif
 endif
 
 #CFLAGS	+= -fstack-protector-strong
@@ -79,20 +79,23 @@ OBJS=main.o utils.o ntlm.o xcrypt.o config.o socket.o acl.o auth.o http.o forwar
 CONFIG_GSS=$(shell grep -c "config_gss 1" config/config.h)
 ifeq ($(CONFIG_GSS),1)
 	OBJS+=kerberos.o
-ifeq ($(OS),Darwin)
-	LDFLAGS+=-framework GSS
-else
-	LDFLAGS+=-lgssapi_krb5
-endif
+	ifeq ($(OS),Darwin)
+		LDFLAGS+=-framework GSS
+	else
+		LDFLAGS+=-lgssapi_krb5
+	endif
 endif
 
 ifneq ($(findstring CYGWIN,$(OS)),)
 	OBJS+=sspi.o win/resources.o
 endif
 
-ENABLE_STATIC=$(shell grep -c ENABLE_STATIC config/config.h)
+# Static linking is not available on macOS
+ENABLE_STATIC=$(shell grep -c ENABLE_STATIC $(STAMP))
 ifeq ($(ENABLE_STATIC),1)
-        LDFLAGS+=-static
+	ifneq ($(OS),Darwin)
+		LDFLAGS+=-static
+	endif
 endif
 
 CFLAGS_DUKTAPE := -Wno-bad-function-cast -Wno-null-dereference -Wno-format-nonliteral -Wno-unused-but-set-variable

auth.c

@@ -33,24 +33,28 @@ struct auth_s *new_auth(void) {
 	if (tmp == NULL)
 		return NULL;
 
-	memset(tmp->user, 0, MINIBUF_SIZE);
-	memset(tmp->domain, 0, MINIBUF_SIZE);
-	memset(tmp->workstation, 0, MINIBUF_SIZE);
-	memset(tmp->passntlm2, 0, MINIBUF_SIZE);
-	memset(tmp->passnt, 0, MINIBUF_SIZE);
-	memset(tmp->passlm, 0, MINIBUF_SIZE);
+	return init_auth(tmp);
+}
+
+struct auth_s *init_auth(struct auth_s *creds) {
+	memset(creds->user, 0, MINIBUF_SIZE);
+	memset(creds->domain, 0, MINIBUF_SIZE);
+	memset(creds->workstation, 0, MINIBUF_SIZE);
+	memset(creds->passntlm2, 0, MINIBUF_SIZE);
+	memset(creds->passnt, 0, MINIBUF_SIZE);
+	memset(creds->passlm, 0, MINIBUF_SIZE);
 #ifdef __CYGWIN__
-	memset(&tmp->sspi, 0, sizeof(struct sspi_handle));
+	memset(&creds->sspi, 0, sizeof(struct sspi_handle));
 #endif
-	tmp->hashntlm2 = 1;
-	tmp->hashnt = 0;
-	tmp->hashlm = 0;
-	tmp->flags = 0;
+	creds->hashntlm2 = 1;
+	creds->hashnt = 0;
+	creds->hashlm = 0;
+	creds->flags = 0;
 #if config_gss == 1
-	tmp->haskrb = 0;
+	creds->haskrb = 0;
 #endif
 
-	return tmp;
+	return creds;
 }
 
 struct auth_s *copy_auth(struct auth_s *dst, const struct auth_s *src, int fullcopy) {

auth.h

@@ -68,6 +68,7 @@ struct auth_s {
  * new_auth() is also just a convenience malloc/memset() wrapper
  */
 extern struct auth_s *new_auth(void);
+extern struct auth_s *init_auth(struct auth_s *creds);
 extern struct auth_s *copy_auth(struct auth_s *dst, const struct auth_s *src, int fullcopy);
 extern struct auth_s *dup_auth(const struct auth_s *creds, int fullcopy);
 extern void dump_auth(const struct auth_s *creds);

config.c

@@ -59,7 +59,7 @@ config_t config_open(const char *fname) {
 		/*
 		 * Find first non-empty character
 		 */
-		for (i = j = 0; j < len && isspace((u_char)buf[j]); ++j);
+		for (j = 0; j < len && isspace((u_char)buf[j]); ++j);
 
 		/*
 		 * Comment?
@@ -98,7 +98,7 @@ config_t config_open(const char *fname) {
 		/*
 		 * Find next non-empty character
 		 */
-		for (i = j; j < len && isspace((u_char)buf[j]); ++j);
+		for (; j < len && isspace((u_char)buf[j]); ++j);
 		if (j >= len || buf[j] == '#' || buf[j] == ';') {
 			free(key);
 			continue;

configure

@@ -63,28 +63,24 @@ for i in $TESTS; do
 	else
 		RET=$(./config/"$i")
 		rc=$?
-		[ -z "$RET" ] && if [ $rc -eq 0 ]; then RET="no"; else RET=yes; fi
+		[ -z "$RET" ] && if [ $rc -eq 0 ]; then RET="no"; else RET="yes"; fi
 	fi
 
 	echo $rc >> $CONFIG
 	echo "$RET"
 done
+echo "" >> $CONFIG
+echo "#endif // CONFIGURE_CONFIG_H" >> $CONFIG
 
 while [ "$1" ]
 do
 	case $1 in
 		--enable-static)
-			printf "#define ENABLE_STATIC" >> $CONFIG
-			echo "" >> $CONFIG
+			echo "ENABLE_STATIC" >> $STAMP
 			;;
 		*)
 			echo "Unknown flag $1"
-			rm -f $CONFIG
 			;;
 	esac
 	shift
 done
-if [ -f $CONFIG ]; then
-	echo "" >> $CONFIG
-	echo "#endif // CONFIGURE_CONFIG_H" >> $CONFIG
-fi

direct.c

@@ -258,8 +258,9 @@ rr_data_t direct_request(void *cdata, rr_data_const_t request) {
 		rsocket[1] = wsocket[0] = &sd;
 
 		conn_alive = 0;
+		loop = 0; // 0 = request from client; 1 = response from server
 
-		for (loop = 0; loop < 2; ++loop) {
+		while (loop < 2) {
 			if (data[loop]->empty) {				// Isn't this the first loop with request supplied by caller?
 				if (debug) {
 					printf("\n******* Round %d C: %d, S: %d *******\n", loop+1, cd, sd);
@@ -501,6 +502,8 @@ rr_data_t direct_request(void *cdata, rr_data_const_t request) {
 					goto bailout;
 				}
 			}
+
+			++loop;
 		}
 
 		free_rr_data(&data[0]);

forward.c

@@ -100,7 +100,7 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 
 beginning:
 	sd = 0;
-	was_cached = noauth = authok = conn_alive = proxy_alive = 0;
+	was_cached = noauth = authok = proxy_alive = 0;
 
 	rsocket[0] = wsocket[1] = &cd;
 	rsocket[1] = wsocket[0] = &sd;
@@ -187,8 +187,9 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 		retry = 0;
 		proxy_alive = 0;
 		conn_alive = 0;
+		loop = 0; // 0 = request from client; 1 = response from server
 
-		for (loop = 0; loop < 2; ++loop) {
+		while (loop < 2) {
 			if (data[loop]->empty) {				// Isn't this the first loop with request supplied by caller?
 				if (debug) {
 					printf("\n******* Round %d C: %d, S: %d (authok=%d, noauth=%d) *******\n", loop+1, cd, sd, authok, noauth);
@@ -462,6 +463,8 @@ rr_data_t forward_request(void *thread_data, rr_data_t request) {
 					rc = (void *)-1;
 				}
 			}
+
+			++loop;
 		}
 
 		free_rr_data(&data[0]);

globals.h

@@ -32,8 +32,6 @@
 
 extern int debug;
 
-extern int request_logging_level;
-
 extern struct auth_s *g_creds;			/* global NTLM credentials */
 
 extern int ntlmbasic;				/* forward_request() */

http.c

@@ -450,7 +450,7 @@ int chunked_data_send(int dst, int src) {
 	} while (csize != 0);
 
 	/* Take care of possible trailer */
-	w = len = i = 0;
+	w = len = 0;
 	do {
 		i = so_recvln(src, &buf, &bsize);
 		if (dst >= 0 && i > 0) {

kerberos.c

@@ -85,7 +85,8 @@ void display_ctx_flags(OM_uint32 flags) {
 }
 
 static void display_status_1(char *m, OM_uint32 code, int type) {
-	OM_uint32 maj_stat, min_stat;
+	OM_uint32 maj_stat;
+	OM_uint32 min_stat;
 	gss_buffer_desc msg;
 	OM_uint32 msg_ctx;
 
@@ -150,7 +151,8 @@ void display_name(char* txt, gss_name_t *name) {
 
 int acquire_name(gss_name_t *target_name, char *service_name, gss_OID oid) {
 	gss_buffer_desc tmp_tok;
-	OM_uint32 maj_stat, min_stat;
+	OM_uint32 maj_stat;
+	OM_uint32 min_stat;
 
 	tmp_tok.value = service_name;
 	tmp_tok.length = strlen(service_name) + 1;
@@ -197,7 +199,9 @@ int client_establish_context(char *service_name,
 		OM_uint32 *ret_flags, gss_buffer_desc* send_tok) {
 	gss_name_t target_name;
 	gss_ctx_id_t gss_context = GSS_C_NO_CONTEXT;
-	OM_uint32 maj_stat, min_stat, init_min_stat;
+	OM_uint32 maj_stat;
+	OM_uint32 min_stat;
+	OM_uint32 init_min_stat;
 
 	if ((maj_stat = acquire_name(&target_name, service_name,
 			GSS_C_NT_HOSTBASED_SERVICE)) != GSS_S_COMPLETE)
@@ -240,7 +244,7 @@ int client_establish_context(char *service_name,
 	if (maj_stat != GSS_S_COMPLETE && debug) {
 		display_status("Deleting context", maj_stat, min_stat);
 	}
-	return GSS_S_COMPLETE;//maj_stat;
+	return GSS_S_COMPLETE;
 }
 
 
@@ -251,7 +255,8 @@ int client_establish_context(char *service_name,
 int acquire_kerberos_token(const char* hostname, struct auth_s *credentials,
 		char** buf, size_t *bufsize) {
 	char service_name[BUFSIZE];
-	OM_uint32 ret_flags, min_stat;
+	OM_uint32 ret_flags;
+	OM_uint32 min_stat;
 
 	if (credentials->haskrb == KRB_KO) {
 		if (debug)

main.c

@@ -83,7 +83,8 @@
  */
 int debug = 0;					/* all debug printf's and possibly external modules */
 
-struct auth_s *g_creds = NULL;			/* throughout the whole module */
+struct auth_s g_creds_s;
+struct auth_s *g_creds = &g_creds_s;		/* throughout the whole module */
 
 int quit = 0;					/* sighandler() */
 int ntlmbasic = 0;				/* forward_request() */
@@ -287,7 +288,6 @@ void *proxy_thread(void *thread_data) {
 
 	do {
 		ret = NULL;
-		keep_alive = 0;
 
 		if (debug) {
 			printf("\n******* Round 1 C: %d *******\n", cd);
@@ -736,7 +736,7 @@ int main(int argc, char **argv) {
 	char *pac_file;
 
 	pac_file = zmalloc(PATH_MAX);
-	g_creds = new_auth();
+
 	cuser = zmalloc(MINIBUF_SIZE);
 	cdomain = zmalloc(MINIBUF_SIZE);
 	cpassword = zmalloc(PASSWORD_BUFSIZE);
@@ -748,6 +748,8 @@ int main(int argc, char **argv) {
 	cuid = zmalloc(MINIBUF_SIZE);
 	cauth = zmalloc(MINIBUF_SIZE);
 
+	init_auth(g_creds);
+
 	int syslog_debug = 0;
 	openlog("cntlm", LOG_CONS | LOG_PERROR, LOG_DAEMON);
 
@@ -1845,7 +1847,6 @@ int main(int argc, char **argv) {
 	free(cuid);
 	free(cpidfile);
 	free(magic_detect);
-	free(g_creds);
 
 	parent_free();
 

proxy.c

@@ -474,7 +474,7 @@ int proxy_connect(struct auth_s *credentials, const char* url, const char* hostn
 		 */
 		if (i < 0) {
 			p = proxylist_get_next(proxylist, proxycurr);
-			if (p) {
+			if (p && p->proxy) {
 				proxycurr = p->key;
 				proxy = p->proxy;
 				syslog(LOG_ERR, "Proxy connect failed, will try %s:%d\n", proxy->hostname, proxy->port);
@@ -654,7 +654,6 @@ int proxy_authenticate(int *sd, rr_data_t request, rr_data_t response, struct au
 					printf("Using Negotiation ...\n");
 
 				request->headers = hlist_mod(request->headers, "Proxy-Authorization", buf, 1);
-				free(tmp);
 			}
 			else {
 #endif