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 server side AWS account ID log redaction #327

Merged
merged 1 commit into from
Jul 7, 2020
Merged
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
19 changes: 12 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,13 @@ as a user could potentially change this on the client side.

## API Authorization from Outside a Cluster

It is possible to make requests to the Kubernetes API from a client that is outside the cluster, be that using the
bare Kubernetes REST API or from one of the language specific Kubernetes clients
It is possible to make requests to the Kubernetes API from a client that is outside the cluster, be that using the
bare Kubernetes REST API or from one of the language specific Kubernetes clients
(e.g., [Python](https://github.com/kubernetes-client/python)). In order to do so, you must create a bearer token that
is included with the request to the API. This bearer token requires you append the string `k8s-aws-v1.` with a
base64 encoded string of a signed HTTP request to the STS GetCallerIdentity Query API. This is then sent it in the
`Authorization` header of the request. Something to note though is that the IAM Authenticator explicitly omits
base64 padding to avoid any `=` characters thus guaranteeing a string safe to use in URLs. Below is an example in
is included with the request to the API. This bearer token requires you append the string `k8s-aws-v1.` with a
base64 encoded string of a signed HTTP request to the STS GetCallerIdentity Query API. This is then sent it in the
`Authorization` header of the request. Something to note though is that the IAM Authenticator explicitly omits
base64 padding to avoid any `=` characters thus guaranteeing a string safe to use in URLs. Below is an example in
Python on how this token would be constructed:

```python
Expand Down Expand Up @@ -340,7 +340,7 @@ def get_bearer_token(cluster_id, region):

# remove any base64 encoding padding:
return 'k8s-aws-v1.' + re.sub(r'=*', '', base64_url)

# If making a HTTP request you would create the authorization headers as follows:

headers = {'Authorization': 'Bearer ' + get_bearer_token('my_cluster', 'us-east-1')}
Expand Down Expand Up @@ -394,6 +394,11 @@ server:
# role to assume before querying EC2 API in order to discover metadata like EC2 private DNS Name
ec2DescribeInstancesRoleARN: arn:aws:iam::000000000000:role/DescribeInstancesRole

# AWS Account IDs to scrub from server logs. (Defaults to empty list)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to give the ability to redact more than just account IDs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could certainly add that at some point

scrubbedAccounts:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: To me "redacted" is more explicit than "scrubbed"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went back and forth on this, but in my mind "redacted" sounded too much like the whole log entry was omitted, and in this case I'm just redacting part of the log line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like and understand "scrub" FWIW. 👍

- "111122223333"
- "222233334444"

# each mapRoles entry maps an IAM role to a username and set of groups
# Each username and group can optionally contain template parameters:
# 1) "{{AccountID}}" is the 12 digit AWS ID.
Expand Down
1 change: 1 addition & 0 deletions cmd/aws-iam-authenticator/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func getConfig() (config.Config, error) {
BackendMode: viper.GetStringSlice("server.backendMode"),
EC2DescribeInstancesQps: viper.GetInt("server.ec2DescribeInstancesQps"),
EC2DescribeInstancesBurst: viper.GetInt("server.ec2DescribeInstancesBurst"),
ScrubbedAWSAccounts: viper.GetStringSlice("server.scrubbedAccounts"),
}
if err := viper.UnmarshalKey("server.mapRoles", &cfg.RoleMappings); err != nil {
return cfg, fmt.Errorf("invalid server role mappings: %v", err)
Expand Down
4 changes: 4 additions & 0 deletions pkg/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ type Config struct {
// IAM ARN from these accounts automatically maps to the Kubernetes username.
AutoMappedAWSAccounts []string

// ScrubbedAWSAccounts is a list of AWS accounts that the role ARNs and uids
// are scrubbed from server log statements
ScrubbedAWSAccounts []string

// ServerEC2DescribeInstancesRoleARN is an optional AWS Resource Name for an IAM Role to be assumed
// before calling ec2:DescribeInstances to determine the private DNS of the calling kubelet (EC2 Instance).
// If nil, defaults to using the IAM Role attached to the instance where aws-iam-authenticator is
Expand Down
70 changes: 45 additions & 25 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,12 @@ var (
// server state (internal)
type handler struct {
http.ServeMux
verifier token.Verifier
metrics metrics
ec2Provider ec2provider.EC2Provider
clusterID string
mappers []mapper.Mapper
verifier token.Verifier
metrics metrics
ec2Provider ec2provider.EC2Provider
clusterID string
mappers []mapper.Mapper
scrubbedAccounts []string
}

// metrics are handles to the collectors for prometheous for the various metrics we are tracking.
Expand Down Expand Up @@ -173,11 +174,12 @@ func (c *Server) getHandler(mappers []mapper.Mapper, ec2DescribeQps int, ec2Desc
}

h := &handler{
verifier: token.NewVerifier(c.ClusterID),
metrics: createMetrics(),
ec2Provider: ec2provider.New(c.ServerEC2DescribeInstancesRoleARN, ec2DescribeQps, ec2DescribeBurst),
clusterID: c.ClusterID,
mappers: mappers,
verifier: token.NewVerifier(c.ClusterID),
metrics: createMetrics(),
ec2Provider: ec2provider.New(c.ServerEC2DescribeInstancesRoleARN, ec2DescribeQps, ec2DescribeBurst),
clusterID: c.ClusterID,
mappers: mappers,
scrubbedAccounts: c.Config.ScrubbedAWSAccounts,
}

h.HandleFunc("/authenticate", h.authenticateEndpoint)
Expand Down Expand Up @@ -240,6 +242,15 @@ func duration(start time.Time) float64 {
return time.Since(start).Seconds()
}

func (h *handler) isLoggableIdentity(identity *token.Identity) bool {
for _, account := range h.scrubbedAccounts {
if identity.AccountID == account {
return false
}
}
return true
}

func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request) {
start := time.Now()
log := logrus.WithFields(logrus.Fields{
Expand Down Expand Up @@ -289,16 +300,18 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
return
}

log.WithFields(logrus.Fields{
"accesskeyid": identity.AccessKeyID,
"arn": identity.ARN,
"accountid": identity.AccountID,
"userid": identity.UserID,
"session": identity.SessionName,
}).Info("STS response")

// look up the ARN in each of our mappings to fill in the username and groups
log = log.WithField("arn", identity.CanonicalARN)
if h.isLoggableIdentity(identity) {
log.WithFields(logrus.Fields{
"accesskeyid": identity.AccessKeyID,
"arn": identity.ARN,
"accountid": identity.AccountID,
"userid": identity.UserID,
"session": identity.SessionName,
}).Info("STS response")

// look up the ARN in each of our mappings to fill in the username and groups
log = log.WithField("arn", identity.CanonicalARN)
}

username, groups, err := h.doMapping(identity)
if err != nil {
Expand All @@ -309,8 +322,11 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
return
}

// use a prefixed UID that includes the AWS account ID and AWS user ID ("AROAAAAAAAAAAAAAAAAAA")
uid := fmt.Sprintf("aws-iam-authenticator:%s:%s", identity.AccountID, identity.UserID)
uid := fmt.Sprintf("aws-iam-authenticator:administrative:%s", username)
if h.isLoggableIdentity(identity) {
// use a prefixed UID that includes the AWS account ID and AWS user ID ("AROAAAAAAAAAAAAAAAAAA")
uid = fmt.Sprintf("aws-iam-authenticator:%s:%s", identity.AccountID, identity.UserID)
}

// the token is valid and the role is mapped, return success!
log.WithFields(logrus.Fields{
Expand All @@ -320,16 +336,20 @@ func (h *handler) authenticateEndpoint(w http.ResponseWriter, req *http.Request)
}).Info("access granted")
h.metrics.latency.WithLabelValues(metricSuccess).Observe(duration(start))
w.WriteHeader(http.StatusOK)

userExtra := map[string]authenticationv1beta1.ExtraValue{}
if h.isLoggableIdentity(identity) {
userExtra["accessKeyId"] = authenticationv1beta1.ExtraValue{identity.AccessKeyID}
}

json.NewEncoder(w).Encode(authenticationv1beta1.TokenReview{
Status: authenticationv1beta1.TokenReviewStatus{
Authenticated: true,
User: authenticationv1beta1.UserInfo{
Username: username,
UID: uid,
Groups: groups,
Extra: map[string]authenticationv1beta1.ExtraValue{
"accessKeyId": {identity.AccessKeyID},
},
Extra: userExtra,
},
},
})
Expand Down
30 changes: 30 additions & 0 deletions pkg/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,36 @@ func TestAuthenticateUnableToDecodeBodyCRD(t *testing.T) {
validateMetrics(t, validateOpts{malformed: 1})
}

func testIsLoggableIdentity(t *testing.T) {
h := &handler{scrubbedAccounts: []string{"111122223333", "012345678901"}}

cases := []struct {
identity *token.Identity
want bool
}{
{
&token.Identity{AccountID: "222233334444"},
true,
},
{
&token.Identity{AccountID: "111122223333"},
false,
},
}

for _, c := range cases {
if got := h.isLoggableIdentity(c.identity); got != c.want {
t.Errorf(
"Unexpected result: isLoggableIdentity(%v): got: %t, wanted %t",
c.identity,
got,
c.want,
)
}
}

}

type testVerifier struct {
identity *token.Identity
err error
Expand Down