Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added accountID to throttled error log #765

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,9 +333,9 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
// if the token is invalid, reject with a 403
identity, err := h.verifier.Verify(tokenReview.Spec.Token)
if err != nil {
if _, ok := err.(token.STSThrottling); ok {
if throttleErr, ok := err.(token.STSThrottling); ok {
metrics.Get().Latency.WithLabelValues(metrics.STSThrottling).Observe(duration(start))
log.WithError(err).Warn("access denied")
log.WithError(throttleErr).WithField("accountID", throttleErr.AccountID()).Warn("access denied")
w.WriteHeader(http.StatusTooManyRequests)
w.Write(tokenReviewDenyJSON)
return
Expand Down
39 changes: 39 additions & 0 deletions pkg/token/akid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package token

import (
"encoding/base32"
"encoding/hex"
"fmt"
)

// accountForAKID is a best-effort method to extract the account ID from an AKID for
// logging on throttled requests. This should not be called on untrusted input (i.e.
// AKID from the request before validating the request from STS).
//
// This is not foolproof, but avoids an `sts:GetAccessKeyInfo` call per AKID.
// adapted from https://hackingthe.cloud/aws/enumeration/get-account-id-from-keys/
func accountForAKID(akid string) string {
if len(akid) < 20 {
// too short
return ""
}
decoded, err := base32.StdEncoding.DecodeString(akid[4:])
if err != nil {
// decoding error
return ""
}
y := decoded[:6]
z := uint64(0)
for i := 0; i < len(y); i++ {
z = (z << 8) | uint64(y[i])
}
// this mask bytestring is always valid
maskBytes, _ := hex.DecodeString("7fffffffff80")
mask := uint64(0)
for i := 0; i < len(maskBytes); i++ {
mask = (mask << 8) | uint64(maskBytes[i])
}
// Apply mask and shift right by 7 bits
e := (z & mask) >> 7
return fmt.Sprintf("%012d", e)
}
44 changes: 44 additions & 0 deletions pkg/token/akid_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package token

import (
"testing"
)

func TestAccountForAKID(t *testing.T) {
testcases := []struct {
name string
akid string
expected string
wantErr error
}{
{
name: "empty akid",
akid: "",
expected: "",
},
{
name: "akid with account",
akid: "ASIAR2TG44V5PDTTBZRR",
expected: "125843596666",
},
{
name: "account starting with a 0",
akid: "ASIAQNZGKIQY56JQ7WML",
expected: "029608264753",
},
{
name: "non base32 encoded akid",
akid: "ASIAc29tZXRoaW5nCg==",
expected: "",
},
}

for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
actual := accountForAKID(tc.akid)
if actual != tc.expected {
t.Errorf("expected %s, got %s", tc.expected, actual)
}
})
}
}
16 changes: 11 additions & 5 deletions pkg/token/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,21 @@ func NewSTSError(m string) STSError {

// STSThrottling is returned when there was STS Throttling.
type STSThrottling struct {
message string
message string
accountID string
}

func (e STSThrottling) AccountID() string {
return e.accountID
}

func (e STSThrottling) Error() string {
return "sts getCallerIdentity was throttled: " + e.message
return "sts getCallerIdentity for account " + e.accountID + " was throttled: " + e.message
}

// NewSTSError creates a error of type STS.
func NewSTSThrottling(m string) STSThrottling {
return STSThrottling{message: m}
func NewSTSThrottling(m, accountID string) STSThrottling {
return STSThrottling{message: m, accountID: accountID}
}

var parameterWhitelist = map[string]bool{
Expand Down Expand Up @@ -586,8 +591,9 @@ func (v tokenVerifier) Verify(token string) (*Identity, error) {
// refer to https://docs.aws.amazon.com/STS/latest/APIReference/CommonErrors.html and log
// response body for STS Throttling is {"Error":{"Code":"Throttling","Message":"Rate exceeded","Type":"Sender"},"RequestId":"xxx"}
if strings.Contains(responseStr, "Throttling") {
// STS validated the request, but throttled it: we can trust that enough to report the accountID
metrics.Get().StsThrottling.Inc()
return nil, NewSTSThrottling(responseStr)
return nil, NewSTSThrottling(responseStr, accountForAKID(accessKeyID))
}
return nil, NewSTSError(fmt.Sprintf("error from AWS (expected 200, got %d). Body: %s", response.StatusCode, responseStr))
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/token/token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func TestVerifyTokenPreSTSValidations(t *testing.T) {
func TestVerifyHTTPThrottling(t *testing.T) {
testVerifier := newVerifier("aws", 400, "{\\\"Error\\\":{\\\"Code\\\":\\\"Throttling\\\",\\\"Message\\\":\\\"Rate exceeded\\\",\\\"Type\\\":\\\"Sender\\\"},\\\"RequestId\\\":\\\"8c2d3520-24e1-4d5c-ac55-7e226335f447\\\"}", nil)
_, err := testVerifier.Verify(validToken)
errorContains(t, err, "sts getCallerIdentity was throttled")
errorContains(t, err, "sts getCallerIdentity for account 073224499664 was throttled")
assertSTSThrottling(t, err)
}

Expand Down
Loading