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

Authenticating AWS ALB #73

Closed
drwxmrrs opened this issue Feb 9, 2025 · 15 comments
Closed

Authenticating AWS ALB #73

drwxmrrs opened this issue Feb 9, 2025 · 15 comments
Labels
bug Something isn't working

Comments

@drwxmrrs
Copy link

drwxmrrs commented Feb 9, 2025

Hey OpenIDC team, just wanted to say thanks for putting in so much effort into this module, it's working great for us so far.

We've just come up against one hurdle which I'd love some help with. How should someone verify a token signed by ALB?

Right now we've set this, and it works:

OAuth2TokenVerify eckey_uri https://public-keys.auth.elb.{{ our region}}.amazonaws.com/{{ our kid }}

My only concern here is that KID field is hardcoded, just wondering if that should come from the token?
I've had a look through the code, and I can see there is a "peek" happening, so maybe the kid could be grabbed from that?

Second concern is - how do we verify the signer? Is that part of the scope for OAuth2TokenVerify, or should we just have a seperate Require oauth2_claim signer XXX for that?

@zandbelt
Copy link
Member

zandbelt commented Feb 9, 2025

Pulling the kid from the token seems to open up an attack surface: one would just allow any token that verifies, including ones stolen/leaked/abused from ALBs that you don't control. I guess that also answers your second question since the signer is associated with the kid.

@drwxmrrs
Copy link
Author

drwxmrrs commented Feb 9, 2025

Thanks @zandbelt,

The kid seems to change (not sure how often, but it's changed on me in the last 30 minutes) - not sure how to approach this 🤔

(I previously edited this comment saying the kid stayed the same, but have now just confirmed it's changed)

@zandbelt
Copy link
Member

zandbelt commented Feb 9, 2025

I just read up on the Amazon docs and they suggest to use the approach that you have described indeed.... I don't think that makes much sense but it would allow them to rotate the key indeed. I'm not sure if they do, I don't have any experience with it myself, I just added this way back when someone suggested it may be useful. I'm not aware of any users today. Also we don't have a real test environment anymore, just unit tests for this feature that don't rotate the key, as we were not aware this was an option at the time.

@zandbelt zandbelt added the bug Something isn't working label Feb 9, 2025
@zandbelt
Copy link
Member

zandbelt commented Feb 9, 2025

since you suggest ALBs do rotate the key, and they do it frequently, this is functionality that is not usable as it is and it should be fixed; I'm not sure when we'll get to this (we also need to have a real test environment during development and adapt our unit tests), of course a PR is welcome...

@drwxmrrs
Copy link
Author

drwxmrrs commented Feb 9, 2025

No problems at all,

In the mean time, we might just re-authenticate the Cognito token (the X-Amzn-AccessToken) using the jwks_uri method.

And yes, I'll see if I can put together a PR 🤔 can't say I've done much C in my life - to keep the feature "tight", and backwards-compatible (just in case anyone is using eckey_uri), do you think a new OAuth2TokenVerify might be best?

OAuth2TokenVerify aws_alb <ALB_ARN>

We'd use the ARN to construct the keys endpoint, append the KID, and also verify the ARN = signer inside the token header.

  • Given OAuth2TokenVerify aws_alb <ALB_ARN>
  • Where ALB_ARN = arn:aws:elasticloadbalancing:<region>:<accountID>:loadbalancer/app/<name>/<id>
  • URI = https://public-keys.auth.elb.<region from ALB_ARN>.amazonaws.com/<kid>
  • Do lookup, grab key, verify
  • Then ensure signer = ALB_ARN

Largely, it would just call the existing eckey_uri to do the actual key verification, but maybe a wrapper to handle the ALB specific stuff.

?

zandbelt added a commit to OpenIDC/liboauth2 that referenced this issue Feb 10, 2025
which supports key rotation, see:
OpenIDC/mod_oauth2#73

Signed-off-by: Hans Zandbelt <[email protected]>
@zandbelt
Copy link
Member

it is now done here https://github.com/OpenIDC/liboauth2/tree/alb-ec-key-signer-update ; by accident the unit test initially pulled live keys from AWS and succeeded so I'm pretty convinced that it all works now; please confirm

@drwxmrrs
Copy link
Author

Thanks, will test this first thing in the morning (AEST) and get back to you!

@drwxmrrs
Copy link
Author

drwxmrrs commented Feb 10, 2025

@zandbelt Looks good to me 🚀

  1. Ensure it resolves kid dynamically ✅
  2. Fails if signer != aws_alb - ✅
  3. Fails if token expired ✅
  4. Sets all relevant OAUTH2_CLAIM_* on success ✅
  5. Require oauth2_claim works ✅

Thanks for your help with this one.

@zandbelt
Copy link
Member

I will merge the branch and release 2.1.0 later this week. Thank you for your contribution!

@drwxmrrs
Copy link
Author

Fantastic, can't wait!

@zandbelt
Copy link
Member

squash-merged and released in https://github.com/OpenIDC/liboauth2/releases/tag/v2.1.0

@drwxmrrs
Copy link
Author

Thanks @zandbelt!

You wouldn't happen to know if there are any arm builds available for Deb? Was looking at the repository and couldn't find any in apt

@zandbelt
Copy link
Member

the 2.1.0 version will take a while to get there, but 2.0.0 is at e.g. https://packages.debian.org/sid/arm64/liboauth2-0/download

@drwxmrrs
Copy link
Author

Great thank you!

Any reason the ARM builders are commented out in GH actions? Not working?

@zandbelt
Copy link
Member

yes, not working; vague crashes in make check, perhaps related to memory alignment; I haven't figured that out yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants