s3tables: harden auth and error handling

- Add authorization checks to all S3 Tables handlers (policy, table ops) to enforce security
- Improve error handling to distinguish between NotFound (404) and InternalError (500)
- Fix directory FileMode usage in filer_ops
- Improve test randomness for version tokens
- Update permissions comments to acknowledge IAM gaps
This commit is contained in:
Chris Lu
2026-01-28 11:49:57 -08:00
parent a3af5eb77a
commit dc4c62e742
4 changed files with 122 additions and 5 deletions

View File

@@ -11,6 +11,13 @@ import (
// handlePutTableBucketPolicy puts a policy on a table bucket
func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("PutTableBucketPolicy", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table bucket policy")
return NewAuthError("PutTableBucketPolicy", principal, "not authorized to put table bucket policy")
}
var req PutTableBucketPolicyRequest
if err := h.readRequestBody(r, &req); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
@@ -65,6 +72,13 @@ func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *h
// handleGetTableBucketPolicy gets the policy of a table bucket
func (h *S3TablesHandler) handleGetTableBucketPolicy(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("GetTableBucketPolicy", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket policy")
return NewAuthError("GetTableBucketPolicy", principal, "not authorized to get table bucket policy")
}
var req GetTableBucketPolicyRequest
if err := h.readRequestBody(r, &req); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
@@ -91,7 +105,11 @@ func (h *S3TablesHandler) handleGetTableBucketPolicy(w http.ResponseWriter, r *h
})
if err != nil {
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchPolicy, "table bucket policy not found")
if errors.Is(err, filer_pb.ErrNotFound) || errors.Is(err, ErrAttributeNotFound) {
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchPolicy, "table bucket policy not found")
} else {
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to get table bucket policy: %v", err))
}
return err
}
@@ -105,6 +123,13 @@ func (h *S3TablesHandler) handleGetTableBucketPolicy(w http.ResponseWriter, r *h
// handleDeleteTableBucketPolicy deletes the policy of a table bucket
func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("DeleteTableBucketPolicy", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table bucket policy")
return NewAuthError("DeleteTableBucketPolicy", principal, "not authorized to delete table bucket policy")
}
var req DeleteTableBucketPolicyRequest
if err := h.readRequestBody(r, &req); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
@@ -138,6 +163,13 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r
// handlePutTablePolicy puts a policy on a table
func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("PutTablePolicy", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table policy")
return NewAuthError("PutTablePolicy", principal, "not authorized to put table policy")
}
var req PutTablePolicyRequest
if err := h.readRequestBody(r, &req); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
@@ -198,6 +230,13 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re
// handleGetTablePolicy gets the policy of a table
func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("GetTablePolicy", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table policy")
return NewAuthError("GetTablePolicy", principal, "not authorized to get table policy")
}
var req GetTablePolicyRequest
if err := h.readRequestBody(r, &req); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
@@ -230,7 +269,11 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re
})
if err != nil {
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchPolicy, "table policy not found")
if errors.Is(err, filer_pb.ErrNotFound) || errors.Is(err, ErrAttributeNotFound) {
h.writeError(w, http.StatusNotFound, ErrCodeNoSuchPolicy, "table policy not found")
} else {
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to get table policy: %v", err))
}
return err
}
@@ -244,6 +287,13 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re
// handleDeleteTablePolicy deletes the policy of a table
func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("DeleteTablePolicy", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table policy")
return NewAuthError("DeleteTablePolicy", principal, "not authorized to delete table policy")
}
var req DeleteTablePolicyRequest
if err := h.readRequestBody(r, &req); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
@@ -299,6 +349,13 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque
return fmt.Errorf("tags are required")
}
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CanManageTags(principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to tag resource")
return NewAuthError("TagResource", principal, "not authorized to tag resource")
}
// Parse resource ARN to determine if it's a bucket or table
resourcePath, extendedKey, err := h.resolveResourcePath(req.ResourceARN)
if err != nil {
@@ -311,10 +368,14 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque
err = filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error {
data, err := h.getExtendedAttribute(r.Context(), client, resourcePath, extendedKey)
if err == nil {
json.Unmarshal(data, &existingTags)
return json.Unmarshal(data, &existingTags)
}
return nil
})
if err != nil {
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to read existing tags: %v", err))
return err
}
// Merge new tags
for k, v := range req.Tags {
@@ -342,6 +403,13 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque
// handleListTagsForResource lists tags for a resource
func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *http.Request, filerClient FilerClient) error {
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("ListTagsForResource", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to list tags for resource")
return NewAuthError("ListTagsForResource", principal, "not authorized to list tags for resource")
}
var req ListTagsForResourceRequest
if err := h.readRequestBody(r, &req); err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
@@ -368,6 +436,11 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht
return json.Unmarshal(data, &tags)
})
if err != nil {
h.writeError(w, http.StatusInternalServerError, ErrCodeInternalError, fmt.Sprintf("failed to list tags: %v", err))
return err
}
resp := &ListTagsForResourceResponse{
Tags: tags,
}
@@ -394,6 +467,13 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req
return fmt.Errorf("tagKeys are required")
}
// Check permission
principal := h.getPrincipalFromRequest(r)
if !CheckPermission("UntagResource", principal, h.accountID) {
h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to untag resource")
return NewAuthError("UntagResource", principal, "not authorized to untag resource")
}
resourcePath, extendedKey, err := h.resolveResourcePath(req.ResourceARN)
if err != nil {
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())