mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-05-14 05:41:29 +00:00
c6302fcb54
* feat(iam): support caller-supplied AccessKeyId and SecretAccessKey in CreateAccessKey Both IAM implementations (standalone and embedded) now check for caller-supplied AccessKeyId and SecretAccessKey form parameters before generating random credentials. If provided, the caller-supplied values are used. If empty, random keys are generated as before. This enables programmatic identity provisioning where the caller needs to control the S3 credentials. Backward-compatible: no behavior change for callers that omit these parameters. * refactor(iam): extract shared caller-supplied credential validation Move the AccessKeyId/SecretAccessKey format checks and the in-memory collision scan into weed/iam so the standalone IAM API, the embedded IAM in s3api, and the admin dashboard all enforce the same rules. - ValidateCallerSuppliedAccessKeyId: 4-128 alphanumeric (rejects SigV4-breaking characters like '/' and '='). - ValidateCallerSuppliedSecretAccessKey: 8-128 chars. - FindAccessKeyOwner: scans identities and service accounts and returns the owning entity type + name for debug logging, without exposing the owner in caller-facing error messages. The admin dashboard previously only length-checked caller-supplied keys; it now enforces the same alphanumeric rule, which matches what SigV4 actually accepts anyway. * fix(iam): reject partial caller-supplied AccessKeyId/SecretAccessKey Previously, if a caller supplied only one of AccessKeyId or SecretAccessKey, CreateAccessKey logged a warning and auto-generated the missing half. That silently returns a credential the caller did not fully choose, which is surprising and easy to miss in a response they expected to echo back their input. Return ErrCodeInvalidInputException instead: either both are supplied or neither is. Updates the mixed-supply tests in weed/iamapi and weed/s3api to assert the rejection. * chore(iam): centralize and broaden sensitive form redaction DoActions and ExecuteAction both had an inline loop that redacted SecretAccessKey from their debug-level request log. Replace the two copies with iam.RedactSensitiveFormValues, backed by an explicit sensitive-keys set. The set now also covers Password, OldPassword, NewPassword, PrivateKey, and SessionToken. None of those parameters are used by today's IAM actions, but naming them here makes the log-safety guarantee survive future additions such as LoginProfile / STS. * test(iam): cover the upper length bound for CreateAccessKey TestCreateAccessKeyBoundary / TestEmbeddedIamCreateAccessKeyBoundary only exercised the 3/4-char lower edge. Add cases for 128 (accepted) and 129 (rejected) for AccessKeyId, plus 7 / 128 / 129-char cases for SecretAccessKey, so both ends of the validator are locked in at the handler level (the pure validators in weed/iam already cover this). * fix(s3api/iam): verify user existence before RNG and collision scan In the embedded IAM CreateAccessKey, the user lookup ran last: a request for a non-existent user still walked the whole identity / service-account list for collisions and, if no caller-supplied keys were present, generated fresh random credentials with crypto/rand before the NoSuchEntity error finally surfaced. Reorder: validate inputs, then find the target identity, then do the collision scan, then generate keys. A missing user now fails fast and consumes no entropy, and the handler returns NoSuchEntity instead of a misleading EntityAlreadyExists when both the user is missing and the supplied AccessKeyId happens to collide with another identity's key. Add TestEmbeddedIamCreateAccessKeyRejectsMissingUser to lock in the "no mutation on unknown user" guarantee. The standalone iamapi CreateAccessKey intentionally keeps its pre-existing "create-or-attach" semantics where a missing user is implicitly provisioned — that is a behavior change beyond the scope of this PR. * test(iam): tighten collision leak assertion and cover 8-char secret - Rename the collision-owner identity in TestCreateAccessKeyRejectsCollision (both iamapi and the embedded s3api test) from "existing" / "ExistingUser" to "ownerAlpha". The old assert.NotContains check was effectively a no-op because the error message never contained those substrings; a distinctive name shared with no part of the expected error body makes the leak guard actually meaningful if the wording ever drifts. The embedded test also adds a NotContains assertion that was previously missing entirely. - Add an explicit 8-char SecretAccessKey pass case to both boundary tests so the lower edge of the validator is locked in at the handler level alongside the 7 / 128 / 129-char cases. * fix(iamapi): enforce both-or-none before the collision lookup In the standalone IAM CreateAccessKey, FindAccessKeyOwner ran before the partial-credential check. If a caller supplied only AccessKeyId and it happened to collide with an existing key, the response was EntityAlreadyExists instead of the more fundamental InvalidInput for omitting SecretAccessKey — wrong error class, and leaked the fact that the probed key is already in use. Swap the order: validate both-or-none first, then do the collision scan. Matches the embedded IAM path and AWS behavior. Add a case to TestCreateAccessKeyRejectsPartialSupply that combines partial supply with a collision to lock in the ordering. * fix(admin): reject partial caller-supplied AccessKey/SecretKey The admin dashboard path silently generated the missing half when a caller supplied only one of AccessKey or SecretKey, while the IAM API and embedded IAM paths now reject this. Align the three: if exactly one is provided, return ErrInvalidInput. Also simplifies the generator block — either both are provided or neither is, so there is no mixed path to handle. * test(s3api/iam): guard dereferences in caller-supplied-keys test TestEmbeddedIamCreateAccessKeyWithCallerSuppliedKeys dereferenced *AccessKeyId/*SecretAccessKey/*UserName and indexed Identities[0].Credentials[0] without first verifying shape, so any future regression that returns a partial response or skips the config mutation would panic mid-assertion instead of failing with a clear message. Add require.NotNil on the response pointers and require.Len on the identities/credentials slices before the asserts. * test(iamapi): exercise the service-account branch of the collision check FindAccessKeyOwner scans both Identities[*].Credentials and ServiceAccounts[*].Credential, but TestCreateAccessKeyRejectsCollision only covered the identity branch. Split the test into two subtests — one per branch — so a future refactor that drops the service-account scan (or mutates the existing credential) trips a failure. Also asserts the existing service-account credential is not mutated and no credential is attached to the target identity on rejection. * test(iam): isolate 129-char secret subcase from prior credential In both TestCreateAccessKeyBoundary (iamapi) and TestEmbeddedIamCreateAccessKeyBoundary (s3api), the 129-char SecretAccessKey subcase reused the "validkey" AccessKeyId that the preceding 8-char subcase had just persisted into the config. The test still asserted the right outcome because the handler validates secret length before running the collision scan — but if the two checks ever swap, the subcase would pass (or fail) for the wrong reason. Reset the in-memory credentials before the 129-char subcase, matching the pattern already used by the 3/128/129-char AccessKeyId and 7-char secret subcases. No behavior change; purely test isolation. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>
69 lines
2.4 KiB
Go
69 lines
2.4 KiB
Go
package iam
|
|
|
|
import (
|
|
"fmt"
|
|
|
|
"github.com/seaweedfs/seaweedfs/weed/pb/iam_pb"
|
|
)
|
|
|
|
// ValidateCallerSuppliedAccessKeyId checks that a caller-supplied AccessKeyId
|
|
// is 4 to 128 ASCII alphanumeric characters. Returns nil if valid.
|
|
//
|
|
// The alphanumeric restriction avoids characters that would break SigV4
|
|
// canonicalization (e.g. '/' and '=' appear as delimiters in Credential
|
|
// headers), so this is a stricter superset of the rule AWS enforces.
|
|
func ValidateCallerSuppliedAccessKeyId(accessKeyId string) error {
|
|
if len(accessKeyId) < 4 || len(accessKeyId) > 128 {
|
|
return fmt.Errorf("AccessKeyId must be 4 to 128 alphanumeric characters")
|
|
}
|
|
for _, r := range accessKeyId {
|
|
if !((r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9')) {
|
|
return fmt.Errorf("AccessKeyId must be 4 to 128 alphanumeric characters")
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// ValidateCallerSuppliedSecretAccessKey checks that a caller-supplied
|
|
// SecretAccessKey is 8 to 128 characters. Returns nil if valid.
|
|
func ValidateCallerSuppliedSecretAccessKey(secretAccessKey string) error {
|
|
if len(secretAccessKey) < 8 || len(secretAccessKey) > 128 {
|
|
return fmt.Errorf("SecretAccessKey must be between 8 and 128 characters")
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// AccessKeyOwner identifies which entity in an S3ApiConfiguration already owns
|
|
// a given AccessKeyId. Returned by FindAccessKeyOwner for collision checks on
|
|
// caller-supplied credentials.
|
|
type AccessKeyOwner struct {
|
|
// Type is "user" or "service account".
|
|
Type string
|
|
// Name is the identity's Name (for users) or the service account's Id.
|
|
Name string
|
|
}
|
|
|
|
// FindAccessKeyOwner scans s3cfg for an identity or service account whose
|
|
// credentials already contain accessKeyId. Returns nil if the key is free.
|
|
//
|
|
// Callers should log Name only at debug level — error responses returned to
|
|
// the caller should not include owner identity to avoid information leaks.
|
|
func FindAccessKeyOwner(s3cfg *iam_pb.S3ApiConfiguration, accessKeyId string) *AccessKeyOwner {
|
|
if s3cfg == nil || accessKeyId == "" {
|
|
return nil
|
|
}
|
|
for _, ident := range s3cfg.Identities {
|
|
for _, cred := range ident.Credentials {
|
|
if cred.AccessKey == accessKeyId {
|
|
return &AccessKeyOwner{Type: "user", Name: ident.Name}
|
|
}
|
|
}
|
|
}
|
|
for _, sa := range s3cfg.ServiceAccounts {
|
|
if sa.Credential != nil && sa.Credential.AccessKey == accessKeyId {
|
|
return &AccessKeyOwner{Type: "service account", Name: sa.Id}
|
|
}
|
|
}
|
|
return nil
|
|
}
|