mirror of
https://github.com/versity/versitygw.git
synced 2026-10-22 06:12:39 +00:00
fix: return InvalidAccessKeyId for unknown access keys in single, Vault and IPA IAM
Fixes #2445 The S3 auth middlewares (header, presigned and POST) return 403 `InvalidAccessKeyId` only when the IAM lookup returns exactly `auth.ErrNoSuchUser`, since they compare with `==`. Three IAM backends returned a different error for an unknown access key, and the client received that error unchanged. The root-only `IAMServiceSingle` returned the admin API's `ErrAdminUserNotFound`, so clients got 404 `XAdminUserNotFound`, which tools such as pgBackRest and rclone read as missing data rather than bad credentials. It now returns `ErrNoSuchUser`. The `ErrAdminUserNotFound` check in `resolveAccountsByLookup` is removed because no backend returns that error anymore. In the Vault backend, `KvV2Read` of a missing key fails with a 404, which was passed through and rendered as 500 `InternalError`. The same error broke the admin `create-bucket`, `change-bucket-owner` and `update-user` commands for unknown users, which returned 500 instead of `XAdminUserNotFound`. `GetUserAccount` now maps a 404 to `ErrNoSuchUser` on both the initial read and the retry after re-authentication, the same way `ListUserAccounts` already handles 404. FreeIPA reports a missing object as `NotFound` (code `4001`), both from `user_show` for an unknown user and from `vault_retrieve_internal` for a user who has no S3 vault. Both cases rendered as 500. `rpcInternal` now returns `errRpcNotFound`, which wraps `errRpc`, for code `4001`, and `GetUserAccount` maps it to `ErrNoSuchUser` in both places. A user without the vault has no S3 secret, so it is treated as a nonexistent account, and the PKCS#1 v1.5 to OAEP retry is skipped in that case.
This commit is contained in:
+1
-1
@@ -30,7 +30,7 @@ func resolveAccountsByLookup(accessKeyIDs []string, getUserAccount func(string)
|
||||
for _, access := range accessKeyIDs {
|
||||
_, err := getUserAccount(access)
|
||||
if err != nil {
|
||||
if err == ErrNoSuchUser || err == s3err.GetAPIError(s3err.ErrAdminUserNotFound) {
|
||||
if err == ErrNoSuchUser {
|
||||
missing = append(missing, access)
|
||||
continue
|
||||
}
|
||||
|
||||
+23
-4
@@ -137,6 +137,9 @@ func (ipa *IpaIAMService) GetUserAccount(access string) (Account, error) {
|
||||
}{}
|
||||
|
||||
err = ipa.rpc(req, &userResult)
|
||||
if errors.Is(err, errRpcNotFound) {
|
||||
return Account{}, ErrNoSuchUser
|
||||
}
|
||||
if err != nil {
|
||||
return Account{}, err
|
||||
}
|
||||
@@ -176,12 +179,16 @@ func (ipa *IpaIAMService) GetUserAccount(access string) (Account, error) {
|
||||
// ipaclient/plugins/vault.py's _do_internal - so try the default first
|
||||
// and fall back to OAEP so both KRA configurations keep working.
|
||||
data, err := ipa.retrieveVaultSecret(access, session_key, false)
|
||||
if err != nil {
|
||||
if err != nil && !errors.Is(err, errRpcNotFound) {
|
||||
debuglogger.IAMLogf("ipa vault_retrieve_internal with PKCS1v15 session key wrap failed, retrying with OAEP: %v", err)
|
||||
data, err = ipa.retrieveVaultSecret(access, session_key, true)
|
||||
if err != nil {
|
||||
return account, err
|
||||
}
|
||||
}
|
||||
if errors.Is(err, errRpcNotFound) {
|
||||
// a user without the vault has no secret, so it isn't an S3 account
|
||||
return Account{}, ErrNoSuchUser
|
||||
}
|
||||
if err != nil {
|
||||
return account, err
|
||||
}
|
||||
|
||||
aes, err := aes.NewCipher(session_key)
|
||||
@@ -332,6 +339,12 @@ func (p rpcResponse) String() string {
|
||||
|
||||
var errRpc = errors.New("IPA RPC error")
|
||||
|
||||
// errRpcNotFound is the IPA RPC error for an object that does not exist,
|
||||
// such as a user or a user's vault (FreeIPA's NotFound error)
|
||||
var errRpcNotFound = fmt.Errorf("not found: %w", errRpc)
|
||||
|
||||
const ipaErrNotFoundCode = 4001
|
||||
|
||||
func (ipa *IpaIAMService) rpc(req rpcRequest, value any) error {
|
||||
err := ipa.login()
|
||||
if err != nil {
|
||||
@@ -403,6 +416,12 @@ func (ipa *IpaIAMService) rpcInternal(req rpcRequest) (rpcResponse, error) {
|
||||
return rpcResponse{}, err
|
||||
}
|
||||
if string(result.Error) != "null" {
|
||||
rpcErr := struct {
|
||||
Code int `json:"code"`
|
||||
}{}
|
||||
if json.Unmarshal(result.Error, &rpcErr) == nil && rpcErr.Code == ipaErrNotFoundCode {
|
||||
return rpcResponse{}, fmt.Errorf("%s: %w", string(result.Error), errRpcNotFound)
|
||||
}
|
||||
return rpcResponse{}, fmt.Errorf("%s: %w", string(result.Error), errRpc)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
// Copyright 2026 Versity Software
|
||||
// This file is licensed under the Apache License, Version 2.0
|
||||
// (the "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
package auth
|
||||
|
||||
import (
|
||||
"crypto/rand"
|
||||
"crypto/rsa"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// FreeIPA JSON-RPC response bodies, as returned by a FreeIPA 4.13 server.
|
||||
const (
|
||||
ipaUserShowNotFound = `{"result": null, "error": {"code": 4001, "message": "nosuchkeyid0000000: user not found", "data": {"reason": "nosuchkeyid0000000: user not found"}, "name": "NotFound"}, "id": 0, "principal": "admin@EXAMPLE.TEST", "version": "4.13.1"}`
|
||||
ipaUserShowFound = `{"result": {"result": {"uid": ["novaultuser"], "uidnumber": ["1828600004"], "gidnumber": ["1828600004"]}, "value": "novaultuser", "summary": null}, "error": null, "id": 0, "principal": "admin@EXAMPLE.TEST", "version": "4.13.1"}`
|
||||
ipaVaultNotFound = `{"result": null, "error": {"code": 4001, "message": "s3secret: vault not found", "data": {"reason": "s3secret: vault not found"}, "name": "NotFound"}, "id": 1, "principal": "admin@EXAMPLE.TEST", "version": "4.13.1"}`
|
||||
ipaVersionError = `{"result": null, "error": {"code": 901, "message": "9.999 client incompatible with 2.257 server at 'https://ipa.example.test/ipa/xml'", "data": {"cver": "9.999", "sver": "2.257", "server": "https://ipa.example.test/ipa/xml"}, "name": "VersionError"}, "id": 0, "principal": "admin@EXAMPLE.TEST", "version": "4.13.1"}`
|
||||
)
|
||||
|
||||
func TestIpaIAMServiceGetUserAccountNotFound(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
responses map[string]string
|
||||
noSuchUser bool
|
||||
}{
|
||||
{
|
||||
name: "unknown user",
|
||||
responses: map[string]string{"user_show/1": ipaUserShowNotFound},
|
||||
noSuchUser: true,
|
||||
},
|
||||
{
|
||||
name: "user without vault",
|
||||
responses: map[string]string{
|
||||
"user_show/1": ipaUserShowFound,
|
||||
"vault_retrieve_internal/1": ipaVaultNotFound,
|
||||
},
|
||||
noSuchUser: true,
|
||||
},
|
||||
{
|
||||
name: "other rpc error",
|
||||
responses: map[string]string{"user_show/1": ipaVersionError},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
ipa := newFakeIpaIAMService(t, tt.responses)
|
||||
|
||||
_, err := ipa.GetUserAccount("NOSUCHKEYID0000000")
|
||||
if tt.noSuchUser {
|
||||
assert.Equal(t, ErrNoSuchUser, err)
|
||||
} else {
|
||||
assert.ErrorIs(t, err, errRpc)
|
||||
assert.NotErrorIs(t, err, ErrNoSuchUser)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// newFakeIpaIAMService returns an IpaIAMService talking to a fake FreeIPA
|
||||
// server that answers each JSON-RPC method with the given response body.
|
||||
func newFakeIpaIAMService(t *testing.T, responses map[string]string) *IpaIAMService {
|
||||
t.Helper()
|
||||
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/ipa/session/login_password" {
|
||||
return
|
||||
}
|
||||
var req struct {
|
||||
Method string `json:"method"`
|
||||
}
|
||||
assert.NoError(t, json.NewDecoder(r.Body).Decode(&req))
|
||||
resp, ok := responses[req.Method]
|
||||
assert.True(t, ok, "unexpected IPA method %q", req.Method)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.Write([]byte(resp))
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
key, err := rsa.GenerateKey(rand.Reader, 2048)
|
||||
assert.NoError(t, err)
|
||||
|
||||
return &IpaIAMService{
|
||||
client: *srv.Client(),
|
||||
version: IpaVersion,
|
||||
host: srv.URL,
|
||||
vaultName: "s3secret",
|
||||
kraTransportKey: &key.PublicKey,
|
||||
rootAcc: Account{Access: "root"},
|
||||
}
|
||||
}
|
||||
+2
-2
@@ -37,12 +37,12 @@ func (IAMServiceSingle) CreateAccount(account Account) error {
|
||||
}
|
||||
|
||||
// GetUserAccount returns root account, if the root access key
|
||||
// is provided and "ErrAdminUserNotFound" otherwise
|
||||
// is provided and ErrNoSuchUser otherwise
|
||||
func (s IAMServiceSingle) GetUserAccount(access string) (Account, error) {
|
||||
if access == s.root.Access {
|
||||
return s.root, nil
|
||||
}
|
||||
return Account{}, s3err.GetAPIError(s3err.ErrAdminUserNotFound)
|
||||
return Account{}, ErrNoSuchUser
|
||||
}
|
||||
|
||||
// ResolveAccounts returns the subset of accessKeyIDs that do not exist.
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
// Copyright 2026 Versity Software
|
||||
// This file is licensed under the Apache License, Version 2.0
|
||||
// (the "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
package auth
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/versity/versitygw/internal/sigv4auth"
|
||||
)
|
||||
|
||||
func TestIAMServiceSingleGetUserAccount(t *testing.T) {
|
||||
root := Account{Access: "root", Secret: "rootsecret", Role: RoleAdmin}
|
||||
iam := NewIAMServiceSingle(root)
|
||||
|
||||
acc, err := iam.GetUserAccount("root")
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, root, acc)
|
||||
|
||||
_, err = iam.GetUserAccount("NOSUCHKEYID0000000")
|
||||
assert.Equal(t, ErrNoSuchUser, err)
|
||||
}
|
||||
|
||||
// The S3 auth middlewares render InvalidAccessKeyId only when
|
||||
// ResolveDerivedKey returns exactly ErrNoSuchUser.
|
||||
func TestIAMServiceSingleResolveDerivedKeyUnknownAccess(t *testing.T) {
|
||||
root := Account{Access: "root", Secret: "rootsecret", Role: RoleAdmin}
|
||||
iam := NewIAMServiceSingle(root)
|
||||
|
||||
_, _, err := ResolveDerivedKey(iam, root, "NOSUCHKEYID0000000", "", "20260928", "us-east-1", sigv4auth.ServiceS3)
|
||||
assert.Equal(t, ErrNoSuchUser, err)
|
||||
}
|
||||
|
||||
func TestIAMServiceSingleResolveAccounts(t *testing.T) {
|
||||
iam := NewIAMServiceSingle(Account{Access: "root", Secret: "rootsecret", Role: RoleAdmin})
|
||||
|
||||
missing, err := iam.ResolveAccounts([]string{"root", "user1", "user2"})
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, []string{"user1", "user2"}, missing)
|
||||
}
|
||||
@@ -245,12 +245,18 @@ func (vt *VaultIAMService) GetUserAccount(access string) (Account, error) {
|
||||
if err != nil {
|
||||
reauthErr := vt.reAuthIfNeeded(err)
|
||||
if reauthErr != nil {
|
||||
if vault.IsErrorStatus(err, http.StatusNotFound) {
|
||||
return Account{}, ErrNoSuchUser
|
||||
}
|
||||
return Account{}, reauthErr
|
||||
}
|
||||
// retry once after re-auth
|
||||
resp, err = vt.client.Secrets.KvV2Read(context.Background(),
|
||||
vt.secretStoragePath+"/"+access, vt.kvReqOpts...)
|
||||
if err != nil {
|
||||
if vault.IsErrorStatus(err, http.StatusNotFound) {
|
||||
return Account{}, ErrNoSuchUser
|
||||
}
|
||||
return Account{}, err
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
// Copyright 2026 Versity Software
|
||||
// This file is licensed under the Apache License, Version 2.0
|
||||
// (the "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
package auth
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
// Vault answers a KV v2 read of a missing secret with 404 and this body.
|
||||
func TestVaultIAMServiceGetUserAccountUnknownReturnsErrNoSuchUser(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, "/v1/kv-v2/data/users/NOSUCHKEYID0000000", r.URL.Path)
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
w.Write([]byte(`{"errors":[]}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
iam, err := NewVaultIAMService(Account{Access: "root"}, srv.URL, "", "users", "", "", "", "", "token", "", "", "", "", "")
|
||||
assert.NoError(t, err)
|
||||
|
||||
_, err = iam.GetUserAccount("NOSUCHKEYID0000000")
|
||||
assert.Equal(t, ErrNoSuchUser, err)
|
||||
|
||||
missing, err := iam.ResolveAccounts([]string{"NOSUCHKEYID0000000"})
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, []string{"NOSUCHKEYID0000000"}, missing)
|
||||
}
|
||||
|
||||
// Any Vault error other than 404, such as a token whose policy doesn't cover
|
||||
// the path, must not be mistaken for a missing account.
|
||||
func TestVaultIAMServiceGetUserAccountPermissionDenied(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"errors":["permission denied"]}`))
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
iam, err := NewVaultIAMService(Account{Access: "root"}, srv.URL, "", "users", "", "", "", "", "token", "", "", "", "", "")
|
||||
assert.NoError(t, err)
|
||||
|
||||
_, err = iam.GetUserAccount("user1")
|
||||
assert.Error(t, err)
|
||||
assert.NotEqual(t, ErrNoSuchUser, err)
|
||||
}
|
||||
Reference in New Issue
Block a user