Repository navigation
fix: encrypt the cookie in sessions - #225
Conversation
WalkthroughThe changes introduce a key derivation process to generate separate HMAC and encryption secrets from a main secret using HKDF. The authentication configuration and session management are updated to use these distinct keys. Error handling is improved when retrieving sessions, and a utility function for key derivation is added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GinContext
participant Auth
participant SessionStore
participant Utils
User->>GinContext: Sends request
GinContext->>Auth: GetSession()
Auth->>SessionStore: Create store with HMACSecret, EncryptionSecret
Auth->>SessionStore: Get session
alt Session retrieval fails
Auth->>GinContext: Clear session cookie
Auth->>SessionStore: Retry Get session
alt Retry fails
Auth->>GinContext: Log error, return error
end
end
Auth->>GinContext: Return session
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/root.go(2 hunks)internal/auth/auth.go(2 hunks)internal/types/config.go(1 hunks)internal/utils/utils.go(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
cmd/root.go (1)
internal/utils/utils.go (1)
DeriveKey(413-440)
internal/auth/auth.go (2)
internal/types/config.go (1)
Config(4-39)internal/constants/constants.go (1)
SessionCookieName(17-17)
🔇 Additional comments (4)
internal/types/config.go (1)
88-89: LGTM! Good security practice to separate HMAC and encryption keys.Using separate keys for HMAC and encryption follows cryptographic best practices and improves security by ensuring that each key serves a single purpose.
cmd/root.go (2)
77-84: LGTM! Proper implementation of key derivation.The code correctly derives separate HMAC and encryption keys using distinct context strings. Error handling is appropriate and follows the established pattern in the codebase.
133-134: LGTM! Correct assignment of derived secrets.The AuthConfig properly uses the derived HMAC and encryption secrets, completing the transition to separate cryptographic keys.
internal/auth/auth.go (1)
36-36: LGTM! Correct usage of separate HMAC and encryption keys.The cookie store properly uses the derived HMAC and encryption secrets in the correct order for the gorilla/sessions library.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/utils/utils.go (1)
413-440: Good implementation with proper fix for previous truncation issue.The
DeriveKeyfunction correctly implements HKDF-based key derivation and addresses the previous concern about key truncation. The function now derives 24 bytes and returns the full base64 encoded string (32 characters), preserving all entropy.A few observations:
- ✅ Fixed truncation issue: 24 bytes encode to exactly 32 base64 characters without truncation
- ✅ Proper error handling: Checks for HKDF errors and empty keys
- ✅ Security practices: Uses HKDF with SHA-256 and validates the derived key
The comment about not using a salt is actually acceptable in this context since you're deriving different keys using different
infoparameters, which provides the necessary differentiation.Consider adding input validation for empty parameters:
func DeriveKey(secret string, info string) (string, error) { + // Validate inputs + if secret == "" { + return "", errors.New("secret cannot be empty") + } + if info == "" { + return "", errors.New("info cannot be empty") + } + // Create hashing function hash := sha256.New
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/api/api_test.go(1 hunks)internal/auth/auth.go(2 hunks)internal/utils/utils.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/api/api_test.go
- internal/auth/auth.go
🔇 Additional comments (1)
internal/utils/utils.go (1)
4-5: LGTM! New imports are appropriate for HKDF implementation.The added imports (
bytes,crypto/sha256,io,golang.org/x/crypto/hkdf) are correctly used in the newDeriveKeyfunction for cryptographic key derivation.Also applies to: 8-8, 17-17
Summary by CodeRabbit