Rule files, and bans for a clear sign of attack (closes #24)

Every *.rules file in SWWAF_RULES_DIR not named with a leading dot is
read at start, and again 2 seconds after the directory's last change.
Each request is checked against the rules after the rate limits: log
notes a match, block refuses with 403, ban refuses and bans the netblock
for SWWAF_ATTACK_BAN_DURATION, made permanent by its next request or
attack. path, query and uri are matched as the request line sent them;
header:Host and header:Transfer-Encoding are refused. Bans gain a cause.
The image ships 00-default.rules.

Judgement call: a header sent twice is matched with its values joined
by ", ".
Judgement call: SWWAF_MAX_BAN_DURATION does not cap a ban for an attack.
Not in this unit: offences for rule matches, with the error burst.

Model: opus-5-5

README.md

@@ -490,9 +490,11 @@ not make the netblock's next ban longer.
 `smallwebwaf` reads every `*.rules` file in `SWWAF_RULES_DIR`,
 `/etc/smallwebwaf/rules.d` by default, in the order of their names, and checks
 each request against their rules in that order, as "Rule files" in
-[`SPEC.md`](SPEC.md) describes. A rule is a line of four fields separated by
+[`SPEC.md`](SPEC.md) describes. A file whose name starts with `.`, such as an
-spaces or tabs: an id, a target, an action and a regex, which runs to the end of
+editor's lock file `.#50-app.rules`, is not a rule file, as a shell's `*.rules`
-the line. Blank lines and lines that start with `#` are ignored.
+would not match it. A rule is a line of four fields separated by spaces or tabs:
+an id, a target, an action and a regex, which runs to the end of the line. Blank
+lines and lines that start with `#` are ignored.
 
 ```
 # id           target      action  regex
@@ -523,10 +525,13 @@ header, a regex that does not compile or an id used twice stops the start with a
 message naming the file and the line, and so does a `SWWAF_RULES_DIR` that does
 not exist. An empty directory is no error, and the log says that it holds no
 rules. While it runs, `smallwebwaf` watches the directory, and reads the rule
-files again whenever one is edited, added or removed. If they then hold one of
+files again once the directory has had no change for 2 seconds after one is
-those errors, the rules stay as they were, the earlier version of the edited
+edited, added or removed, so that a file saved in place, appended to or copied
-file included, the log names the file and the line, and the files are read again
+in with `scp` is read only once whole, unless its writing stops for longer. It
-at the next change.
+also reads them 2 seconds after it starts watching, so that an edit saved while
+it started is not missed. If they then hold one of those errors, the rules stay
+as they were, the earlier version of the edited file included, the log names the
+file and the line, and the files are read again after the next change.
 
 The image ships one rule file, `share/rules.d/00-default.rules` here: rules that
 ban probes no real visitor sends, for secrets, version control directories,

internal/rules/rules.go

@@ -1,7 +1,8 @@
 // Package rules reads the rule files: the plain text files in
 // SWWAF_RULES_DIR, one rule to a line, that each request is checked
 // against, as the "Rule files" section of SPEC.md describes. They are read
-// at start, and again whenever one is edited, added or removed.
+// at start, and again once the directory has had no change for a short
+// time after one is edited, added or removed.
 package rules
 
 import (
@@ -17,6 +18,7 @@ import (
 	"slices"
 	"strings"
 	"sync/atomic"
+	"time"
 
 	"github.com/fsnotify/fsnotify"
 )
@@ -35,6 +37,12 @@ const (
 // extension ends the name of every rule file.
 const extension = ".rules"
 
+// quietTime is how long SWWAF_RULES_DIR must go without a change before
+// the rule files are read again, so that a file still being written, such
+// as one saved in place, appended to or copied in with scp, is read only
+// once whole.
+const quietTime = 2 * time.Second
+
 // headerTarget starts the target that is one request header,
 // header:<Name>.
 const headerTarget = "header:"
@@ -150,11 +158,12 @@ func (f *Files) Len() int {
 }
 
 // Watch watches Dir until ctx is done, and reads the rule files again
-// whenever one is edited, added or removed. If they then hold an error,
+// once Dir has had no change for quietTime, after one is edited, added or
+// removed, and after Watch starts watching. If they then hold an error,
 // the rules stay as they were, the error is logged with its file and
-// line, and the files are read again at the next change. If Dir cannot be
+// line, and the files are read again after the next change. If Dir cannot
-// watched, that is logged, and the rules stay as they were loaded. While
+// be watched, that is logged, and the rules stay as they were loaded.
-// Enabled is false, Watch returns at once.
+// While Enabled is false, Watch returns at once.
 func (f *Files) Watch(ctx context.Context) {
 	if !f.params.Enabled {
 		return
@@ -179,15 +188,29 @@ func (f *Files) Watch(ctx context.Context) {
 	f.params.ProcessLog.Info("watching the rule files for edits",
 		"directory", f.params.Dir)
 
+	f.readAfterChanges(ctx, watcher.Events, watcher.Errors)
+}
+
+// readAfterChanges reads the rule files again once quietTime has passed
+// without a change from events, until ctx is done, and logs the errors
+// from errs. The wait starts at once, as if for a change, so that an edit
+// saved after Load read the files, and before Dir was watched, is taken
+// in too.
+func (f *Files) readAfterChanges(
+	ctx context.Context, events <-chan fsnotify.Event, errs <-chan error,
+) {
+	quiet := time.NewTimer(quietTime)
+	defer quiet.Stop()
+
 	for {
 		select {
 		case <-ctx.Done():
 			return
-		case event := <-watcher.Events:
+		case <-events:
-			if filepath.Ext(event.Name) == extension {
+			quiet.Reset(quietTime)
-				f.readAgain()
+		case <-quiet.C:
-			}
+			f.readAgain()
-		case err = <-watcher.Errors:
+		case err := <-errs:
 			f.params.ProcessLog.Warn("watching the rule files failed",
 				"error", err.Error())
 		}
@@ -218,7 +241,9 @@ func (f *Files) logRead(count int) {
 }
 
 // read returns the rules of every rule file in dir, in the order of the
-// files' names, and then of their lines.
+// files' names, and then of their lines. A file whose name starts with a
+// dot, such as an editor's lock file .#50-app.rules, is not a rule file,
+// as a shell's *.rules would not match it.
 func read(dir string) ([]Rule, error) {
 	entries, err := os.ReadDir(dir)
 	if err != nil {
@@ -231,11 +256,12 @@ func read(dir string) ([]Rule, error) {
 	places := map[string]string{}
 
 	for _, entry := range entries {
-		if entry.IsDir() || filepath.Ext(entry.Name()) != extension {
+		name := entry.Name()
+		if entry.IsDir() || strings.HasPrefix(name, ".") || filepath.Ext(name) != extension {
 			continue
 		}
 
-		rules, err = readFile(filepath.Join(dir, entry.Name()), rules, places)
+		rules, err = readFile(filepath.Join(dir, name), rules, places)
 		if err != nil {
 			return nil, err
 		}
@@ -373,15 +399,14 @@ func value(target string, r *http.Request) string {
 	}
 }
 
-// pathAndQuery returns the path and the query of r as the client sent
+// pathAndQuery returns the target of r's request line, r.RequestURI, as
-// them, as the app is sent them: the target of its request line,
+// the client sent it, less any scheme and host: a target with a scheme
-// r.RequestURI, of which a target with a scheme gives what follows the
+// gives what follows the scheme and its :, and the host when // follows.
-// scheme and its :, and the host when // follows. So http://host/path, as
+// So http://host/path, as a client sends it to a proxy, gives /path, and
-// a client sends it to a proxy, gives /path, and so does http:/path,
+// so does http:/path, which Go reads as a target with a scheme and no
-// which Go reads as a target with a scheme and no host. r.URL is not
+// host. r.URL is not used: when the path holds a character it escapes,
-// used: when the path holds a character it escapes, such as \ or a
+// such as \ or a non-ASCII byte, it decodes the whole path and escapes it
-// non-ASCII byte, it decodes the whole path and escapes it again, so that
+// again, so that \ becomes %5C and %2e a dot.
-// \ becomes %5C and %2e a dot.
 func pathAndQuery(r *http.Request) string {
 	if !r.URL.IsAbs() {
 		return r.RequestURI

internal/rules/rules_test.go

@@ -164,6 +164,28 @@ func TestFilesReadInNameOrderThenLineOrder(t *testing.T) {
 	}
 }
 
+func TestFileWhoseNameStartsWithADotIsNotARuleFile(t *testing.T) {
+	t.Parallel()
+
+	dir := writeFiles(t, ruleFiles{firstFile: "probe path block ^/probe\n"})
+
+	// The lock file Emacs makes beside a file while it is edited: a link to
+	// nothing, which cannot be read.
+	err := os.Symlink("user@host.1234:1700000000", filepath.Join(dir, ".#"+firstFile))
+	if err != nil {
+		t.Fatalf("symlink: %v", err)
+	}
+
+	params, _ := newParams(dir)
+
+	files, err := rules.Load(params)
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+
+	wantMatched(t, files, get(t, "/probe"), "probe")
+}
+
 func TestFaultStopsTheStartNamingTheFileAndLine(t *testing.T) {
 	t.Parallel()
 
@@ -600,8 +622,8 @@ func (l processLog) waitFor(t *testing.T, msg string) map[string]any {
 }
 
 // waitUntil waits for the rule files to be read until done reports true,
-// as it does once they have been read after the test's last change. One
+// as it does once they have been read after the test's last change. They
-// change can be seen more than once, and so read more than once.
+// can be read before then too, as they are once Watch starts watching.
 func (l processLog) waitUntil(t *testing.T, done func() bool) {
 	t.Helper()
 

internal/rules/watch_internal_test.go

@@ -0,0 +1,162 @@
+package rules
+
+import (
+	"context"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"slices"
+	"testing"
+	"testing/synctest"
+	"time"
+
+	"github.com/fsnotify/fsnotify"
+)
+
+// The tests below run readAfterChanges in a synctest bubble, where time is
+// a clock of the test's own: time.Sleep moves it on at once, and
+// synctest.Wait returns once readAfterChanges waits again, so that every
+// reading due by then is done. The test sends the changes itself, as the
+// watch of a directory cannot run in a bubble.
+
+func TestFileWrittenInTwoPartsTakenInOnlyWhole(t *testing.T) {
+	t.Parallel()
+
+	synctest.Test(t, func(t *testing.T) {
+		dir := t.TempDir()
+		path := filepath.Join(dir, "50-app.rules")
+		writeFile(t, path, "first path block ^/first\n")
+		files := load(t, dir)
+		changes := run(t, files)
+
+		file, err := os.Create(path) //nolint:gosec // a file the test wrote
+		if err != nil {
+			t.Fatalf("create: %v", err)
+		}
+
+		defer func() {
+			_ = file.Close()
+		}()
+
+		// The first part ends in the middle of a ban rule's regex, which,
+		// read then, would ban every request.
+		write(t, file, "first path block ^/first\nprobe path ban ^/")
+
+		changes <- fsnotify.Event{Name: path, Op: fsnotify.Write}
+
+		time.Sleep(quietTime - time.Nanosecond)
+		synctest.Wait()
+		wantMatched(t, files, "/anything")
+
+		// The second part starts the wait again.
+		write(t, file, `\.env$`+"\n")
+
+		changes <- fsnotify.Event{Name: path, Op: fsnotify.Write}
+
+		time.Sleep(quietTime - time.Nanosecond)
+		synctest.Wait()
+		wantMatched(t, files, "/.env")
+
+		time.Sleep(time.Nanosecond)
+		synctest.Wait()
+		wantMatched(t, files, "/.env", "probe")
+		wantMatched(t, files, "/anything")
+	})
+}
+
+func TestEditSavedBeforeTheWatchStartsTakenIn(t *testing.T) {
+	t.Parallel()
+
+	synctest.Test(t, func(t *testing.T) {
+		dir := t.TempDir()
+		path := filepath.Join(dir, "50-app.rules")
+		writeFile(t, path, "first path block ^/first\n")
+		files := load(t, dir)
+
+		// Saved after Load read the files, and before the directory was
+		// watched, so that no change is seen for it.
+		writeFile(t, path, "first path block ^/edited\n")
+		run(t, files)
+		time.Sleep(quietTime)
+		synctest.Wait()
+		wantMatched(t, files, "/edited", "first")
+	})
+}
+
+// load loads the rules in dir.
+func load(t *testing.T, dir string) *Files {
+	t.Helper()
+
+	files, err := Load(Params{
+		Dir: dir, Enabled: true, ProcessLog: slog.New(slog.DiscardHandler),
+	})
+	if err != nil {
+		t.Fatalf("load: %v", err)
+	}
+
+	return files
+}
+
+// run runs files' readAfterChanges until the test ends, and returns the
+// channel that sends it changes.
+func run(t *testing.T, files *Files) chan<- fsnotify.Event {
+	t.Helper()
+
+	changes := make(chan fsnotify.Event)
+	ctx, stop := context.WithCancel(t.Context())
+	stopped := make(chan struct{})
+
+	go func() {
+		files.readAfterChanges(ctx, changes, nil)
+		close(stopped)
+	}()
+
+	t.Cleanup(func() {
+		stop()
+		<-stopped
+	})
+
+	return changes
+}
+
+// writeFile writes content to the file at path.
+func writeFile(t *testing.T, path, content string) {
+	t.Helper()
+
+	err := os.WriteFile(path, []byte(content), 0o600)
+	if err != nil {
+		t.Fatalf("write %s: %v", path, err)
+	}
+}
+
+// write writes text to the end of file.
+func write(t *testing.T, file *os.File, text string) {
+	t.Helper()
+
+	_, err := file.WriteString(text)
+	if err != nil {
+		t.Fatalf("write: %v", err)
+	}
+}
+
+// wantMatched checks the ids of the rules that a GET request for path
+// matches, in order.
+func wantMatched(t *testing.T, files *Files, path string, want ...string) {
+	t.Helper()
+
+	r := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
+		"http://app.example"+path, nil)
+
+	matched := files.Match(r)
+
+	got := make([]string, 0, len(matched))
+	for _, rule := range matched {
+		got = append(got, rule.ID)
+	}
+
+	if !slices.Equal(got, want) {
+		t.Errorf("%s matched %v, want %v", path, got, want)
+	}
+}

internal/smallwebwaf/smallwebwaf_test.go

@@ -343,8 +343,21 @@ func TestRuleFileAddedWhileRunningTakesEffect(t *testing.T) {
 
 	out := runUntilStopped(t, env, func(url string) {
 		wantGreeting(t, url)
-		saveUntilAnswered(t, filepath.Join(dir, "50-app.rules"),
+
-			"everything path block ^/\n", url, "203.0.113.9", http.StatusForbidden)
+		// Written once: each change would start the rule files' wait
+		// again. A file written before smallwebwaf watches the directory is
+		// read once it does.
+		err := os.WriteFile(filepath.Join(dir, "50-app.rules"),
+			[]byte("everything path block ^/\n"), 0o600)
+		if err != nil {
+			t.Fatalf("write the rule file: %v", err)
+		}
+
+		// As long as that takes, so that a slow test process cannot fail
+		// the test.
+		for statusFrom(t, url, "203.0.113.9") != http.StatusForbidden {
+			time.Sleep(pollInterval)
+		}
 	})
 	out.line(t, "action", "rule_blocked")
 }