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

Allow strictly binding tokens to source IP address #202

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

cipherboy
Copy link
Member

This adds the option, token_strictly_bind_ip, to strictly bind the
token's approved CIDRs to the source IP address a login request was
associated with. This means if the IP address were ever ported to a
system with a new IP address, the login workflow would have to be
restarted to request a fresh token.

This binding is done in accordance with the X-Forwarded-For header,
allowing the CIDR value to be populated and validated against the
correct end-client's IP address (when sent via trusted X-Forwarded-For
header).


This also fixes the rejection of invalid X-Forwarded-For headers:

When x_forwarded_for_reject_not_authorized=false is explicitly set in
the listener configuration, we need to remove the header before
processing the request, otherwise downstream consumers of the request
will not know if the header is authorized or not, and presume that,
because we passed it along, we trusted it.


This is missing tests; I might try using the container based testing infrastructure to do this; I think I can run OpenBao within the test suite and use a curl container to check if the binding works. Though, I don't think I'll have the capabilities to test X-Forwarded-For there.

Curious to get your thoughts on this one @thequailman!

Resolves: #32

@cipherboy cipherboy force-pushed the strictly-bind-tokens branch 2 times, most recently from b0be203 to 390494d Compare March 16, 2024 22:41
@cipherboy cipherboy marked this pull request as ready for review March 16, 2024 22:41
@cipherboy cipherboy marked this pull request as draft March 18, 2024 16:17
@cipherboy
Copy link
Member Author

I see, I'll need to address how the number of auth plugins are computed:


2024-03-16T22:56:36.3123147Z === �[31mFailed�[0m
2024-03-16T22:56:36.3123917Z === �[31mFAIL�[0m: command TestAuthEnableCommand_Run/mount_all (2.01s)
2024-03-16T22:56:36.3124851Z     auth_enable_test.go:208: expected 9 credential backends, got 10
2024-03-16T22:56:36.3125964Z         	expected: []string{"ldap", "oidc", "radius", "kerberos", "kubernetes", "userpass", "approle", "cert", "jwt"}
2024-03-16T22:56:36.3127811Z         	backends: []string{"approle", "cert", "jwt", "kerberos", "kubernetes", "ldap", "radius", "token", "userpass", "userpass_binary"}

Perhaps I'll move backend/credential/userpass_binary to vault/external_tests for now.

sdk/helper/tokenutil/tokenutil.go Outdated Show resolved Hide resolved
@cipherboy cipherboy marked this pull request as ready for review March 18, 2024 20:30
@cipherboy cipherboy force-pushed the strictly-bind-tokens branch 2 times, most recently from 1d2b2ea to 1767f29 Compare March 18, 2024 20:54
@naphelps naphelps self-requested a review March 19, 2024 17:31
This adds the option, token_strictly_bind_ip, to strictly bind the
token's approved CIDRs to the source IP address a login request was
associated with. This means if the IP address were ever ported to a
system with a new IP address, the login workflow would have to be
restarted to request a fresh token.

This binding is done in accordance with the X-Forwarded-For header,
allowing the CIDR value to be populated and validated against the
correct end-client's IP address (when sent via trusted X-Forwarded-For
header).

Resolves: openbao#32

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
When x_forwarded_for_reject_not_authorized=false is explicitly set in
the listener configuration, we need to remove the header before
processing the request, otherwise downstream consumers of the request
will not know if the header is authorized or not, and presume that,
because we passed it along, we trusted it.

This is likely not a security issue as plugins do not have access to
X-Forwarded-For by default; only if it were added to the
passthrough_request_headers would it be accessible to plugins.

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
@naphelps naphelps merged commit 2c27dde into openbao:main Mar 19, 2024
73 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bind tokens to an IP address on issuance
3 participants