Add web for Superday #124
Add web for Superday #124
Conversation
Greptile OverviewGreptile SummaryThis PR adds web browser support for PAM (Privileged Access Management) sessions by implementing a WebSocket-based proxy system with ECDH key exchange and AES-256-GCM encryption. Key Changes:
Critical Security Issues:
Additional Concerns:
Confidence Score: 1/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| var wsUpgrader = websocket.Upgrader{ | ||
| CheckOrigin: func(req *http.Request) bool { return true }, |
There was a problem hiding this comment.
Allowing all origins (CheckOrigin: func(req *http.Request) bool { return true }) disables CORS protection completely, exposing the WebSocket endpoint to requests from any origin. This creates a security vulnerability where malicious websites can establish WebSocket connections to your relay server.
While authentication is performed separately, this still allows unauthorized origins to attempt connections and potentially exploit timing attacks or resource exhaustion.
Consider restricting to trusted origins or implement origin validation:
| CheckOrigin: func(req *http.Request) bool { return true }, | |
| CheckOrigin: func(req *http.Request) bool { | |
| origin := req.Header.Get("Origin") | |
| // Add your trusted origins here | |
| return origin == "https://your-trusted-domain.com" | |
| }, |
|
|
||
| func corsMiddleware(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { | ||
| w.Header().Set("Access-Control-Allow-Origin", "*") |
There was a problem hiding this comment.
Wildcard CORS (Access-Control-Allow-Origin: *) allows any website to make requests to this endpoint, which could enable CSRF-style attacks or information disclosure.
| w.Header().Set("Access-Control-Allow-Origin", "*") | |
| w.Header().Set("Access-Control-Allow-Origin", "https://your-trusted-domain.com") |
| if creds, err := credentialsManager.GetPAMSessionCredentials(handshake.SessionID, time.Now().Add(timeUntilExpiry)); err == nil { | ||
| _ = creds | ||
| } |
There was a problem hiding this comment.
Unused code block - fetching credentials but immediately discarding the result with _ = creds. This appears to be dead code.
| if creds, err := credentialsManager.GetPAMSessionCredentials(handshake.SessionID, time.Now().Add(timeUntilExpiry)); err == nil { | |
| _ = creds | |
| } | |
| // Pre-fetch credentials to ensure session is still valid | |
| if _, err := credentialsManager.GetPAMSessionCredentials(handshake.SessionID, time.Now().Add(timeUntilExpiry)); err != nil { | |
| log.Error().Err(err).Str("sessionId", handshake.SessionID).Msg("Failed to validate session on expiry check") | |
| } |
| g.handleMTLSConnection(channel, firstByte) | ||
| case 0x00: | ||
| // ECDH magic byte - handle web proxy connection | ||
| virtualConn := &prefixedVirtualConnection{channel: channel} |
There was a problem hiding this comment.
Bug: Creating prefixedVirtualConnection without passing the firstByte that was read. This means the magic byte (0x00) won't be available to the PAM web proxy handler, potentially causing protocol issues.
| virtualConn := &prefixedVirtualConnection{channel: channel} | |
| virtualConn := newPrefixedVirtualConnection(firstByte, channel) |
| parsed, parseErr := url.Parse(credentials.Url) | ||
| if parseErr != nil { | ||
| return fmt.Errorf("failed to parse URL: %w", parseErr) | ||
| } |
There was a problem hiding this comment.
Verify API validates credentials.Url to prevent SSRF (e.g., blocking internal IPs like http://localhost:6443, http://169.254.169.254).
1 participant
Description 📣
Add web for Superday
Type ✨
Tests 🛠️
# Here's some code block to paste some code snippets