diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 21317e7..4fe55bc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,7 @@ jobs: - name: Run comprehensive tests run: | make test + make connection-limit-test cd tests ./test_security_features.sh # Skipping anonymous access test in CI as it requires interactive pty handling which might be flaky diff --git a/.gitignore b/.gitignore index 9f369cb..fcd0eeb 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ tests/unit/test_system_message tests/unit/test_help_text tests/unit/test_support_text tests/unit/test_cli_text +tests/unit/test_ratelimit diff --git a/Makefile b/Makefile index 118e75c..7375971 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ BINDIR ?= $(PREFIX)/bin MANDIR ?= $(PREFIX)/share/man SYSTEMD_UNIT_DIR ?= $(PREFIX)/lib/systemd/system -.PHONY: all clean install install-systemd uninstall uninstall-systemd debug release release-check release-check-strict asan valgrind check test test-advisory unit-test integration-test info +.PHONY: all clean install install-systemd uninstall uninstall-systemd debug release release-check release-check-strict asan valgrind check test test-advisory unit-test integration-test connection-limit-test info all: $(TARGET) @@ -112,6 +112,10 @@ integration-test: all @cd tests && PORT=$$(($${PORT:-2222} + 1)) ./test_exec_mode.sh @cd tests && PORT=$$(($${PORT:-2222} + 2)) ./test_interactive_input.sh +connection-limit-test: all + @echo "Running connection limit tests..." + @cd tests && PORT=$${PORT:-2222} ./test_connection_limits.sh + # Show build info info: @echo "Compiler: $(CC)" diff --git a/README.md b/README.md index 866a7d8..11b0b19 100644 --- a/README.md +++ b/README.md @@ -202,6 +202,7 @@ make clean # clean build artifacts ```sh make test # run comprehensive test suite and fail on regressions make test-advisory # run integration tests as advisory checks +make connection-limit-test # verify per-IP concurrency and rate limits # Individual tests cd tests diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index bd7eaed..216efba 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -45,6 +45,12 @@ environments can use `make test-advisory` for the previous advisory behavior. - Removed the duplicate `deploy.yml` CI workflow so automated checks stay focused on CI while production deployment remains manual. +- Fixed the per-IP connection-rate limit to allow the configured number of + attempts before blocking, added unit coverage, and exposed + `make connection-limit-test` for the black-box limit regression test. +- Security feature checks now use isolated ports and temporary state + directories, so they no longer require `timeout`/`gtimeout` or write + `host_key` / `messages.log` into the test directory. - NORMAL mode now opens at the latest visible messages instead of the oldest in-memory message. Use `k`/PageUp to browse older history and `G`/End to return to the latest messages. diff --git a/docs/CICD.md b/docs/CICD.md index a43110c..63ba3c1 100644 --- a/docs/CICD.md +++ b/docs/CICD.md @@ -7,6 +7,7 @@ Every push or PR automatically runs: - Build on Ubuntu - AddressSanitizer build - Unit and strict integration tests + - Per-IP concurrency and connection-rate limit tests - Release/package preflight (`make release-check`) Check status: diff --git a/docs/Development-Guide.md b/docs/Development-Guide.md index 75d8ac1..29a99eb 100644 --- a/docs/Development-Guide.md +++ b/docs/Development-Guide.md @@ -161,6 +161,7 @@ make install # Install to /usr/local/bin ```sh make test # Run all tests and fail on regressions make test-advisory # Run integration tests as advisory checks +make connection-limit-test # Verify per-IP concurrency and rate limits # Individual tests cd tests diff --git a/src/ratelimit.c b/src/ratelimit.c index 5f2ff7f..c898bcb 100644 --- a/src/ratelimit.c +++ b/src/ratelimit.c @@ -134,7 +134,7 @@ bool ratelimit_check_ip(const char *ip) { } entry->recent_connection_count++; - if (entry->recent_connection_count >= g_max_conn_rate_per_ip) { + if (entry->recent_connection_count > g_max_conn_rate_per_ip) { entry->is_blocked = true; entry->block_until = now + BLOCK_DURATION; pthread_mutex_unlock(&g_rate_limit_lock); diff --git a/tests/test_security_features.sh b/tests/test_security_features.sh index b0bb8c3..1cd66e4 100755 --- a/tests/test_security_features.sh +++ b/tests/test_security_features.sh @@ -11,6 +11,9 @@ NC='\033[0m' PASS=0 FAIL=0 +STATE_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/tnt-security-test.XXXXXX") +SERVER_PIDS="" +NEXT_PORT=${PORT:-13600} print_test() { echo -e "\n${YELLOW}[TEST]${NC} $1" @@ -27,8 +30,11 @@ fail() { } cleanup() { - pkill -f "^\.\./tnt" 2>/dev/null || true - sleep 1 + for pid in $SERVER_PIDS; do + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + done + rm -rf "$STATE_ROOT" } trap cleanup EXIT @@ -43,23 +49,47 @@ if [ ! -f "$BIN" ]; then exit 1 fi -# Detect timeout command -TIMEOUT_CMD="timeout" -if command -v gtimeout >/dev/null 2>&1; then - TIMEOUT_CMD="gtimeout" -fi +run_server_probe() { + local name="$1" + local port="$NEXT_PORT" + local pid + local state_dir + local log_file + shift + + NEXT_PORT=$((NEXT_PORT + 1)) + state_dir="$STATE_ROOT/$name" + log_file="$state_dir/server.log" + mkdir -p "$state_dir" + + "$@" "$BIN" -p "$port" -d "$state_dir" >"$log_file" 2>&1 & + pid=$! + SERVER_PIDS="$SERVER_PIDS $pid" + + for _ in 1 2 3 4 5 6 7 8; do + if grep -q "TNT chat server listening" "$log_file"; then + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + return 0 + fi + if ! kill -0 "$pid" 2>/dev/null; then + break + fi + sleep 1 + done + + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + sed -n '1,120p' "$log_file" + return 1 +} # Test 1: 4096-bit RSA Key Generation print_test "1. RSA 4096-bit Key Generation" -rm -f host_key -$BIN & -PID=$! -sleep 8 # Wait for key generation -kill $PID 2>/dev/null || true -sleep 1 +KEY_DIR="$STATE_ROOT/host-key" -if [ -f host_key ]; then - KEY_SIZE=$(ssh-keygen -l -f host_key 2>/dev/null | awk '{print $1}') +if run_server_probe host-key env && [ -f "$KEY_DIR/host_key" ]; then + KEY_SIZE=$(ssh-keygen -l -f "$KEY_DIR/host_key" 2>/dev/null | awk '{print $1}') if [ "$KEY_SIZE" = "4096" ]; then pass "RSA key upgraded to 4096 bits (was 2048)" else @@ -68,9 +98,9 @@ if [ -f host_key ]; then # Check permissions if [[ "$OSTYPE" == "darwin"* ]]; then - PERMS=$(stat -f "%OLp" host_key) + PERMS=$(stat -f "%OLp" "$KEY_DIR/host_key") else - PERMS=$(stat -c "%a" host_key) + PERMS=$(stat -c "%a" "$KEY_DIR/host_key") fi if [ "$PERMS" = "600" ]; then @@ -86,33 +116,34 @@ fi print_test "2. Environment Variable Configuration" # Test bind address -TNT_BIND_ADDR=127.0.0.1 $TIMEOUT_CMD 3 $BIN 2>&1 | grep -q "TNT chat server" && \ +run_server_probe bind-addr env TNT_BIND_ADDR=127.0.0.1 && \ pass "TNT_BIND_ADDR configuration works" || fail "TNT_BIND_ADDR not working" # Test with access token set (just verify it starts) -TNT_ACCESS_TOKEN="test123" $TIMEOUT_CMD 3 $BIN 2>&1 | grep -q "TNT chat server" && \ +run_server_probe access-token env TNT_ACCESS_TOKEN="test123" && \ pass "TNT_ACCESS_TOKEN configuration accepted" || fail "TNT_ACCESS_TOKEN not working" # Test max connections configuration -TNT_MAX_CONNECTIONS=10 $TIMEOUT_CMD 3 $BIN 2>&1 | grep -q "TNT chat server" && \ +run_server_probe max-connections env TNT_MAX_CONNECTIONS=10 && \ pass "TNT_MAX_CONNECTIONS configuration accepted" || fail "TNT_MAX_CONNECTIONS not working" # Test per-IP connection rate configuration -TNT_MAX_CONN_RATE_PER_IP=20 $TIMEOUT_CMD 3 $BIN 2>&1 | grep -q "TNT chat server" && \ +run_server_probe conn-rate env TNT_MAX_CONN_RATE_PER_IP=20 && \ pass "TNT_MAX_CONN_RATE_PER_IP configuration accepted" || fail "TNT_MAX_CONN_RATE_PER_IP not working" # Test rate limit toggle -TNT_RATE_LIMIT=0 $TIMEOUT_CMD 3 $BIN 2>&1 | grep -q "TNT chat server" && \ +run_server_probe rate-toggle env TNT_RATE_LIMIT=0 && \ pass "TNT_RATE_LIMIT configuration accepted" || fail "TNT_RATE_LIMIT not working" sleep 1 # Test 3: Input Validation in Message Log print_test "3. Message Log Sanitization" -rm -f messages.log +MESSAGE_DIR="$STATE_ROOT/message-log" +mkdir -p "$MESSAGE_DIR" # Create a test message log with potentially dangerous content -cat > messages.log < "$MESSAGE_DIR/messages.log" </dev/null || true -sleep 1 - -# Check if server handled malformed log entries safely -if grep -q "validuser" messages.log; then +# Start server and let it load messages, then verify it kept valid entries. +if run_server_probe message-log env >/dev/null && + grep -q "validuser" "$MESSAGE_DIR/messages.log"; then pass "Server loads messages from log file" else fail "Server message loading issue" @@ -215,21 +240,16 @@ fi # Test 7: Resource Management (Dynamic Allocation) print_test "7. Resource Management (Large Log Files)" -rm -f messages.log +LARGE_DIR="$STATE_ROOT/large-log" +mkdir -p "$LARGE_DIR" # Create a large message log (2000 entries, more than old fixed 1000 limit) for i in $(seq 1 2000); do - echo "2026-01-22T$(printf "%02d" $((i/100))):$(printf "%02d" $((i%60))):00Z|user$i|message $i" >> messages.log + echo "2026-01-22T$(printf "%02d" $((i/100))):$(printf "%02d" $((i%60))):00Z|user$i|message $i" >> "$LARGE_DIR/messages.log" done -$BIN & -PID=$! -sleep 4 -kill $PID 2>/dev/null || true -sleep 1 - # Check if server started successfully with large log -if [ -f messages.log ]; then - LINE_COUNT=$(wc -l < messages.log) +if run_server_probe large-log env >/dev/null && [ -f "$LARGE_DIR/messages.log" ]; then + LINE_COUNT=$(wc -l < "$LARGE_DIR/messages.log") if [ "$LINE_COUNT" -ge 2000 ]; then pass "Server handles large message log (${LINE_COUNT} messages)" else diff --git a/tests/unit/Makefile b/tests/unit/Makefile index ef2dafe..edc09d9 100644 --- a/tests/unit/Makefile +++ b/tests/unit/Makefile @@ -20,8 +20,9 @@ I18N_SRC = ../../src/i18n.c SYSTEM_MESSAGE_SRC = ../../src/system_message.c HELP_TEXT_SRC = ../../src/help_text.c SUPPORT_TEXT_SRC = ../../src/support_text.c +RATELIMIT_SRC = ../../src/ratelimit.c -TESTS = test_utf8 test_message test_chat_room test_history_view test_i18n test_system_message test_help_text test_support_text test_cli_text +TESTS = test_utf8 test_message test_chat_room test_history_view test_i18n test_system_message test_help_text test_support_text test_cli_text test_ratelimit .PHONY: all clean run @@ -54,6 +55,9 @@ test_support_text: test_support_text.c $(SUPPORT_TEXT_SRC) test_cli_text: test_cli_text.c $(CLI_TEXT_SRC) $(COMMON_SRC) $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) +test_ratelimit: test_ratelimit.c $(RATELIMIT_SRC) $(COMMON_SRC) + $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) + run: all @echo "=== Running UTF-8 Tests ===" ./test_utf8 @@ -81,6 +85,9 @@ run: all @echo "" @echo "=== Running CLI Text Tests ===" ./test_cli_text + @echo "" + @echo "=== Running Rate Limit Tests ===" + ./test_ratelimit clean: rm -f $(TESTS) *.o test_messages.log diff --git a/tests/unit/test_ratelimit.c b/tests/unit/test_ratelimit.c new file mode 100644 index 0000000..e2b05b7 --- /dev/null +++ b/tests/unit/test_ratelimit.c @@ -0,0 +1,71 @@ +/* Unit tests for connection and rate-limit accounting */ + +#include "../../include/ratelimit.h" +#include +#include +#include + +#define TEST(name) static void test_##name() +#define RUN_TEST(name) do { \ + printf("Running %s... ", #name); \ + test_##name(); \ + printf("āœ“\n"); \ + tests_passed++; \ +} while(0) + +static int tests_passed = 0; + +TEST(per_ip_concurrent_limit_blocks_second_active_connection) { + const char *ip = "203.0.113.10"; + + setenv("TNT_RATE_LIMIT", "0", 1); + setenv("TNT_MAX_CONN_PER_IP", "1", 1); + ratelimit_init(); + + assert(ratelimit_check_ip(ip) == true); + assert(ratelimit_check_ip(ip) == false); + + ratelimit_release_ip(ip); + assert(ratelimit_check_ip(ip) == true); + ratelimit_release_ip(ip); +} + +TEST(rate_limit_allows_configured_burst_then_blocks) { + const char *ip = "203.0.113.20"; + + setenv("TNT_RATE_LIMIT", "1", 1); + setenv("TNT_MAX_CONN_PER_IP", "10", 1); + setenv("TNT_MAX_CONN_RATE_PER_IP", "2", 1); + ratelimit_init(); + + assert(ratelimit_check_ip(ip) == true); + ratelimit_release_ip(ip); + assert(ratelimit_check_ip(ip) == true); + ratelimit_release_ip(ip); + assert(ratelimit_check_ip(ip) == false); +} + +TEST(global_limit_tracks_active_total) { + setenv("TNT_MAX_CONNECTIONS", "1", 1); + ratelimit_init(); + + assert(ratelimit_check_and_increment_total() == true); + assert(ratelimit_get_active_total() == 1); + assert(ratelimit_check_and_increment_total() == false); + + ratelimit_decrement_total(); + assert(ratelimit_get_active_total() == 0); + assert(ratelimit_check_and_increment_total() == true); + ratelimit_decrement_total(); +} + +int main(void) { + printf("Running rate-limit unit tests...\n\n"); + + RUN_TEST(per_ip_concurrent_limit_blocks_second_active_connection); + RUN_TEST(rate_limit_allows_configured_burst_then_blocks); + RUN_TEST(global_limit_tracks_active_total); + + printf("\nāœ“ All %d tests passed!\n", tests_passed); + return 0; +}