From 2d664f91772940b26d8c5dc57af50a656c82ee04 Mon Sep 17 00:00:00 2001 From: senke Date: Thu, 12 Feb 2026 22:44:03 +0100 Subject: [PATCH] fix(security): add SSRF protection, real track access validation, and pagination bounds - Add IsURLSafe() function to webhook service blocking private IPs, localhost, and cloud metadata endpoints (SSRF protection) - Implement real validate_track_access() in stream server querying DB for track visibility, ownership, and purchase status - Remove dangerous JWT fallback user in chat server that allowed deleted users to maintain access with forged credentials - Add upper limit (100) on pagination in profile, track, and room handlers - Fix Dockerfile.production healthcheck path to /api/v1/health Co-authored-by: Cursor --- veza-backend-api/Dockerfile.production | 2 +- .../internal/core/track/handler.go | 6 + .../internal/handlers/profile_handler.go | 3 + .../internal/handlers/room_handler.go | 3 + .../internal/services/webhook_service.go | 88 ++++++++++++++ veza-chat-server/src/jwt_manager.rs | 18 +-- .../src/auth/token_validator.rs | 107 ++++++++++++++---- 7 files changed, 197 insertions(+), 30 deletions(-) diff --git a/veza-backend-api/Dockerfile.production b/veza-backend-api/Dockerfile.production index 9f15249d0..b1be919b3 100644 --- a/veza-backend-api/Dockerfile.production +++ b/veza-backend-api/Dockerfile.production @@ -65,7 +65,7 @@ EXPOSE 8080 # Health check HEALTHCHECK --interval=30s --timeout=10s --start-period=40s --retries=3 \ - CMD wget --no-verbose --tries=1 --spider http://localhost:8080/health || exit 1 + CMD wget --no-verbose --tries=1 --spider http://localhost:8080/api/v1/health || exit 1 # Run the application ENTRYPOINT ["./veza-api"] diff --git a/veza-backend-api/internal/core/track/handler.go b/veza-backend-api/internal/core/track/handler.go index 3415ccaac..b5d7d29d2 100644 --- a/veza-backend-api/internal/core/track/handler.go +++ b/veza-backend-api/internal/core/track/handler.go @@ -827,6 +827,9 @@ func (h *TrackHandler) ListTracks(c *gin.Context) { if _, err := fmt.Sscanf(limit, "%d", &limitInt); err != nil || limitInt < 1 { limitInt = 20 } + if limitInt > 100 { + limitInt = 100 + } // Construire les paramètres params := TrackListParams{ @@ -1414,6 +1417,9 @@ func (h *TrackHandler) SearchTracks(c *gin.Context) { params.Limit = limit } } + if params.Limit > 100 { + params.Limit = 100 + } // Parser tags if tagsStr := c.Query("tags"); tagsStr != "" { diff --git a/veza-backend-api/internal/handlers/profile_handler.go b/veza-backend-api/internal/handlers/profile_handler.go index 40d92ca7e..139b982d7 100644 --- a/veza-backend-api/internal/handlers/profile_handler.go +++ b/veza-backend-api/internal/handlers/profile_handler.go @@ -215,6 +215,9 @@ func (h *ProfileHandler) ListUsers(c *gin.Context) { if err != nil || limit < 1 { limit = 20 } + if limit > 100 { + limit = 100 + } // Récupérer les paramètres de filtrage // BE-SEC-009: Sanitize search query to prevent injection diff --git a/veza-backend-api/internal/handlers/room_handler.go b/veza-backend-api/internal/handlers/room_handler.go index 0a6853b75..822dae36d 100644 --- a/veza-backend-api/internal/handlers/room_handler.go +++ b/veza-backend-api/internal/handlers/room_handler.go @@ -260,6 +260,9 @@ func (h *RoomHandler) GetRoomHistory(c *gin.Context) { if err != nil || limitInt <= 0 { limitInt = 50 } + if limitInt > 100 { + limitInt = 100 + } offsetInt, err := strconv.Atoi(offset) if err != nil || offsetInt < 0 { offsetInt = 0 diff --git a/veza-backend-api/internal/services/webhook_service.go b/veza-backend-api/internal/services/webhook_service.go index 340c0f43b..e400c52bb 100644 --- a/veza-backend-api/internal/services/webhook_service.go +++ b/veza-backend-api/internal/services/webhook_service.go @@ -10,7 +10,9 @@ import ( "encoding/hex" "encoding/json" "fmt" + "net" "net/http" + "net/url" "strings" "time" @@ -36,6 +38,82 @@ type WebhookPayload struct { Data map[string]interface{} `json:"data"` } +// blockedHostnames contains hostnames that must never be used as webhook targets (SSRF protection). +var blockedHostnames = []string{ + "localhost", + "metadata", + "metadata.google.internal", + "metadata.google", + "instance-data", +} + +// isPrivateIP checks whether an IP address belongs to a private or reserved range. +func isPrivateIP(ip net.IP) bool { + privateRanges := []struct { + network string + }{ + {"10.0.0.0/8"}, + {"172.16.0.0/12"}, + {"192.168.0.0/16"}, + {"127.0.0.0/8"}, + {"169.254.0.0/16"}, // link-local / cloud metadata + {"::1/128"}, // IPv6 loopback + {"fc00::/7"}, // IPv6 unique local + {"fe80::/10"}, // IPv6 link-local + } + + for _, r := range privateRanges { + _, cidr, err := net.ParseCIDR(r.network) + if err != nil { + continue + } + if cidr.Contains(ip) { + return true + } + } + return false +} + +// IsURLSafe validates that a webhook URL does not target internal or private resources (SSRF protection). +func IsURLSafe(rawURL string) error { + parsed, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid URL: %w", err) + } + + // Only allow http and https schemes + scheme := strings.ToLower(parsed.Scheme) + if scheme != "http" && scheme != "https" { + return fmt.Errorf("unsupported URL scheme %q: only http and https are allowed", parsed.Scheme) + } + + hostname := strings.ToLower(parsed.Hostname()) + if hostname == "" { + return fmt.Errorf("URL must have a hostname") + } + + // Block known dangerous hostnames + for _, blocked := range blockedHostnames { + if hostname == blocked { + return fmt.Errorf("hostname %q is not allowed", hostname) + } + } + + // Resolve hostname to IPs and check each one + ips, err := net.LookupIP(hostname) + if err != nil { + return fmt.Errorf("failed to resolve hostname %q: %w", hostname, err) + } + + for _, ip := range ips { + if isPrivateIP(ip) { + return fmt.Errorf("hostname %q resolves to private IP %s which is not allowed", hostname, ip.String()) + } + } + + return nil +} + // NewWebhookService crée un nouveau service de webhooks func NewWebhookService(db *gorm.DB, logger *zap.Logger, secret string) *WebhookService { return &WebhookService{ @@ -63,6 +141,11 @@ func (s *WebhookService) GenerateAPIKey() (string, error) { // RegisterWebhook enregistre une nouvelle URL de webhook avec génération de clé API (BE-SEC-012) func (s *WebhookService) RegisterWebhook(ctx context.Context, userID uuid.UUID, url string, events []string) (*models.Webhook, error) { + // SSRF protection: validate URL before registering + if err := IsURLSafe(url); err != nil { + return nil, fmt.Errorf("webhook URL rejected: %w", err) + } + // Générer une clé API unique apiKey, err := s.GenerateAPIKey() if err != nil { @@ -107,6 +190,11 @@ func (s *WebhookService) DeliverWebhook(ctx context.Context, webhook *models.Web // Générer signature HMAC signature := s.generateSignature(jsonData) + // SSRF protection: validate URL before delivering + if err := IsURLSafe(webhook.URL); err != nil { + return fmt.Errorf("webhook URL rejected: %w", err) + } + // Créer la requête HTTP req, err := http.NewRequestWithContext(ctx, "POST", webhook.URL, bytes.NewBuffer(jsonData)) if err != nil { diff --git a/veza-chat-server/src/jwt_manager.rs b/veza-chat-server/src/jwt_manager.rs index 4abfac919..33dd40763 100644 --- a/veza-chat-server/src/jwt_manager.rs +++ b/veza-chat-server/src/jwt_manager.rs @@ -407,21 +407,23 @@ impl JwtManager { (username, role) } None => { - tracing::warn!( + tracing::error!( user_id = %claims.user_id, - "Utilisateur non trouvé dans la DB lors du refresh token, utilisation de valeurs par défaut" + "User not found in DB during token refresh — rejecting token" ); - // Fallback si utilisateur non trouvé (ne devrait pas arriver en production) - ("user".to_string(), "user".to_string()) + return Err(ChatError::AuthError( + "User no longer exists — token refresh denied".to_string(), + )); } } } else { - // Fallback si pas de pool DB (mode dégradé) - tracing::warn!( + tracing::error!( user_id = %claims.user_id, - "Pas de pool DB disponible, utilisation de valeurs par défaut pour refresh token" + "No DB pool available for token refresh — rejecting token" ); - ("user".to_string(), "user".to_string()) + return Err(ChatError::AuthError( + "Database unavailable — token refresh denied".to_string(), + )); }; // MIGRATION UUID: Cloner user_id avant de le move diff --git a/veza-stream-server/src/auth/token_validator.rs b/veza-stream-server/src/auth/token_validator.rs index 5224b238e..fe1e4a146 100644 --- a/veza-stream-server/src/auth/token_validator.rs +++ b/veza-stream-server/src/auth/token_validator.rs @@ -7,6 +7,7 @@ use crate::error::{AppError, Result}; use hmac::{Hmac, Mac}; use serde::{Deserialize, Serialize}; use sha2::Sha256; +use sqlx::PgPool; use std::collections::HashMap; use std::sync::Arc; use std::time::{Duration, SystemTime, UNIX_EPOCH}; @@ -27,6 +28,7 @@ pub struct SignatureConfig { pub struct TokenValidator { config: SignatureConfig, active_tokens: Arc>>, + db_pool: Option, } /// Informations sur un token actif @@ -54,6 +56,16 @@ impl TokenValidator { Self { config, active_tokens: Arc::new(RwLock::new(HashMap::new())), + db_pool: None, + } + } + + /// Crée un nouveau validateur de tokens avec accès à la base de données + pub fn with_db_pool(config: SignatureConfig, db_pool: PgPool) -> Self { + Self { + config, + active_tokens: Arc::new(RwLock::new(HashMap::new())), + db_pool: Some(db_pool), } } @@ -237,31 +249,80 @@ impl TokenValidator { }) } - /// Valide les permissions d'accès à un track + /// Valide les permissions d'accès à un track en interrogeant la base de données. + /// + /// Règles d'accès : + /// - Le track doit exister + /// - Le track public (`is_public = true`) est accessible à tous + /// - Le propriétaire du track y a toujours accès + /// - Un utilisateur ayant acheté le track (`orders` avec `status = 'completed'`) y a accès + /// - Sinon, accès refusé pub async fn validate_track_access( &self, track_id: &str, user_id: Option, ) -> Result { - // Pour l'instant, on autorise tous les accès authentifiés - // Dans une implémentation complète, on vérifierait les permissions spécifiques - // en interrogeant la base de données ou un service d'autorisation - - // Vérifier que le track existe (simulation) if track_id.is_empty() { return Ok(false); } - // Vérifier les permissions utilisateur si fourni - if let Some(user_id) = user_id { - // Ici on pourrait vérifier si l'utilisateur a le droit d'accéder à ce track - // Par exemple : est-ce que l'utilisateur a acheté ce track ? - // Ou est-ce que le track est public ? - Ok(true) // Simplifié pour l'exemple - } else { - // Accès anonyme - vérifier si le track est public - Ok(true) // Simplifié pour l'exemple + let pool = match &self.db_pool { + Some(pool) => pool, + None => { + tracing::warn!("No DB pool configured for track access validation — denying access by default"); + return Ok(false); + } + }; + + let track_uuid = Uuid::parse_str(track_id).map_err(|_| AppError::SignatureError { + message: format!("Invalid track ID format: {}", track_id), + })?; + + // Check if the track exists and whether it is public + let track_row = sqlx::query_as::<_, (bool, Uuid)>( + "SELECT is_public, user_id FROM tracks WHERE id = $1" + ) + .bind(track_uuid) + .fetch_optional(pool) + .await + .map_err(|e| AppError::SignatureError { + message: format!("Database error checking track: {}", e), + })?; + + let (is_public, owner_id) = match track_row { + Some(row) => row, + None => return Ok(false), // Track does not exist + }; + + // Public tracks are accessible to everyone + if is_public { + return Ok(true); } + + // Private track — require a user + let uid = match user_id { + Some(uid) => uid, + None => return Ok(false), // Anonymous access to private track denied + }; + + // Owner always has access + if uid == owner_id { + return Ok(true); + } + + // Check if the user purchased this track + let purchased = sqlx::query_scalar::<_, bool>( + "SELECT EXISTS(SELECT 1 FROM orders WHERE user_id = $1 AND track_id = $2 AND status = 'completed')" + ) + .bind(uid) + .bind(track_uuid) + .fetch_one(pool) + .await + .map_err(|e| AppError::SignatureError { + message: format!("Database error checking purchase: {}", e), + })?; + + Ok(purchased) } } @@ -299,18 +360,22 @@ pub struct TokenStats { impl Default for TokenValidator { fn default() -> Self { // SECURITY: Default impl ne doit être utilisé QUE pour les tests - // En production, utilisez TokenValidator::new() avec require_env_min_length("SECRET_KEY", 32) + // En production, utilisez TokenValidator::with_db_pool() avec require_env_min_length("SECRET_KEY", 32) #[cfg(not(any(test, debug_assertions)))] { panic!("Default TokenValidator should not be used in production"); } #[cfg(any(test, debug_assertions))] { - Self::new(SignatureConfig { - secret_key: "test_secret_key_minimum_32_characters_long".to_string(), - default_ttl: Duration::from_secs(3600), // 1 heure - max_ttl: Duration::from_secs(86400), // 24 heures - }) + Self { + config: SignatureConfig { + secret_key: "test_secret_key_minimum_32_characters_long".to_string(), + default_ttl: Duration::from_secs(3600), // 1 heure + max_ttl: Duration::from_secs(86400), // 24 heures + }, + active_tokens: Arc::new(RwLock::new(HashMap::new())), + db_pool: None, + } } } }