fix(security): add JWT nbf claim and env var for token secret
- commit
- 6f1f4f3313109fadc1ad892d48ededc8b5d869ce
- author
- bnema · · 2026-01-17 20:58 UTC
- parent
- 12ad37b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +152 -11
M internal/app/run.go +89 -11
| @@ -467,13 +467,44 @@ type InternalCredentials struct { | |||
| 467 | 467 | Password string `json:"password"` | |
| 468 | 468 | } | |
| 469 | 469 | ||
| 470 | + | // getSecureRuntimeDir returns a secure directory for runtime files. | |
| 471 | + | // Priority: XDG_RUNTIME_DIR > ~/.gordon/run | |
| 472 | + | func getSecureRuntimeDir() (string, error) { | |
| 473 | + | // Try XDG_RUNTIME_DIR first (typically /run/user/<uid> on Linux) | |
| 474 | + | if runtimeDir := os.Getenv("XDG_RUNTIME_DIR"); runtimeDir != "" { | |
| 475 | + | gordonDir := filepath.Join(runtimeDir, "gordon") | |
| 476 | + | if err := os.MkdirAll(gordonDir, 0700); err == nil { | |
| 477 | + | return gordonDir, nil | |
| 478 | + | } | |
| 479 | + | } | |
| 480 | + | ||
| 481 | + | // Fall back to ~/.gordon/run | |
| 482 | + | homeDir, err := os.UserHomeDir() | |
| 483 | + | if err != nil { | |
| 484 | + | return "", fmt.Errorf("failed to get home directory: %w", err) | |
| 485 | + | } | |
| 486 | + | ||
| 487 | + | gordonDir := filepath.Join(homeDir, ".gordon", "run") | |
| 488 | + | if err := os.MkdirAll(gordonDir, 0700); err != nil { | |
| 489 | + | return "", fmt.Errorf("failed to create runtime directory: %w", err) | |
| 490 | + | } | |
| 491 | + | ||
| 492 | + | return gordonDir, nil | |
| 493 | + | } | |
| 494 | + | ||
| 470 | 495 | // getInternalCredentialsFile returns the path to the internal credentials file. | |
| 496 | + | // SECURITY: Credentials are stored in a secure location with restricted permissions. | |
| 471 | 497 | func getInternalCredentialsFile() string { | |
| 472 | - | return filepath.Join(os.TempDir(), "gordon-internal-creds.json") | |
| 498 | + | runtimeDir, err := getSecureRuntimeDir() | |
| 499 | + | if err != nil { | |
| 500 | + | // Fall back to temp dir if we can't get secure dir (shouldn't happen) | |
| 501 | + | return filepath.Join(os.TempDir(), "gordon-internal-creds.json") | |
| 502 | + | } | |
| 503 | + | return filepath.Join(runtimeDir, "internal-creds.json") | |
| 473 | 504 | } | |
| 474 | 505 | ||
| 475 | - | // persistInternalCredentials saves the internal registry credentials to a temp file. | |
| 476 | - | // Security note: Credentials are stored in the system temp directory with 0600 permissions. | |
| 506 | + | // persistInternalCredentials saves the internal registry credentials to a secure file. | |
| 507 | + | // SECURITY: Credentials are stored in XDG_RUNTIME_DIR or ~/.gordon/run with 0600 permissions. | |
| 477 | 508 | // The file is cleaned up on graceful shutdown but may persist if Gordon crashes. | |
| 478 | 509 | // These credentials are for internal loopback communication only and are regenerated on each start. | |
| 479 | 510 | func persistInternalCredentials(username, password string) error { | |
| @@ -485,7 +516,15 @@ func persistInternalCredentials(username, password string) error { | |||
| 485 | 516 | if err != nil { | |
| 486 | 517 | return fmt.Errorf("failed to marshal credentials: %w", err) | |
| 487 | 518 | } | |
| 519 | + | ||
| 488 | 520 | credFile := getInternalCredentialsFile() | |
| 521 | + | ||
| 522 | + | // Ensure parent directory exists with secure permissions | |
| 523 | + | if err := os.MkdirAll(filepath.Dir(credFile), 0700); err != nil { | |
| 524 | + | return fmt.Errorf("failed to create credentials directory: %w", err) | |
| 525 | + | } | |
| 526 | + | ||
| 527 | + | // Write file with restrictive permissions (owner read/write only) | |
| 489 | 528 | if err := os.WriteFile(credFile, data, 0600); err != nil { | |
| 490 | 529 | return fmt.Errorf("failed to write credentials file: %w", err) | |
| 491 | 530 | } | |
| @@ -646,9 +685,25 @@ func loadTokenConfig(ctx context.Context, cfg Config, backend domain.SecretsBack | |||
| 646 | 685 | return secret, expiry, nil | |
| 647 | 686 | } | |
| 648 | 687 | ||
| 688 | + | // TokenSecretEnvVar is the environment variable for the JWT signing secret. | |
| 689 | + | // SECURITY: This takes priority over config file to allow secure secret injection. | |
| 690 | + | const TokenSecretEnvVar = "GORDON_AUTH_TOKEN_SECRET" //nolint:gosec // This is an env var name, not a credential | |
| 691 | + | ||
| 649 | 692 | func loadTokenSecret(ctx context.Context, cfg Config, backend domain.SecretsBackend, dataDir string, log zerowrap.Logger) ([]byte, error) { | |
| 693 | + | // SECURITY: Priority order for token secret: | |
| 694 | + | // 1. Environment variable (most secure - no disk exposure) | |
| 695 | + | // 2. Secrets backend (pass/sops - encrypted) | |
| 696 | + | // 3. Config file path (least preferred) | |
| 697 | + | ||
| 698 | + | // Check environment variable first | |
| 699 | + | if envSecret := os.Getenv(TokenSecretEnvVar); envSecret != "" { | |
| 700 | + | log.Debug().Msg("using token secret from environment variable") | |
| 701 | + | return []byte(envSecret), nil | |
| 702 | + | } | |
| 703 | + | ||
| 704 | + | // Fall back to config-specified path via secrets backend | |
| 650 | 705 | if cfg.Auth.TokenSecret == "" { | |
| 651 | - | return nil, fmt.Errorf("token_secret is required for token authentication") | |
| 706 | + | return nil, fmt.Errorf("token_secret is required for token authentication; set %s environment variable or configure auth.token_secret", TokenSecretEnvVar) | |
| 652 | 707 | } | |
| 653 | 708 | ||
| 654 | 709 | secret, err := loadSecret(ctx, backend, cfg.Auth.TokenSecret, dataDir, log) | |
| @@ -1079,19 +1134,32 @@ func readDeployRequest() (string, error) { | |||
| 1079 | 1134 | } | |
| 1080 | 1135 | ||
| 1081 | 1136 | // createPidFile creates a PID file for the Gordon process. | |
| 1137 | + | // SECURITY: Prefers secure locations (XDG_RUNTIME_DIR, ~/.gordon/run) over /tmp | |
| 1138 | + | // to prevent symlink attacks and unauthorized access. | |
| 1082 | 1139 | func createPidFile(log zerowrap.Logger) string { | |
| 1083 | 1140 | pid := os.Getpid() | |
| 1084 | 1141 | ||
| 1085 | - | locations := []string{ | |
| 1086 | - | "/tmp/gordon.pid", | |
| 1087 | - | filepath.Join(os.TempDir(), "gordon.pid"), | |
| 1142 | + | // SECURITY: Prioritize secure locations over /tmp | |
| 1143 | + | var locations []string | |
| 1144 | + | ||
| 1145 | + | // Try secure runtime directory first | |
| 1146 | + | if runtimeDir, err := getSecureRuntimeDir(); err == nil { | |
| 1147 | + | locations = append(locations, filepath.Join(runtimeDir, "gordon.pid")) | |
| 1088 | 1148 | } | |
| 1089 | 1149 | ||
| 1150 | + | // Fall back to home directory | |
| 1090 | 1151 | if homeDir, err := os.UserHomeDir(); err == nil { | |
| 1091 | - | locations = append(locations, filepath.Join(homeDir, ".gordon.pid")) | |
| 1152 | + | locations = append(locations, filepath.Join(homeDir, ".gordon", "gordon.pid")) | |
| 1092 | 1153 | } | |
| 1093 | 1154 | ||
| 1155 | + | // Last resort: /tmp (least secure due to world-writable) | |
| 1156 | + | locations = append(locations, filepath.Join(os.TempDir(), "gordon.pid")) | |
| 1157 | + | ||
| 1094 | 1158 | for _, location := range locations { | |
| 1159 | + | // Ensure parent directory exists with secure permissions | |
| 1160 | + | if err := os.MkdirAll(filepath.Dir(location), 0700); err != nil { | |
| 1161 | + | continue | |
| 1162 | + | } | |
| 1095 | 1163 | if err := os.WriteFile(location, []byte(fmt.Sprintf("%d", pid)), 0600); err == nil { | |
| 1096 | 1164 | log.Debug().Str("pid_file", location).Int("pid", pid).Msg("created PID file") | |
| 1097 | 1165 | return location | |
| @@ -1112,16 +1180,26 @@ func removePidFile(pidFile string, log zerowrap.Logger) { | |||
| 1112 | 1180 | } | |
| 1113 | 1181 | ||
| 1114 | 1182 | // findPidFile finds the Gordon PID file. | |
| 1183 | + | // Checks secure locations first, then falls back to legacy /tmp location. | |
| 1115 | 1184 | func findPidFile() string { | |
| 1116 | - | locations := []string{ | |
| 1117 | - | "/tmp/gordon.pid", | |
| 1118 | - | filepath.Join(os.TempDir(), "gordon.pid"), | |
| 1185 | + | var locations []string | |
| 1186 | + | ||
| 1187 | + | // Check secure runtime directory first | |
| 1188 | + | if runtimeDir, err := getSecureRuntimeDir(); err == nil { | |
| 1189 | + | locations = append(locations, filepath.Join(runtimeDir, "gordon.pid")) | |
| 1119 | 1190 | } | |
| 1120 | 1191 | ||
| 1192 | + | // Check home directory | |
| 1121 | 1193 | if homeDir, err := os.UserHomeDir(); err == nil { | |
| 1194 | + | locations = append(locations, filepath.Join(homeDir, ".gordon", "gordon.pid")) | |
| 1195 | + | // Legacy location for backward compatibility | |
| 1122 | 1196 | locations = append(locations, filepath.Join(homeDir, ".gordon.pid")) | |
| 1123 | 1197 | } | |
| 1124 | 1198 | ||
| 1199 | + | // Legacy /tmp locations for backward compatibility | |
| 1200 | + | locations = append(locations, filepath.Join(os.TempDir(), "gordon.pid")) | |
| 1201 | + | locations = append(locations, "/tmp/gordon.pid") | |
| 1202 | + | ||
| 1125 | 1203 | for _, location := range locations { | |
| 1126 | 1204 | if _, err := os.Stat(location); err == nil { | |
| 1127 | 1205 | return location | |
| @@ -467,13 +467,44 @@ type InternalCredentials struct { | |||||
| 467 | Password string `json:"password"` | 467 | Password string `json:"password"` | ||
| 468 | } | 468 | } | ||
| 469 | 469 | ||||
| 470 | + | // getSecureRuntimeDir returns a secure directory for runtime files. | |||
| 471 | + | // Priority: XDG_RUNTIME_DIR > ~/.gordon/run | |||
| 472 | + | func getSecureRuntimeDir() (string, error) { | |||
| 473 | + | // Try XDG_RUNTIME_DIR first (typically /run/user/<uid> on Linux) | |||
| 474 | + | if runtimeDir := os.Getenv("XDG_RUNTIME_DIR"); runtimeDir != "" { | |||
| 475 | + | gordonDir := filepath.Join(runtimeDir, "gordon") | |||
| 476 | + | if err := os.MkdirAll(gordonDir, 0700); err == nil { | |||
| 477 | + | return gordonDir, nil | |||
| 478 | + | } | |||
| 479 | + | } | |||
| 480 | + | ||||
| 481 | + | // Fall back to ~/.gordon/run | |||
| 482 | + | homeDir, err := os.UserHomeDir() | |||
| 483 | + | if err != nil { | |||
| 484 | + | return "", fmt.Errorf("failed to get home directory: %w", err) | |||
| 485 | + | } | |||
| 486 | + | ||||
| 487 | + | gordonDir := filepath.Join(homeDir, ".gordon", "run") | |||
| 488 | + | if err := os.MkdirAll(gordonDir, 0700); err != nil { | |||
| 489 | + | return "", fmt.Errorf("failed to create runtime directory: %w", err) | |||
| 490 | + | } | |||
| 491 | + | ||||
| 492 | + | return gordonDir, nil | |||
| 493 | + | } | |||
| 494 | + | ||||
| 470 | // getInternalCredentialsFile returns the path to the internal credentials file. | 495 | // getInternalCredentialsFile returns the path to the internal credentials file. | ||
| 496 | + | // SECURITY: Credentials are stored in a secure location with restricted permissions. | |||
| 471 | func getInternalCredentialsFile() string { | 497 | func getInternalCredentialsFile() string { | ||
| 472 | - | return filepath.Join(os.TempDir(), "gordon-internal-creds.json") | 498 | + | runtimeDir, err := getSecureRuntimeDir() |
| 499 | + | if err != nil { | |||
| 500 | + | // Fall back to temp dir if we can't get secure dir (shouldn't happen) | |||
| 501 | + | return filepath.Join(os.TempDir(), "gordon-internal-creds.json") | |||
| 502 | + | } | |||
| 503 | + | return filepath.Join(runtimeDir, "internal-creds.json") | |||
| 473 | } | 504 | } | ||
| 474 | 505 | ||||
| 475 | - | // persistInternalCredentials saves the internal registry credentials to a temp file. | 506 | + | // persistInternalCredentials saves the internal registry credentials to a secure file. |
| 476 | - | // Security note: Credentials are stored in the system temp directory with 0600 permissions. | 507 | + | // SECURITY: Credentials are stored in XDG_RUNTIME_DIR or ~/.gordon/run with 0600 permissions. |
| 477 | // The file is cleaned up on graceful shutdown but may persist if Gordon crashes. | 508 | // The file is cleaned up on graceful shutdown but may persist if Gordon crashes. | ||
| 478 | // These credentials are for internal loopback communication only and are regenerated on each start. | 509 | // These credentials are for internal loopback communication only and are regenerated on each start. | ||
| 479 | func persistInternalCredentials(username, password string) error { | 510 | func persistInternalCredentials(username, password string) error { | ||
| @@ -485,7 +516,15 @@ func persistInternalCredentials(username, password string) error { | |||||
| 485 | if err != nil { | 516 | if err != nil { | ||
| 486 | return fmt.Errorf("failed to marshal credentials: %w", err) | 517 | return fmt.Errorf("failed to marshal credentials: %w", err) | ||
| 487 | } | 518 | } | ||
| 519 | + | ||||
| 488 | credFile := getInternalCredentialsFile() | 520 | credFile := getInternalCredentialsFile() | ||
| 521 | + | ||||
| 522 | + | // Ensure parent directory exists with secure permissions | |||
| 523 | + | if err := os.MkdirAll(filepath.Dir(credFile), 0700); err != nil { | |||
| 524 | + | return fmt.Errorf("failed to create credentials directory: %w", err) | |||
| 525 | + | } | |||
| 526 | + | ||||
| 527 | + | // Write file with restrictive permissions (owner read/write only) | |||
| 489 | if err := os.WriteFile(credFile, data, 0600); err != nil { | 528 | if err := os.WriteFile(credFile, data, 0600); err != nil { | ||
| 490 | return fmt.Errorf("failed to write credentials file: %w", err) | 529 | return fmt.Errorf("failed to write credentials file: %w", err) | ||
| 491 | } | 530 | } | ||
| @@ -646,9 +685,25 @@ func loadTokenConfig(ctx context.Context, cfg Config, backend domain.SecretsBack | |||||
| 646 | return secret, expiry, nil | 685 | return secret, expiry, nil | ||
| 647 | } | 686 | } | ||
| 648 | 687 | ||||
| 688 | + | // TokenSecretEnvVar is the environment variable for the JWT signing secret. | |||
| 689 | + | // SECURITY: This takes priority over config file to allow secure secret injection. | |||
| 690 | + | const TokenSecretEnvVar = "GORDON_AUTH_TOKEN_SECRET" //nolint:gosec // This is an env var name, not a credential | |||
| 691 | + | ||||
| 649 | func loadTokenSecret(ctx context.Context, cfg Config, backend domain.SecretsBackend, dataDir string, log zerowrap.Logger) ([]byte, error) { | 692 | func loadTokenSecret(ctx context.Context, cfg Config, backend domain.SecretsBackend, dataDir string, log zerowrap.Logger) ([]byte, error) { | ||
| 693 | + | // SECURITY: Priority order for token secret: | |||
| 694 | + | // 1. Environment variable (most secure - no disk exposure) | |||
| 695 | + | // 2. Secrets backend (pass/sops - encrypted) | |||
| 696 | + | // 3. Config file path (least preferred) | |||
| 697 | + | ||||
| 698 | + | // Check environment variable first | |||
| 699 | + | if envSecret := os.Getenv(TokenSecretEnvVar); envSecret != "" { | |||
| 700 | + | log.Debug().Msg("using token secret from environment variable") | |||
| 701 | + | return []byte(envSecret), nil | |||
| 702 | + | } | |||
| 703 | + | ||||
| 704 | + | // Fall back to config-specified path via secrets backend | |||
| 650 | if cfg.Auth.TokenSecret == "" { | 705 | if cfg.Auth.TokenSecret == "" { | ||
| 651 | - | return nil, fmt.Errorf("token_secret is required for token authentication") | 706 | + | return nil, fmt.Errorf("token_secret is required for token authentication; set %s environment variable or configure auth.token_secret", TokenSecretEnvVar) |
| 652 | } | 707 | } | ||
| 653 | 708 | ||||
| 654 | secret, err := loadSecret(ctx, backend, cfg.Auth.TokenSecret, dataDir, log) | 709 | secret, err := loadSecret(ctx, backend, cfg.Auth.TokenSecret, dataDir, log) | ||
| @@ -1079,19 +1134,32 @@ func readDeployRequest() (string, error) { | |||||
| 1079 | } | 1134 | } | ||
| 1080 | 1135 | ||||
| 1081 | // createPidFile creates a PID file for the Gordon process. | 1136 | // createPidFile creates a PID file for the Gordon process. | ||
| 1137 | + | // SECURITY: Prefers secure locations (XDG_RUNTIME_DIR, ~/.gordon/run) over /tmp | |||
| 1138 | + | // to prevent symlink attacks and unauthorized access. | |||
| 1082 | func createPidFile(log zerowrap.Logger) string { | 1139 | func createPidFile(log zerowrap.Logger) string { | ||
| 1083 | pid := os.Getpid() | 1140 | pid := os.Getpid() | ||
| 1084 | 1141 | ||||
| 1085 | - | locations := []string{ | 1142 | + | // SECURITY: Prioritize secure locations over /tmp |
| 1086 | - | "/tmp/gordon.pid", | 1143 | + | var locations []string |
| 1087 | - | filepath.Join(os.TempDir(), "gordon.pid"), | 1144 | + | |
| 1145 | + | // Try secure runtime directory first | |||
| 1146 | + | if runtimeDir, err := getSecureRuntimeDir(); err == nil { | |||
| 1147 | + | locations = append(locations, filepath.Join(runtimeDir, "gordon.pid")) | |||
| 1088 | } | 1148 | } | ||
| 1089 | 1149 | ||||
| 1150 | + | // Fall back to home directory | |||
| 1090 | if homeDir, err := os.UserHomeDir(); err == nil { | 1151 | if homeDir, err := os.UserHomeDir(); err == nil { | ||
| 1091 | - | locations = append(locations, filepath.Join(homeDir, ".gordon.pid")) | 1152 | + | locations = append(locations, filepath.Join(homeDir, ".gordon", "gordon.pid")) |
| 1092 | } | 1153 | } | ||
| 1093 | 1154 | ||||
| 1155 | + | // Last resort: /tmp (least secure due to world-writable) | |||
| 1156 | + | locations = append(locations, filepath.Join(os.TempDir(), "gordon.pid")) | |||
| 1157 | + | ||||
| 1094 | for _, location := range locations { | 1158 | for _, location := range locations { | ||
| 1159 | + | // Ensure parent directory exists with secure permissions | |||
| 1160 | + | if err := os.MkdirAll(filepath.Dir(location), 0700); err != nil { | |||
| 1161 | + | continue | |||
| 1162 | + | } | |||
| 1095 | if err := os.WriteFile(location, []byte(fmt.Sprintf("%d", pid)), 0600); err == nil { | 1163 | if err := os.WriteFile(location, []byte(fmt.Sprintf("%d", pid)), 0600); err == nil { | ||
| 1096 | log.Debug().Str("pid_file", location).Int("pid", pid).Msg("created PID file") | 1164 | log.Debug().Str("pid_file", location).Int("pid", pid).Msg("created PID file") | ||
| 1097 | return location | 1165 | return location | ||
| @@ -1112,16 +1180,26 @@ func removePidFile(pidFile string, log zerowrap.Logger) { | |||||
| 1112 | } | 1180 | } | ||
| 1113 | 1181 | ||||
| 1114 | // findPidFile finds the Gordon PID file. | 1182 | // findPidFile finds the Gordon PID file. | ||
| 1183 | + | // Checks secure locations first, then falls back to legacy /tmp location. | |||
| 1115 | func findPidFile() string { | 1184 | func findPidFile() string { | ||
| 1116 | - | locations := []string{ | 1185 | + | var locations []string |
| 1117 | - | "/tmp/gordon.pid", | 1186 | + | |
| 1118 | - | filepath.Join(os.TempDir(), "gordon.pid"), | 1187 | + | // Check secure runtime directory first |
| 1188 | + | if runtimeDir, err := getSecureRuntimeDir(); err == nil { | |||
| 1189 | + | locations = append(locations, filepath.Join(runtimeDir, "gordon.pid")) | |||
| 1119 | } | 1190 | } | ||
| 1120 | 1191 | ||||
| 1192 | + | // Check home directory | |||
| 1121 | if homeDir, err := os.UserHomeDir(); err == nil { | 1193 | if homeDir, err := os.UserHomeDir(); err == nil { | ||
| 1194 | + | locations = append(locations, filepath.Join(homeDir, ".gordon", "gordon.pid")) | |||
| 1195 | + | // Legacy location for backward compatibility | |||
| 1122 | locations = append(locations, filepath.Join(homeDir, ".gordon.pid")) | 1196 | locations = append(locations, filepath.Join(homeDir, ".gordon.pid")) | ||
| 1123 | } | 1197 | } | ||
| 1124 | 1198 | ||||
| 1199 | + | // Legacy /tmp locations for backward compatibility | |||
| 1200 | + | locations = append(locations, filepath.Join(os.TempDir(), "gordon.pid")) | |||
| 1201 | + | locations = append(locations, "/tmp/gordon.pid") | |||
| 1202 | + | ||||
| 1125 | for _, location := range locations { | 1203 | for _, location := range locations { | ||
| 1126 | if _, err := os.Stat(location); err == nil { | 1204 | if _, err := os.Stat(location); err == nil { | ||
| 1127 | return location | 1205 | return location | ||
M internal/usecase/auth/service.go +2 -0
| @@ -292,6 +292,7 @@ func (s *Service) GenerateToken(ctx context.Context, subject string, scopes []st | |||
| 292 | 292 | "sub": subject, | |
| 293 | 293 | "iss": TokenIssuer, | |
| 294 | 294 | "iat": now.Unix(), | |
| 295 | + | "nbf": now.Unix(), // SECURITY: Not-before claim for clock skew protection | |
| 295 | 296 | "scopes": scopes, | |
| 296 | 297 | } | |
| 297 | 298 | ||
| @@ -351,6 +352,7 @@ func (s *Service) GenerateAccessToken(ctx context.Context, subject string, scope | |||
| 351 | 352 | "sub": subject, | |
| 352 | 353 | "iss": TokenIssuer, | |
| 353 | 354 | "iat": now.Unix(), | |
| 355 | + | "nbf": now.Unix(), // SECURITY: Not-before claim for clock skew protection | |
| 354 | 356 | "scopes": scopes, | |
| 355 | 357 | "exp": now.Add(expiry).Unix(), | |
| 356 | 358 | } | |
| @@ -292,6 +292,7 @@ func (s *Service) GenerateToken(ctx context.Context, subject string, scopes []st | |||||
| 292 | "sub": subject, | 292 | "sub": subject, | ||
| 293 | "iss": TokenIssuer, | 293 | "iss": TokenIssuer, | ||
| 294 | "iat": now.Unix(), | 294 | "iat": now.Unix(), | ||
| 295 | + | "nbf": now.Unix(), // SECURITY: Not-before claim for clock skew protection | |||
| 295 | "scopes": scopes, | 296 | "scopes": scopes, | ||
| 296 | } | 297 | } | ||
| 297 | 298 | ||||
| @@ -351,6 +352,7 @@ func (s *Service) GenerateAccessToken(ctx context.Context, subject string, scope | |||||
| 351 | "sub": subject, | 352 | "sub": subject, | ||
| 352 | "iss": TokenIssuer, | 353 | "iss": TokenIssuer, | ||
| 353 | "iat": now.Unix(), | 354 | "iat": now.Unix(), | ||
| 355 | + | "nbf": now.Unix(), // SECURITY: Not-before claim for clock skew protection | |||
| 354 | "scopes": scopes, | 356 | "scopes": scopes, | ||
| 355 | "exp": now.Add(expiry).Unix(), | 357 | "exp": now.Add(expiry).Unix(), | ||
| 356 | } | 358 | } | ||
M internal/usecase/auth/service_test.go +61 -0
| @@ -755,3 +755,64 @@ func TestService_GeneratePasswordHash_DifferentHashesForSamePassword(t *testing. | |||
| 755 | 755 | assert.NoError(t, bcrypt.CompareHashAndPassword([]byte(hash1), []byte(password))) | |
| 756 | 756 | assert.NoError(t, bcrypt.CompareHashAndPassword([]byte(hash2), []byte(password))) | |
| 757 | 757 | } | |
| 758 | + | ||
| 759 | + | func TestService_GenerateToken_HasNbfClaim(t *testing.T) { | |
| 760 | + | // Test that generated tokens include the nbf (not-before) claim | |
| 761 | + | // This is a security requirement per SEC-MED-006 | |
| 762 | + | tokenStore := mocks.NewMockTokenStore(t) | |
| 763 | + | ||
| 764 | + | tokenStore.EXPECT(). | |
| 765 | + | SaveToken(mock.Anything, mock.Anything, mock.Anything). | |
| 766 | + | Return(nil) | |
| 767 | + | ||
| 768 | + | svc := NewService(Config{ | |
| 769 | + | Enabled: true, | |
| 770 | + | AuthType: domain.AuthTypeToken, | |
| 771 | + | TokenSecret: []byte("test-secret-key-for-jwt-signing"), | |
| 772 | + | }, tokenStore, zerowrap.Default()) | |
| 773 | + | ||
| 774 | + | ctx := testContext() | |
| 775 | + | tokenStr, err := svc.GenerateToken(ctx, "testsubject", []string{"push"}, time.Hour) | |
| 776 | + | require.NoError(t, err) | |
| 777 | + | ||
| 778 | + | // Parse the token to verify nbf claim | |
| 779 | + | claims, err := svc.parseTokenClaims(tokenStr) | |
| 780 | + | require.NoError(t, err) | |
| 781 | + | ||
| 782 | + | // Verify nbf claim exists and is valid | |
| 783 | + | nbf, ok := claims["nbf"].(float64) | |
| 784 | + | require.True(t, ok, "nbf claim should exist and be a number") | |
| 785 | + | ||
| 786 | + | iat, ok := claims["iat"].(float64) | |
| 787 | + | require.True(t, ok, "iat claim should exist") | |
| 788 | + | ||
| 789 | + | // nbf should equal iat for immediate validity | |
| 790 | + | assert.Equal(t, iat, nbf, "nbf should equal iat") | |
| 791 | + | assert.InDelta(t, time.Now().Unix(), int64(nbf), 5, "nbf should be current time") | |
| 792 | + | } | |
| 793 | + | ||
| 794 | + | func TestService_GenerateAccessToken_HasNbfClaim(t *testing.T) { | |
| 795 | + | // Test that access tokens also include the nbf claim | |
| 796 | + | svc := NewService(Config{ | |
| 797 | + | Enabled: true, | |
| 798 | + | AuthType: domain.AuthTypeToken, | |
| 799 | + | TokenSecret: []byte("test-secret-key-for-jwt-signing"), | |
| 800 | + | }, nil, zerowrap.Default()) | |
| 801 | + | ||
| 802 | + | ctx := testContext() | |
| 803 | + | tokenStr, err := svc.GenerateAccessToken(ctx, "testsubject", []string{"pull"}, time.Minute) | |
| 804 | + | require.NoError(t, err) | |
| 805 | + | ||
| 806 | + | // Parse the token to verify nbf claim | |
| 807 | + | claims, err := svc.parseTokenClaims(tokenStr) | |
| 808 | + | require.NoError(t, err) | |
| 809 | + | ||
| 810 | + | // Verify nbf claim exists | |
| 811 | + | nbf, ok := claims["nbf"].(float64) | |
| 812 | + | require.True(t, ok, "nbf claim should exist and be a number") | |
| 813 | + | ||
| 814 | + | iat, ok := claims["iat"].(float64) | |
| 815 | + | require.True(t, ok, "iat claim should exist") | |
| 816 | + | ||
| 817 | + | assert.Equal(t, iat, nbf, "nbf should equal iat for access tokens") | |
| 818 | + | } | |
| @@ -755,3 +755,64 @@ func TestService_GeneratePasswordHash_DifferentHashesForSamePassword(t *testing. | |||||
| 755 | assert.NoError(t, bcrypt.CompareHashAndPassword([]byte(hash1), []byte(password))) | 755 | assert.NoError(t, bcrypt.CompareHashAndPassword([]byte(hash1), []byte(password))) | ||
| 756 | assert.NoError(t, bcrypt.CompareHashAndPassword([]byte(hash2), []byte(password))) | 756 | assert.NoError(t, bcrypt.CompareHashAndPassword([]byte(hash2), []byte(password))) | ||
| 757 | } | 757 | } | ||
| 758 | + | ||||
| 759 | + | func TestService_GenerateToken_HasNbfClaim(t *testing.T) { | |||
| 760 | + | // Test that generated tokens include the nbf (not-before) claim | |||
| 761 | + | // This is a security requirement per SEC-MED-006 | |||
| 762 | + | tokenStore := mocks.NewMockTokenStore(t) | |||
| 763 | + | ||||
| 764 | + | tokenStore.EXPECT(). | |||
| 765 | + | SaveToken(mock.Anything, mock.Anything, mock.Anything). | |||
| 766 | + | Return(nil) | |||
| 767 | + | ||||
| 768 | + | svc := NewService(Config{ | |||
| 769 | + | Enabled: true, | |||
| 770 | + | AuthType: domain.AuthTypeToken, | |||
| 771 | + | TokenSecret: []byte("test-secret-key-for-jwt-signing"), | |||
| 772 | + | }, tokenStore, zerowrap.Default()) | |||
| 773 | + | ||||
| 774 | + | ctx := testContext() | |||
| 775 | + | tokenStr, err := svc.GenerateToken(ctx, "testsubject", []string{"push"}, time.Hour) | |||
| 776 | + | require.NoError(t, err) | |||
| 777 | + | ||||
| 778 | + | // Parse the token to verify nbf claim | |||
| 779 | + | claims, err := svc.parseTokenClaims(tokenStr) | |||
| 780 | + | require.NoError(t, err) | |||
| 781 | + | ||||
| 782 | + | // Verify nbf claim exists and is valid | |||
| 783 | + | nbf, ok := claims["nbf"].(float64) | |||
| 784 | + | require.True(t, ok, "nbf claim should exist and be a number") | |||
| 785 | + | ||||
| 786 | + | iat, ok := claims["iat"].(float64) | |||
| 787 | + | require.True(t, ok, "iat claim should exist") | |||
| 788 | + | ||||
| 789 | + | // nbf should equal iat for immediate validity | |||
| 790 | + | assert.Equal(t, iat, nbf, "nbf should equal iat") | |||
| 791 | + | assert.InDelta(t, time.Now().Unix(), int64(nbf), 5, "nbf should be current time") | |||
| 792 | + | } | |||
| 793 | + | ||||
| 794 | + | func TestService_GenerateAccessToken_HasNbfClaim(t *testing.T) { | |||
| 795 | + | // Test that access tokens also include the nbf claim | |||
| 796 | + | svc := NewService(Config{ | |||
| 797 | + | Enabled: true, | |||
| 798 | + | AuthType: domain.AuthTypeToken, | |||
| 799 | + | TokenSecret: []byte("test-secret-key-for-jwt-signing"), | |||
| 800 | + | }, nil, zerowrap.Default()) | |||
| 801 | + | ||||
| 802 | + | ctx := testContext() | |||
| 803 | + | tokenStr, err := svc.GenerateAccessToken(ctx, "testsubject", []string{"pull"}, time.Minute) | |||
| 804 | + | require.NoError(t, err) | |||
| 805 | + | ||||
| 806 | + | // Parse the token to verify nbf claim | |||
| 807 | + | claims, err := svc.parseTokenClaims(tokenStr) | |||
| 808 | + | require.NoError(t, err) | |||
| 809 | + | ||||
| 810 | + | // Verify nbf claim exists | |||
| 811 | + | nbf, ok := claims["nbf"].(float64) | |||
| 812 | + | require.True(t, ok, "nbf claim should exist and be a number") | |||
| 813 | + | ||||
| 814 | + | iat, ok := claims["iat"].(float64) | |||
| 815 | + | require.True(t, ok, "iat claim should exist") | |||
| 816 | + | ||||
| 817 | + | assert.Equal(t, iat, nbf, "nbf should equal iat for access tokens") | |||
| 818 | + | } | |||