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 cert auth login attempts if ocsp_fail_open is true and servers are unreachable #25982

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

stevendpclark
Copy link
Contributor

When the cert auth had the role property ocsp_fail_open set to true, when network errors occurred, the errors would bubble up and prevent login attempts.

The error would look something like this:

vault: 2024/02/03 12:01:16 [ERR] GET .... = request failed: Get "...": context deadline exceeded (Client.Timeout exceeded while awaiting headers)

ocsp_max_retries config was added to allow easier testing of these use cases

@stevendpclark stevendpclark added bug Used to indicate a potential bug auth/cert Authentication - certificates backport/1.14.x labels Mar 15, 2024
@stevendpclark stevendpclark added this to the 1.14.11 milestone Mar 15, 2024
@stevendpclark stevendpclark self-assigned this Mar 15, 2024
@stevendpclark stevendpclark requested a review from a team as a code owner March 15, 2024 18:28
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Mar 15, 2024
Copy link

github-actions bot commented Mar 15, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Mar 15, 2024

Build Results:
All builds succeeded! ✅

})
}
goodTs := httptest.NewServer(ocspHandler(ocsp.Good))
revokeTs := httptest.NewServer(ocspHandler(ocsp.Revoked))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great functional reuse

require.NoError(t, err, "failed parsing certificate from file %s", certFile)

return caTLS
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome tests. Much thorough, wow.

@stevendpclark stevendpclark force-pushed the stevendpclark/vault-24614-ocsp-no-nextupdate branch from ddc3903 to 929d092 Compare March 18, 2024 19:29
Base automatically changed from stevendpclark/vault-24614-ocsp-no-nextupdate to main March 18, 2024 22:12
@stevendpclark stevendpclark force-pushed the stevendpclark/vault-24316-ocsp-fail-open branch from cb053c6 to 2c9f681 Compare March 19, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth/cert Authentication - certificates bug Used to indicate a potential bug hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants