Security concerns and recommendations

[gpappsoft]

1. Sensitive Data in Source Code

Your keyrad.yaml contains a real client_secret. Never commit secrets to source control. Use environment variables or secret management tools.

2. TLS Verification

The option insecure_skip_tls_verify: true disables TLS verification. This is insecure and should only be used for testing. Warn users in documentation and code comments.

3. Challenge State Store

The challenge state store is in-memory and not time-limited. This could allow replay attacks if a state is reused.
Recommendation: Add expiration for challenge states (e.g., store a timestamp and clean up old entries).

4. Concurrency and Race Conditions

The challenge state store is not protected by a mutex. With multiple goroutines, this can lead to race conditions.
Recommendation: Use sync.Mutex or sync.Map for thread-safe access.

5. Logging Sensitive Data

Ensure you do not log secrets, passwords, or OTPs.
Review all log.Printf statements for accidental leaks.

6. Error Handling

Some errors are returned to the client with generic messages. Avoid leaking internal error details.

7. UDP Protocol

UDP is stateless and can be spoofed. Make sure to validate client IPs and secrets strictly.

8. HMAC-MD5 for Message-Authenticator

HMAC-MD5 is required by RADIUS, but MD5 is considered weak. If possible, prefer stronger algorithms for other cryptographic operations.

9. No Rate Limiting

There is no rate limiting or DoS protection. Consider adding limits per client IP.

10. Configurable Worker Count

The worker count is hardcoded. If set too high, it could exhaust system resources.

11. No Brute Force Protection

There is no lockout or delay for repeated failed authentication attempts.

12. No Input Validation

Validate all user input, especially usernames and passwords, to prevent injection attacks.

Summary of critical fixes:

• Protect challenge state store with a mutex.
• Add expiration to challenge states.
• Never log secrets or sensitive data.
• Warn about insecure TLS in docs and code.
• Consider rate limiting and brute force protection.