Add unit tests for ExtractRoleNameFromArn and ExtractRoleNameFromPrincipal
to verify support for both AWS standard and legacy IAM role ARN formats:
TestExtractRoleNameFromArn (14 test cases):
- Legacy format without account ID: arn:aws:iam::role/RoleName
- Standard format with account ID: arn:aws:iam::ACCOUNT:role/RoleName
- With role paths: arn:aws:iam::role/Path/RoleName
- Invalid ARNs and edge cases
TestExtractRoleNameFromPrincipal (11 test cases):
- STS assumed role format (legacy and standard)
- IAM role format (legacy and standard)
- With and without account ID
- Invalid principals and edge cases
All tests pass with 100% coverage for both functions.
Fix issue #7946 where SeaweedFS only recognized legacy IAM role ARN format
(arn:aws:iam::role/RoleName) but not the standard AWS format with account ID
(arn:aws:iam::ACCOUNT:role/RoleName). This was breaking EKS pod identity
integration which expects the standard format.
Changes:
- Update ExtractRoleNameFromArn() to handle both formats by searching for
'role/' marker instead of matching a fixed prefix
- Update ExtractRoleNameFromPrincipal() to clearly document both STS and IAM
formats it supports
- Simplify role ARN validation in validateRoleAssumptionForWebIdentity() and
validateRoleAssumptionForCredentials() to use the extraction function
The fix maintains backward compatibility with legacy format while adding
support for standard AWS format with account ID.
Fixes: https://github.com/seaweedfs/seaweedfs/issues/7946
- Define iamAuthPath as a named string type (similar to existing authType enum)
- Update constants to use explicit type: iamAuthPathJWT, iamAuthPathSTS_V4, etc.
- Update determineIAMAuthPath() to return typed iamAuthPath
- Improves type safety and prevents accidental string value misuse
- Rename authorization path constants to avoid conflict with existing authType enum
- Replace nested if/else with clean switch statement in authorizeWithIAM()
- Add determineIAMAuthPath() helper for clearer intent and testability
- Optimize key counting in auth_signature_v4.go: remove unnecessary slice allocation
- Fix timing assertion in session_claims_test.go: use WithinDuration for symmetric tolerance
These changes improve code readability, maintainability, and performance while
maintaining full backward compatibility and test coverage.
Added new test file auth_sts_v4_test.go with comprehensive tests for the
STS session token authorization fix:
1. TestAuthorizeWithIAMSessionTokenExtraction:
- Verifies X-SeaweedFS-Session-Token is extracted from JWT auth headers
- Verifies X-Amz-Security-Token is extracted from V4 STS auth headers
- Verifies X-Amz-Security-Token is extracted from query parameters (presigned URLs)
- Verifies JWT tokens take precedence when both are present
- Regression test for the bug where V4 STS tokens were not being passed to authorization
2. TestSTSSessionTokenIntoCredentials:
- Verifies STS credentials have all required fields (AccessKeyId, SecretAccessKey, SessionToken)
- Verifies deterministic generation from sessionId (same sessionId = same credentials)
- Verifies different sessionIds produce different credentials
- Critical for signature verification: same session must regenerate same secret key
3. TestActionConstantsForV4Auth:
- Verifies S3 action constants are available for authorization checks
- Ensures ACTION_READ, ACTION_WRITE, etc. are properly defined
These tests ensure that:
- V4 Signature auth with STS tokens properly extracts and uses session tokens
- Session tokens are prioritized correctly (JWT > X-Amz-Security-Token header > query param)
- STS credentials are deterministically generated for signature verification
- The fix for passing STS session tokens to authorization is properly covered
All 3 test functions pass (6 test cases total).
CRITICAL FIX: Session tokens were not being passed to the authorization
check when using AWS Signature V4 authentication with STS credentials.
The bug:
1. AWS SDK sends request with X-Amz-Security-Token header (V4 signature)
2. validateSTSSessionToken validates the token, creates Identity with PrincipalArn
3. authorizeWithIAM only checked X-SeaweedFS-Session-Token (JWT auth header)
4. Since it was empty, fell into 'static V4' branch which set SessionToken = ''
5. AuthorizeAction returned ErrAccessDenied because SessionToken was empty
The fix (in authorizeWithIAM):
- Check X-SeaweedFS-Session-Token first (JWT auth)
- If empty, fallback to X-Amz-Security-Token header (V4 STS auth)
- If still empty, check X-Amz-Security-Token query param (presigned URLs)
- When session token is found with PrincipalArn, use 'STS V4 signature' path
- Only use 'static V4' path when there's no session token
This ensures:
- JWT Bearer auth with session tokens works (existing path)
- STS V4 signature auth with session tokens works (new path)
- Static V4 signature auth without session tokens works (existing path)
Logging updated to distinguish:
- 'JWT-based IAM authorization'
- 'STS V4 signature IAM authorization' (new)
- 'static V4 signature IAM authorization' (clarified)
The session_helpers.go file contained two unused IsExpired() methods:
- Credentials.IsExpired()
- SessionInfo.IsExpired()
These were never called anywhere in the codebase. The actual expiration
checks use:
- isCredentialExpired() in weed/s3api/auth_credentials.go (S3 auth)
- Direct time.Now().After() checks
Removing unused code improves code clarity and reduces maintenance burden.
Improved defensive programming in IsExpired() methods:
1. Credentials.IsExpired():
- Added explicit check for zero-time expiration (time.Time{})
- Treats uninitialized credentials as expired
- Prevents accidentally treating uninitialized creds as valid
2. SessionInfo.IsExpired():
- Added same explicit zero-time check
- Treats uninitialized sessions as expired
- Protects against bugs where sessions might not be properly initialized
This is important because time.Now().After(time.Time{}) returns true,
but explicitly checking for zero time makes the intent clear and helps
catch initialization bugs during code review and debugging.
Updated tests to verify that secret access keys are now deterministic:
1. Updated TestSTSSessionClaimsToSessionInfoCredentialGeneration:
- Changed comment from 'NOT deterministic' to 'NOW deterministic'
- Added assertion that same sessionId produces identical secret key
- Explains why this is critical for signature verification
2. Added TestSecretAccessKeyDeterminism (new dedicated test):
- Verifies secret key is identical across multiple calls with same sessionId
- Verifies access key ID and session token are also identical
- Verifies different sessionIds produce different credentials
- Includes detailed comments explaining why determinism is critical
These tests ensure that the STS implementation correctly regenerates
deterministic credentials during signature verification. Without
determinism, signature verification would always fail because the
server would use different secret keys than the client used to sign.
CRITICAL FIX: The secret access key was being randomly generated, causing
signature verification failures when the same session token was used twice:
1. AssumeRoleWithWebIdentity generates random secret key X
2. Client signs request using secret key X
3. Server validates token, regenerates credentials via ToSessionInfo()
4. ToSessionInfo() calls generateSecretAccessKey(), which generates random key Y
5. Server tries to verify signature using key Y, but signature was made with X
6. Signature verification fails (SignatureDoesNotMatch)
Solution: Make generateSecretAccessKey() deterministic by using SHA256 hash
of 'secret-key:' + sessionId, just like generateAccessKeyId() already does.
This ensures:
- AssumeRoleWithWebIdentity generates deterministic secret key from sessionId
- ToSessionInfo() regenerates the same secret key from the same sessionId
- Client signature verification succeeds because keys match
Fixes: AWS SDK v2 CORS tests failing with 'ExpiredToken' errors
Affected files:
- weed/iam/sts/token_utils.go: Updated generateSecretAccessKey() signature
and implementation to be deterministic
- Updated GenerateTemporaryCredentials() to pass sessionId parameter
Tests: All 54 STS tests pass with this fix
Critical fix: The previous cleanup of sensitive logging inadvertently removed
the error return statement when access key lookup fails. This caused the code
to continue and call isCredentialExpired() on nil pointer, crashing the server.
This explains EOF errors in CORS tests - server was panicking on requests
with invalid keys.
Remove session_helpers.go and update TestSTSSessionClaimsToSessionInfoCredentialExpiration to assert against sessionInfo.Credentials.Expiration directly as requested by reviewer.,
- Add Credentials.IsExpired() and SessionInfo.IsExpired() in new file session_helpers.go.
- Update TestSTSSessionClaimsToSessionInfoCredentialExpiration to use helpers for clearer intent.
Replace wallclock assertions comparing tc.expiresAt to time.Now() (which only verified test setup)
with assertions that check sessionInfo.Credentials.Expiration relative to time.Now(), thus
exercising the code under test. Include clarifying comment for intent.
Update TestSTSSessionClaimsToSessionInfoCredentialGeneration to properly verify
deterministic credential generation:
- Remove misleading comment about 'randomness' - parts of credentials ARE deterministic
- Add assertions that AccessKeyId is identical for same SessionId (hash-based, deterministic)
- Add assertions that SessionToken is identical for same SessionId (hash-based, deterministic)
- Verify Expiration matches when SessionId is identical
- Document that SecretAccessKey is NOT deterministic (uses random.Read)
- Truncate expiresAt to second precision to avoid timing issues
This test now properly verifies that the deterministic components of credential
generation work correctly while acknowledging the cryptographic randomness of
the secret access key.
Two improvements to error handling and security:
1. weed/iam/sts/session_claims.go:
- Add logging for credential generation failures in ToSessionInfo()
- Wrap errors with context (session ID) to aid debugging
- Use glog.Warningf() to surface errors instead of silently swallowing them
- Add fmt import for error wrapping
2. weed/s3api/auth_signature_v4.go:
- Remove debug logging of actual access key IDs (glog.V(2) call)
- Security improvement: avoid exposing sensitive access keys even at debug level
- Keep warning-level logging that shows only count of available keys
This ensures credential generation failures are observable while protecting
sensitive authentication material from logs.
Optimize ToSessionInfo() to reuse a package-level defaultCredentialGenerator
instead of allocating a new CredentialGenerator on every call. This reduces
allocation overhead since this method is called frequently during signature
verification (potentially once per request).
The CredentialGenerator is stateless and deterministic, making it safe to
reuse across concurrent calls without synchronization.
- Check for X-Amz-Security-Token header in verifyV4Signature
- Call validateSTSSessionToken for STS requests
- Skip regular access key lookup and expiration check for STS sessions
* fix: directory incorrectly listed as object in S3 ListObjects
Regular directories (without MIME type) were only added to CommonPrefixes
when delimiter was exactly '/'. This caused directories to be silently
skipped for other delimiter values.
Changed the condition from 'delimiter == "/"' to 'delimiter != ""' to
ensure directories are correctly added to CommonPrefixes for any delimiter.
Fixes issue where directories like 'data/file.vhd' were being returned as
objects instead of prefixes in ListObjects responses.
* fix: complete the directory listing fix for all delimiters
Address reviewer feedback:
- Changed doListFilerEntries line 549 from 'delimiter != "/"' to 'delimiter == ""'
This ensures directories are yielded to the callback for ANY delimiter, not just "/"
- Parameterized test to verify fix works with multiple delimiters (/, _, :)
The previous fix only addressed line 260 but line 549 was still causing
recursion for non-"/" delimiters, preventing directories from being
added to CommonPrefixes.
* docs: update test comment to reflect multiple delimiters
Address reviewer feedback - clarify that the test verifies behavior
for any non-empty delimiter, not just '/'.
* docs: clarify test comment with delimiter examples
Add specific examples of delimiters ('/', '_', ':') to make it clear
that the test verifies behavior with multiple delimiter types.
* fix: revert line 549 to original logic, only line 260 needed changing
The fix for directories being listed as objects only required changing
line 260 from 'delimiter == "/"' to 'delimiter != ""'.
Line 549 should remain as 'delimiter != "/"' to allow recursion for
delimiters that don't exist in paths (e.g., delimiter=z for paths like
b/a/c). This is correct S3 behavior.
Updated test to only verify delimiter="/" since other delimiters should
recurse into directories to find actual files.
* docs: clarify test scope in directory listing test
* optimize: enable immediate EC shard reporting during startup
Ported the immediate EC shard reporting feature from Enterprise to Community version.
This allows the master to be notified about EC shards immediately during volume server startup,
instead of waiting for the first heartbeat.
Changes:
1. Updated NewStore to initialize notification channels BEFORE loading volumes (fixes potential nil panic).
2. Added ecShardNotifyHandler to report EC shards to NewEcShardsChan during startup.
3. Implemented non-blocking channel send for EC reporting to prevent deadlock when loading many EC shards (fixing the enterprise bug 17ac1290c).
4. Updated DiskLocation and EC loading logic to support the callback.
This optimization improves cluster state consistency and startup speed for EC-heavy clusters.
* optimize: report actual EC shard size during startup
* optimize: increase notification channel buffer size to 1024
* optimize: fix variable shadowing in store.go
* feat(iam): add TLS configuration support for OIDC provider
Adds tlsCaCert and tlsInsecureSkipVerify options to OIDC provider configuration to allow using custom CA certificates and skipping verification in development environments.
* fix: use SystemCertPool for custom CA and add security warning
- Use x509.SystemCertPool() to preserve trust in public CAs
- Add warning log when TLSInsecureSkipVerify is enabled
- Addresses code review feedback from gemini-code-assist
* docs: enhance TLS configuration field documentation
- Add explicit warning about TLSInsecureSkipVerify production usage
- Clarify TLSCACert is for custom/self-signed certificates
* security: enforce TLS 1.2 minimum version
- Set MinVersion to TLS 1.2 to prevent downgrade attacks
- Ensures secure communication with OIDC providers
* security: validate CA cert path is absolute
- Add filepath.IsAbs check before reading CA certificate
- Prevents reading unintended files from relative paths
- Fail fast on misconfigured paths
* Fix: Add -admin.grpc flag to worker for explicit gRPC port configuration
* Fix(helm): Add adminGrpcServer to worker configuration
* Refactor: Support host:port.grpcPort address format, revert -admin.grpc flag
* Helm: Conditionally append grpcPort to worker admin address
* weed/admin: fix "send on closed channel" panic in worker gRPC server
Make unregisterWorker connection-aware to prevent closing channels
belonging to newer connections.
* weed/worker: improve gRPC client stability and logging
- Fix goroutine leak in reconnection logic
- Refactor reconnection loop to exit on success and prevent busy-waiting
- Add session identification and enhanced logging to client handlers
- Use constant for internal reset action and remove unused variables
* weed/worker: fix worker state initialization and add lifecycle logs
- Revert workerState to use running boolean correctly
- Prevent handleStart failing by checking running state instead of startTime
- Add more detailed logs for worker startup events
This adds support for the new FUSE performance options to the 'weed fuse' command,
matching the functionality available in 'weed mount'.
Added options:
- writebackCache: Enable FUSE writeback cache for improved write performance
- asyncDio: Enable async direct I/O for better concurrency
- cacheSymlink: Enable symlink caching to reduce metadata lookups
- sys.novncache: (macOS only) Disable vnode name caching to avoid stale data
These options can now be used with mount -t weed:
mount -t weed fuse /mnt -o "filer=localhost:8888,writebackCache=true,asyncDio=true"
This ensures feature parity between 'weed mount' and 'weed fuse' commands.
* mount: add -asyncDio flag for async direct I/O
This adds support for async direct I/O via the -asyncDio flag.
Async DIO enables the FUSE_CAP_ASYNC_DIO capability, allowing the kernel
to perform direct I/O operations asynchronously. This improves concurrency
for applications that use O_DIRECT flag.
Benefits:
- Better concurrency for direct I/O operations
- Improved performance for applications using O_DIRECT
- Reduced blocking on I/O operations
Use cases:
- Database workloads that use direct I/O
- Applications that bypass page cache intentionally
- High-performance I/O scenarios
Implementation inspired by JuiceFS which enables this capability
for improved I/O performance.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -asyncDio
* mount: add all remaining FUSE options (asyncDio, cacheSymlink, novncache)
This combines the remaining three FUSE mount options on top of the merged writebackCache PR:
1. asyncDio: Enable async direct I/O for better concurrency
2. cacheSymlink: Enable symlink caching to reduce metadata lookups
3. novncache: (macOS only) Disable vnode name caching to avoid stale data
All options use the function parameter 'option' instead of global 'mountOptions'.
* mount: add -asyncDio flag for async direct I/O
This adds support for async direct I/O via the -asyncDio flag.
Async DIO enables the FUSE_CAP_ASYNC_DIO capability, allowing the kernel
to perform direct I/O operations asynchronously. This improves concurrency
for applications that use O_DIRECT flag.
Benefits:
- Better concurrency for direct I/O operations
- Improved performance for applications using O_DIRECT
- Reduced blocking on I/O operations
Use cases:
- Database workloads that use direct I/O
- Applications that bypass page cache intentionally
- High-performance I/O scenarios
Implementation inspired by JuiceFS which enables this capability
for improved I/O performance.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -asyncDio
* mount: add all remaining FUSE options (asyncDio, cacheSymlink, novncache)
This combines the remaining three FUSE mount options on top of the merged writebackCache PR:
1. asyncDio: Enable async direct I/O for better concurrency
2. cacheSymlink: Enable symlink caching to reduce metadata lookups
3. novncache: (macOS only) Disable vnode name caching to avoid stale data
All options use the function parameter 'option' instead of global 'mountOptions'.
* mount: add -asyncDio flag for async direct I/O
This adds support for async direct I/O via the -asyncDio flag.
Async DIO enables the FUSE_CAP_ASYNC_DIO capability, allowing the kernel
to perform direct I/O operations asynchronously. This improves concurrency
for applications that use O_DIRECT flag.
Benefits:
- Better concurrency for direct I/O operations
- Improved performance for applications using O_DIRECT
- Reduced blocking on I/O operations
Use cases:
- Database workloads that use direct I/O
- Applications that bypass page cache intentionally
- High-performance I/O scenarios
Implementation inspired by JuiceFS which enables this capability
for improved I/O performance.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -asyncDio
* mount: add all remaining FUSE options (asyncDio, cacheSymlink, novncache)
This combines the remaining three FUSE mount options on top of the merged writebackCache PR:
1. asyncDio: Enable async direct I/O for better concurrency
2. cacheSymlink: Enable symlink caching to reduce metadata lookups
3. novncache: (macOS only) Disable vnode name caching to avoid stale data
All options use the function parameter 'option' instead of global 'mountOptions'.
2025-12-31 00:30:12 -08:00
Chris LuGitHubgemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* mount: add -writebackCache flag for FUSE writeback caching
This adds support for FUSE writeback caching via the -writebackCache flag.
Writeback caching buffers writes in the kernel page cache before flushing
to the filesystem. This significantly improves performance for workloads
with many small writes by reducing the number of write syscalls.
Benefits:
- Improved write performance for small files (2-5x faster)
- Reduced latency for write-heavy workloads
- Better handling of bursty write patterns
Trade-offs:
- Data may be lost if system crashes before kernel flushes
- Not recommended for critical data without proper fsync usage
- Disabled by default for safety
Inspired by JuiceFS implementation which uses the same FUSE option.
Usage:
weed mount -filer=localhost:8888 -dir=/mnt/seaweedfs -writebackCache
* Apply suggestion from @gemini-code-assist[bot]
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
When writing metadata logs to /topics/.system/log, the filer was not
respecting the disk type configuration from path-specific rules
(fs.configure). This caused volume assignment failures when volume
servers used a specific disk type (e.g., "ssd") because the assign
request defaulted to empty disk type.
The fix adds DiskType to the VolumeAssignRequest in the filer's
metadata log write path, ensuring that path-specific disk type
configurations are properly honored for internal system writes.
Fixes errors like:
"metadata log write failed /topics/.system/log/...: AssignVolume:
failed to find writable volumes for collection"
Signed-off-by: Charles Darke <s.cduk@toodevious.com>
Co-authored-by: Charles Darke <s.cduk@toodevious.com>
Chris Lu
Chris LuandGitHub
Sheya BernsteinandGitHub
