Fix: Address Concurrency Issues and Document Findings

AUDIT-CONCURRENCY.md

@@ -0,0 +1,43 @@
+# Concurrency and Race Condition Analysis (`AUDIT-CONCURRENCY.md`)
+
+## 1. Executive Summary
+
+A concurrency audit was performed on the codebase to identify potential race conditions and thread-safety issues. The audit focused on encryption operations, connection pooling, cache synchronization, and key management.
+
+A critical data race was identified in the lazy initialization of the RSA and PGP services within the `pkg/crypt` package. This could lead to unpredictable behavior and resource leaks under concurrent use.
+
+The issue was resolved by implementing thread-safe initialization using the `sync.Once` primitive from the Go standard library.
+
+## 2. Methodology
+
+The audit consisted of the following steps:
+
+1.  **Static Analysis:** A manual review of the codebase was conducted, focusing on areas with high potential for concurrency issues, such as the `pkg/crypt` package.
+2.  **Dynamic Analysis:** The Go race detector (`go test -race ./...`) was used to identify data races during test execution. A new test case was created to specifically trigger the suspected race condition.
+
+## 3. Findings
+
+### 3.1. Data Race in Lazy Initialization
+
+-   **Location:** `pkg/crypt/crypt.go`, in the `ensureRSA()` and `ensurePGP()` methods.
+-   **Description:** The `ensureRSA()` and `ensurePGP()` methods used a non-thread-safe `if s.rsa == nil` check for lazy initialization. This created a race condition where multiple goroutines could enter the initialization block simultaneously, leading to multiple service instantiations and overwriting the service pointer.
+-   **Impact:** This could cause unpredictable behavior, memory leaks, and potentially panics if the service initialization had side effects.
+-   **Reproduction:** The race condition was reliably reproduced by creating a new test case (`pkg/crypt/race_test.go`) that called the `ensure` methods from multiple goroutines in parallel.
+
+## 4. Remediation
+
+The data race was remediated by using `sync.Once` to ensure that the initialization of the `rsa.Service` and `pgp.Service` happens exactly once, even when called from multiple goroutines.
+
+The following changes were made to `pkg/crypt/crypt.go`:
+
+1.  Added `rsaOnce sync.Once` and `pgpOnce sync.Once` fields to the `Service` struct.
+2.  Modified `ensureRSA()` to use `s.rsaOnce.Do()`.
+3.  Modified `ensurePGP()` to use `s.pgpOnce.Do()`.
+
+## 5. Verification
+
+The fix was verified by running the test suite with the race detector (`go test -race ./...`). The tests passed without any race warnings, confirming that the fix is effective. The new test case in `pkg/crypt/race_test.go` now serves as a regression test for this specific issue.
+
+## 6. Conclusion
+
+The audit identified and remediated a critical data race in the `pkg/crypt` package. The codebase is now more robust and safe for concurrent use. No other concurrency issues were identified in the audited areas.

pkg/crypt/crypt.go

@@ -3,13 +3,14 @@ package crypt
 import (
 	"crypto/md5"
 	"crypto/sha1"
-	"errors"
 	"crypto/sha256"
 	"crypto/sha512"
 	"encoding/binary"
 	"encoding/hex"
+	"errors"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/Snider/Enchantrix/pkg/crypt/std/lthn"
 	"github.com/Snider/Enchantrix/pkg/crypt/std/pgp"
@@ -21,6 +22,9 @@ import (
 type Service struct {
 	rsa *rsa.Service
 	pgp *pgp.Service
+
+	rsaOnce sync.Once
+	pgpOnce sync.Once
 }
 
 // NewService creates a new crypt Service and initialises its embedded services.
@@ -165,9 +169,9 @@ func (s *Service) Fletcher64(payload string) uint64 {
 
 // ensureRSA initializes the RSA service if it is not already.
 func (s *Service) ensureRSA() {
-	if s.rsa == nil {
+	s.rsaOnce.Do(func() {
 		s.rsa = rsa.NewService()
-	}
+	})
 }
 
 // GenerateRSAKeyPair creates a new RSA key pair.
@@ -192,9 +196,9 @@ func (s *Service) DecryptRSA(privateKey, ciphertext, label []byte) ([]byte, erro
 
 // ensurePGP initializes the PGP service if it is not already.
 func (s *Service) ensurePGP() {
-	if s.pgp == nil {
+	s.pgpOnce.Do(func() {
 		s.pgp = pgp.NewService()
-	}
+	})
 }
 
 // GeneratePGPKeyPair creates a new PGP key pair.

pkg/crypt/race_test.go

@@ -0,0 +1,26 @@
+package crypt
+
+import (
+	"sync"
+	"testing"
+)
+
+func TestServiceRace(t *testing.T) {
+	s := &Service{}
+	var wg sync.WaitGroup
+
+	//These goroutines will race to initialize the pgp and rsa services.
+	for i := 0; i < 10; i++ {
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			s.ensurePGP()
+		}()
+		go func() {
+			defer wg.Done()
+			s.ensureRSA()
+		}()
+	}
+
+	wg.Wait()
+}