Integrate security-hardening (#1) + fork-improvements (#2) from @Michm0TheH00d#4
Merged
Conversation
Syslog fields are attacker-controlled network input. Values beginning with '=', '+', '-' or '@' are interpreted as formulas by Excel and LibreOffice when the exported CSV is opened, enabling data exfiltration or command execution via DDE on the analyst's workstation. Neutralize hostname, app name, proc id and message fields by prefixing a single quote when they start with a formula trigger character, including triggers smuggled behind leading tab/CR characters. Timestamp, severity/facility labels and protocol are app-generated; source IP comes from the network stack, so these remain untouched.
The previous lexicographic comparison would report "1.10.0" as older than "1.9.0", so the update check would silently stop working once a double-digit version component is released. Compare components as integers, falling back to string comparison only for non-numeric components such as pre-release suffixes.
…ompares DisableEncryption skipped password verification entirely when the in-memory session password was empty — the state after an app restart with a still-locked encrypted database. Any caller could then flip the encryption flag off and desynchronize config state from the encrypted on-disk database. Now, disabling (or changing the password) while encryption is enabled requires a prior successful unlock, and the password checks use crypto/subtle constant-time comparison.
Previously all three listeners (UDP, TCP, TLS) bound unconditionally to all interfaces. On multi-homed hosts — e.g. an engineering laptop attached to both an office network and a control-system network — this exposes the syslog service on networks it was never meant to face. Add an optional BindAddress to ServerConfig, validated as an IP literal, and use net.JoinHostPort so IPv6 addresses work. Empty keeps the previous listen-on-all behavior. Exposed in the server controls UI with translations for all supported locales.
Each accepted connection spawns a handler goroutine with a 64 KiB scan buffer and no upper bound, so a connection-flooding client could bind unbounded memory and goroutines. Introduce a semaphore (512 slots shared by TCP and TLS) acquired non-blockingly on accept; connections beyond the limit are closed immediately and logged. Handlers release into the semaphore instance they were admitted under, so a server restart cannot cause cross-instance releases.
Any host on the local segment could deliver messages, trigger alert rules, and flood the retention window with junk that displaces real events. Add an optional allowlist of IP literals and CIDR ranges, validated in ValidateServerConfig and compiled once at server start. UDP datagrams from non-allowed sources are dropped; TCP/TLS connections are closed on accept before consuming a connection slot. An empty list preserves the previous allow-all behavior. The UI hint documents explicitly that UDP source addresses are spoofable, so the allowlist is network hygiene rather than authentication — mutual TLS remains the mechanism for authenticated senders.
handleTCPConnection blocks in scanner.Scan() with a five-minute read deadline. Cancelling the context does not interrupt an in-progress read, so Stop (which waits on the shared WaitGroup) could block for up to tcpReadTimeout when an idle TCP/TLS connection was open. Track active connections in a map and close them in Stop after the listeners are closed and before waiting on the WaitGroup, unblocking their reads immediately. trackConn returns false once the server is stopping so a connection accepted during shutdown is closed cleanly.
The failed-attempt counter lived only in memory and, on reaching the limit, quit the application. Both are weak: relaunching reset the counter (so the limit was trivially bypassable), and force-quitting is hostile to a legitimate user who simply mistyped. Persist failed attempts and a lockout deadline in the config file. After five consecutive failures an exponential backoff (30s doubling, capped at 15m) is enforced and survives restarts; a successful unlock clears the state. The shift is overflow-guarded so the cap always holds. The unlock screen now reads the authoritative lockout window from the backend and counts it down instead of maintaining its own counter.
A generated CA lived only in memory, so after a restart the CA — and the ability to sign further device certificates against it — was lost. This is impractical for the intended workflow of provisioning multiple devices over time against one trusted root. Persist the CA certificate and key in the config directory. When encryption is enabled the bundle is stored with the same AES-256-GCM/Argon2id scheme as the log database (loaded after unlock); otherwise it is written as plaintext PEM at 0600, and the user is warned that the key rests unencrypted. Enabling/disabling encryption and changing the password re-persist the CA under the new protection. Adds reusable EncryptBytes/DecryptBytes to the storage crypto package (same on-disk format as the file API) to avoid duplicating the crypto.
TCP and TLS messages were split on LF only (RFC 6587 non-transparent framing). RFC 5425 mandates octet counting for syslog over TLS, so many devices send "MSG-LEN SP SYSLOG-MSG" frames that line splitting mangles — and any message legitimately containing a newline was truncated. Add a bufio.SplitFunc that detects octet counting (digit run + space + declared byte length) and otherwise falls back to LF framing, so both wire formats interoperate on the same port. The declared length is bounded by the scan buffer to prevent a hostile MSG-LEN from forcing an unbounded buffer; CR/LF between frames is skipped and trailing CR is stripped from LF-framed messages. The split function is dependency-free and covered by table tests (single/multiple octet frames, embedded newline, LF and CRLF framing, mixed streams, digit-led non-counted messages, incomplete frame at EOF).
Correctif appliqué par-dessus le merge de fork-improvements (@Michm0TheH00d, PR #2 qui englobe PR #1) pour faire passer tous les checks du projet. - Régénère les bindings Wails (frontend/wailsjs) : models.ServerConfig avait gagné bindAddress/allowedSources côté Go sans régénération, ce qui cassait svelte-check (3 erreurs de type dans api.ts). Expose aussi les nouvelles méthodes GetUnlockLockoutSeconds / LoadPersistedCA. - Ajoute les <input> bind-address et allowed-sources manquants dans ServerControls.svelte (les champs étaient dans le state et stylés mais jamais rendus) et complète le config par défaut de serverStatus dans stores.ts. Corrige 1 erreur de type + 2 warnings "Unused CSS selector" et rend surtout la fonctionnalité accessible depuis l'UI. - Corrige TestStop_ClosesIdleTCPConnection : il utilisait TCPPort=0, rejeté par ValidateServerConfig ; réserve un vrai port libre et bind sur loopback. - Garde l'assertion de permission 0600 de castore_test derrière GOOS != "windows" (Windows ne préserve pas les bits Unix) : go test passe désormais sur Windows/macOS. go vet ./..., go test ./... et svelte-check --fail-on-warnings passent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integrates the fork contributions from @Michm0TheH00d — security-hardening (#1) and fork-improvements (#2) — on top of
main, plus a follow-up commit that gets every project check green. PR #2 already contained all of #1's commits (same SHAs), so this single branch supersedes both #1 and #2.Included (authored by @Michm0TheH00d)
1.10.0 > 1.9.0)Stopto avoid a shutdown hangFollow-up fixes (to green up CI)
frontend/wailsjs) soServerConfigexposesbindAddress/allowedSourcesand the newGetUnlockLockoutSeconds/LoadPersistedCAmethods are typed — the Go model change had not been regenerated, breakingsvelte-check(3 type errors inapi.ts).ServerControls.svelte(they were in component state and styled but never rendered) and completed theserverStatusdefault config instores.ts— fixes 1 type error + 2 unused-CSS warnings and makes the feature actually reachable from the UI.TestStop_ClosesIdleTCPConnection: it usedTCPPort=0, whichValidateServerConfigrejects; it now reserves a real free port on loopback.0600permission assertion incastore_testbehindGOOS != "windows"(Windows does not preserve Unix permission bits), sogo testpasses on Windows/macOS too.Checks
go vet ./...✅go test ./...✅ (all packages)svelte-check --fail-on-warnings✅ — 0 errors, 0 warningsnpm run build(Vite) ✅Supersedes #1 and #2.