Is this vulnerable to the presigned GetCallerIdentity issue (CVE-2020-16250) impacting Hashicorp Vault?

[0xdabbad00]

Google's Project Zero posted this issue today with Hashicorp Vault: https://googleprojectzero.blogspot.com/2020/10/enter-the-vault-auth-issues-hashicorp-vault.html

Your README describes a similar mechanism and states "This mechanism is borrowed with a few changes from Vault." I've not dug into the code, but I'm suspicious that you may be vulnerable to something similar.

[aidansteele]

Looks like it's not vulnerable to the same issue

aws-iam-authenticator/pkg/token/token.go

Lines 466 to 468 in 292b9b8

	if queryParamsLower.Get("action") != "GetCallerIdentity" {
	return nil, FormatError{"unexpected action parameter in pre-signed URL"}
	}

[mattmoyer]

tl;dr no, we're safe

Some caveats:

• I originally wrote some of the code in question, but I'm not the most active maintainer recently.
• This is not an official security opinion of my employer.
• I'm not a full time security researcher, but I do have some security background and was feeling particularly paranoid when I copied this approach from Vault originally 😄 .

I think aws-iam-authenticator is not vulnerable to the techniques demonstrated in the excellent Project Zero writeup:

1. We have stricter validation for the signed request. In aws-iam-authenticator the request is always just a single URL, parsed by net/url which is pretty well hardened against URL parser tricks. We don't allow a request body at all, force the upstream STS request to always be a GET, allow only certain known URL parameters, and as @aidansteele mentioned, we check that action is always GetCallerIdentity.

   Together, I think these should always prevent the root bug that P0 demonstrated in Vault, and I'm cautiously optimistic that this code is solid.

2. As of v0.5.2 (released today) the Host validation was switched from a regex to one based on a list of STS endpoints in each AWS partition I'm not aware of any known vulnerabilities in the previous validation here, but the new approach seems very robust.

3. We do not rely on the Go XML parser libraries, instead we always make the STS request with Accept: application/json and use encoding/json to unmarshal the response body. This was mostly accidental, but I would expect the encoding/json parser to be more strict about the input than encoding/xml seems to be. I don't know that this is foolproof, but it seems like a bit more rigid mechanism.

Again, this is just my thoughts after an initial triage, but I'm comfortable closing the issue for now.

I'd encourage anyone with more idea for further hardening to please share here. If you think you've found a way to exploit this or any other vulnerability in a Kubernetes component, please follow the disclosure procedure.

Thanks for sharing!

/label security
/close

[k8s-ci-robot]

@mattmoyer: The label(s) /label security cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash

Details

In response to this:

tl;dr no, we're safe

Some caveats:

• I originally wrote some of the code in question, but I'm not the most active maintainer recently.
• This is not an official security opinion of my employer.
• I'm not a full time security researcher, but I do have some security background and was feeling particularly paranoid when I copied this approach from Vault originally 😄 .

I think aws-iam-authenticator is not vulnerable to the techniques demonstrated in the excellent Project Zero writeup:

1. We have stricter validation for the signed request. In aws-iam-authenticator the request is always just a single URL, parsed by net/url which is pretty well hardened against URL parser tricks. We don't allow a request body at all, force the upstream STS request to always be a GET, allow only certain known URL parameters, and as @aidansteele mentioned, we check that action is always GetCallerIdentity.

Together, I think these should always prevent the root bug that P0 demonstrated in Vault, and I'm cautiously optimistic that this code is solid.

1. As of v0.5.2 (released today) the Host validation was switched from a regex to one based on a list of STS endpoints in each AWS partition I'm not aware of any known vulnerabilities in the previous validation here, but the new approach seems very robust.

2. We do not rely on the Go XML parser libraries, instead we always make the STS request with Accept: application/json and use encoding/json to unmarshal the response body. This was mostly accidental, but I would expect the encoding/json parser to be more strict about the input than encoding/xml seems to be. I don't know that this is foolproof, but it seems like a bit more rigid mechanism.

Again, this is just my thoughts after an initial triage, but I'm comfortable closing the issue for now.

I'd encourage anyone with more idea for further hardening to please share here. If you think you've found a way to exploit this or any other vulnerability in a Kubernetes component, please follow the disclosure procedure.

Thanks for sharing!

/label security
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

@mattmoyer: You can't close an active issue/PR unless you authored it or you are a collaborator.

Details

In response to this:

tl;dr no, we're safe

Some caveats:

• I originally wrote some of the code in question, but I'm not the most active maintainer recently.
• This is not an official security opinion of my employer.
• I'm not a full time security researcher, but I do have some security background and was feeling particularly paranoid when I copied this approach from Vault originally 😄 .

I think aws-iam-authenticator is not vulnerable to the techniques demonstrated in the excellent Project Zero writeup:

1. We have stricter validation for the signed request. In aws-iam-authenticator the request is always just a single URL, parsed by net/url which is pretty well hardened against URL parser tricks. We don't allow a request body at all, force the upstream STS request to always be a GET, allow only certain known URL parameters, and as @aidansteele mentioned, we check that action is always GetCallerIdentity.

Together, I think these should always prevent the root bug that P0 demonstrated in Vault, and I'm cautiously optimistic that this code is solid.

1. As of v0.5.2 (released today) the Host validation was switched from a regex to one based on a list of STS endpoints in each AWS partition I'm not aware of any known vulnerabilities in the previous validation here, but the new approach seems very robust.

2. We do not rely on the Go XML parser libraries, instead we always make the STS request with Accept: application/json and use encoding/json to unmarshal the response body. This was mostly accidental, but I would expect the encoding/json parser to be more strict about the input than encoding/xml seems to be. I don't know that this is foolproof, but it seems like a bit more rigid mechanism.

Again, this is just my thoughts after an initial triage, but I'm comfortable closing the issue for now.

I'd encourage anyone with more idea for further hardening to please share here. If you think you've found a way to exploit this or any other vulnerability in a Kubernetes component, please follow the disclosure procedure.

Thanks for sharing!

/label security
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[micahhausler]

/close

[k8s-ci-robot]

@micahhausler: Closing this issue.

Details

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[0xdabbad00]

Felix Wilhelm, who found the original issue with Vault, was able to find an issue with this code, although a different one than the Vault project had: https://bugs.chromium.org/p/project-zero/issues/detail?id=2066