receive SM_CMD_AUTH_BAN_IP_REPLY asynchronously to prevent race conditions

src/ipc.proto

@@ -144,12 +144,14 @@ message ban_ip_msg
 {
 	required string ip = 1;
 	required uint32 score = 2;
+	optional bytes sid = 3; /* sec-mod sends it */
 }
 
 message ban_ip_reply_msg
 {
 	/* whether to disconnect the user */
 	required AUTH_REP reply = 1;
+	optional bytes sid = 2; /* sec-mod needs it */
 }
 
 /* Messages to and from the security module */

src/main-sec-mod-cmd.c

@@ -58,6 +58,7 @@ int handle_sec_mod_commands(main_server_st * s)
 	int ret, raw_len, e;
 	void *pool = talloc_new(s);
 	PROTOBUF_ALLOCATOR(pa, pool);
+	BanIpMsg *tmsg = NULL;
 
 	if (pool == NULL)
 		return -1;
@@ -112,7 +113,6 @@ int handle_sec_mod_commands(main_server_st * s)
 
 	switch (cmd) {
 	case SM_CMD_AUTH_BAN_IP:{
-			BanIpMsg *tmsg;
 			BanIpReplyMsg reply = BAN_IP_REPLY_MSG__INIT;
 
 			tmsg = ban_ip_msg__unpack(&pa, raw_len, raw);
@@ -122,9 +122,6 @@ int handle_sec_mod_commands(main_server_st * s)
 				goto cleanup;
 			}
 			ret = add_ip_to_ban_list(s, tmsg->ip, tmsg->score);
-
-			ban_ip_msg__free_unpacked(tmsg, &pa);
-
 			if (ret < 0) {
 				reply.reply =
 				    AUTH__REP__FAILED;
@@ -133,6 +130,9 @@ int handle_sec_mod_commands(main_server_st * s)
 				    AUTH__REP__OK;
 			}
 
+			reply.sid.data = tmsg->sid.data;
+			reply.sid.len = tmsg->sid.len;
+			reply.has_sid = tmsg->has_sid;
 
 			mslog(s, NULL, LOG_DEBUG, "sending msg %s to sec-mod", cmd_request_to_str(SM_CMD_AUTH_BAN_IP_REPLY));
 
@@ -157,6 +157,8 @@ int handle_sec_mod_commands(main_server_st * s)
 
 	ret = 0;
  cleanup:
+	if (tmsg != NULL)
+		ban_ip_msg__free_unpacked(tmsg, &pa);
 	talloc_free(raw);
 	talloc_free(pool);
 

src/sec-mod-auth.c

@@ -75,55 +75,39 @@ void sec_auth_init(sec_mod_st * sec, struct perm_cfg_st *config)
 /* returns a negative number if we have reached the score for this client.
  */
 static
-int sec_mod_add_score_to_ip(sec_mod_st *sec, void *pool, const char *ip, unsigned points)
+void sec_mod_add_score_to_ip(sec_mod_st *sec, client_entry_st *e, const char *ip, unsigned points)
 {
-	void *lpool = talloc_new(pool);
-	int ret, e;
+	void *lpool = talloc_new(e);
+	int ret, err;
 	BanIpMsg msg = BAN_IP_MSG__INIT;
-	BanIpReplyMsg *reply = NULL;
-	PROTOBUF_ALLOCATOR(pa, lpool);
 
 	/* no reporting if banning is disabled */
 	if (sec->config->max_ban_score == 0)
-		return 0;
+		return;
 
 	msg.ip = (char*)ip;
 	msg.score = points;
+	msg.sid.data = e->sid;
+	msg.sid.len = sizeof(e->sid);
+	msg.has_sid = 1;
 
 	if (lpool == NULL) {
-		return 0;
+		return;
 	}
 
 	ret = send_msg(lpool, sec->cmd_fd, SM_CMD_AUTH_BAN_IP, &msg,
 				(pack_size_func) ban_ip_msg__get_packed_size,
 				(pack_func) ban_ip_msg__pack);
 	if (ret < 0) {
-		e = errno;
-		seclog(sec, LOG_WARNING, "error in sending BAN IP message: %s", strerror(e));
-		ret = -1;
+		err = errno;
+		seclog(sec, LOG_WARNING, "error in sending BAN IP message: %s", strerror(err));
 		goto fail;
 	}
 
-	ret = recv_msg(lpool, sec->cmd_fd, SM_CMD_AUTH_BAN_IP_REPLY, (void*)&reply,
-		       (unpack_func) ban_ip_reply_msg__unpack);
-	if (ret < 0) {
-		seclog(sec, LOG_ERR, "error receiving BAN IP reply message");
-		ret = -1;
-		goto fail;
-	}
-
-	if (reply->reply != AUTH__REP__OK) {
-		/* we have exceeded the maximum score */
-		ret = -1;
-	} else {
-		ret = 0;
-	}
-	ban_ip_reply_msg__free_unpacked(reply, &pa);
-
  fail:
 	talloc_free(lpool);
 
-	return ret;
+	return;
 }
 
 static int generate_cookie(sec_mod_st * sec, client_entry_st * entry)
@@ -326,11 +310,7 @@ int handle_sec_auth_res(int cfd, sec_mod_st * sec, client_entry_st * e, int resu
 	if (result == ERR_AUTH_CONTINUE) {
 		/* if the module allows multiple retries for the password */
 		if (e->status != PS_AUTH_INIT && e->module && e->module->allows_retries) {
-			ret = sec_mod_add_score_to_ip(sec, e, e->auth_info.remote_ip, sec->config->ban_points_wrong_password);
-			if (ret < 0) {
-				e->status = PS_AUTH_FAILED;
-				return send_sec_auth_reply(cfd, sec, e, AUTH__REP__FAILED);
-			}
+			sec_mod_add_score_to_ip(sec, e, e->auth_info.remote_ip, sec->config->ban_points_wrong_password);
 		}
 
 		ret = send_sec_auth_reply_msg(cfd, sec, e);
@@ -340,7 +320,7 @@ int handle_sec_auth_res(int cfd, sec_mod_st * sec, client_entry_st * e, int resu
 			return ret;
 		}
 		return 0;	/* wait for another command */
-	} else if (result == 0) {
+	} else if (result == 0 && e->status != PS_AUTH_FAILED) {
 		e->status = PS_AUTH_COMPLETED;
 
 		if (e->module) {
@@ -566,6 +546,28 @@ int handle_sec_auth_session_cmd(int cfd, sec_mod_st *sec, const SecAuthSessionMs
 		return handle_sec_auth_session_close(cfd, sec, req);
 }
 
+void handle_sec_auth_ban_ip_reply(int cfd, sec_mod_st *sec, const BanIpReplyMsg *msg)
+{
+	client_entry_st *e;
+
+	if (msg->sid.len != SID_SIZE) {
+		seclog(sec, LOG_ERR, "ban IP reply but with illegal sid size (%d)!",
+		       (int)msg->sid.len);
+		return;
+	}
+
+	e = find_client_entry(sec, msg->sid.data);
+	if (e == NULL) {
+		return;
+	}
+
+	if (msg->reply != AUTH__REP__OK) {
+		e->status = PS_AUTH_FAILED;
+	}
+
+	return;
+}
+
 int handle_sec_auth_stats_cmd(sec_mod_st * sec, const CliStatsMsg * req)
 {
 	client_entry_st *e;

src/sec-mod.c

@@ -317,6 +317,22 @@ int process_packet_from_main(void *pool, int cfd, sec_mod_st * sec, cmd_request_
 	data.size = buffer_size;
 
 	switch (cmd) {
+	case SM_CMD_AUTH_BAN_IP_REPLY:{
+		BanIpReplyMsg *msg = NULL;
+
+		msg =
+		    ban_ip_reply_msg__unpack(&pa, data.size,
+					     data.data);
+		if (msg == NULL) {
+			seclog(sec, LOG_INFO, "error unpacking auth ban ip reply\n");
+			return -1;
+		}
+
+		handle_sec_auth_ban_ip_reply(cfd, sec, msg);
+		ban_ip_reply_msg__free_unpacked(msg, &pa);
+
+		return 0;
+	}
 	case SM_CMD_AUTH_SESSION_OPEN:
 	case SM_CMD_AUTH_SESSION_CLOSE:{
 			SecAuthSessionMsg *msg;

src/sec-mod.h

@@ -119,6 +119,7 @@ void  seclog_hex(const struct sec_mod_st* sec, int priority,
 
 void sec_auth_init(sec_mod_st *sec, struct perm_cfg_st *config);
 
+void handle_sec_auth_ban_ip_reply(int cfd, sec_mod_st *sec, const BanIpReplyMsg *msg);
 int handle_sec_auth_init(int cfd, sec_mod_st *sec, const SecAuthInitMsg * req);
 int handle_sec_auth_cont(int cfd, sec_mod_st *sec, const SecAuthContMsg * req);
 int handle_sec_auth_session_cmd(int cfd, sec_mod_st *sec, const SecAuthSessionMsg *req, unsigned cmd);