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

Adjust reload keystore test to pass in FIPS #56889

Merged
merged 2 commits into from
May 19, 2020

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented May 18, 2020

In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is
throwing(AEADBadTagException). Other implementations from other
Security Providers fail with a different exception and as such we
cannot differentiate between a corrupted file and a wrong password
in a foolproof way.
As in other tests such as in
KeyStoreWrapperTests#testDecryptKeyStoreWithWrongPassword
we handle this by matching both possible exception messages.

In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is
throwing(AEADBadTagException). Other implementations from other
Security Providers fail with a different exception and as such we
cannot differentiate between a corrupted file and a wrong password
in a foolproof way.
In other tests such as in
KeyStoreWrapperTests#testDecryptKeyStoreWithWrongPassword we handle
this by matching both possible exception messages but it is not as
easy to do in the REST yml tests. What's more, we don't have a way
to skip YAML tests in FIPS mode, thus this change removes the
matching of the exception string entirely so that the test can
run successfully in FIPS mode.
@jkakavas jkakavas added >test-failure Triaged test failures from CI :Security/Security Security issues without another label v8.0.0 v7.7.1 v7.8.1 labels May 18, 2020
@jkakavas jkakavas requested a review from ywangd May 18, 2020 09:50
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Security)

@elasticmachine elasticmachine added the Team:Security Meta label for security team label May 18, 2020
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

It is technically not part of this PR, but I believe the following skip version should be 7.99.99:

- skip:
version: " - 7.9.99"
reason: "support for reloading password protected keystores was introduced in 8.0.0"

It does not really cause any issue since the same code in 7.x branch does use 7.99.99. But still would be better to have consistency.

@@ -18,7 +18,6 @@ setup:
- is_true: nodes
- is_true: cluster_name
- match: { nodes.$node_id.reload_exception.type: "security_exception" }
- match: { nodes.$node_id.reload_exception.reason: "Provided keystore password was incorrect" }
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, it is possible to test for both error texts with regex

  - match: { nodes.$node_id.reload_exception.reason:
               /^(Provided\skeystore\spassword\swas\sincorrect|
               Keystore\shas\sbeen\scorrupted\sor\stampered\swith)$/ }

@jkakavas
Copy link
Member Author

jkakavas commented May 19, 2020

It does not really cause any issue since the same code in 7.x branch does use 7.99.99. But still would be better to have consistency.

This can actually be changed to 7.6.99, the password protected keystore is in 7.x and 7.7. Spotted in #55069 but refrained from changing since I would change it soon ( but then failed to , which I'll do in a followup PR for all branches today )

@jkakavas jkakavas requested a review from ywangd May 19, 2020 10:17
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

@jkakavas jkakavas merged commit 9ae9bdc into elastic:master May 19, 2020
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request May 19, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is
throwing(AEADBadTagException). Other implementations from other
Security Providers fail with a different exception and as such we
cannot differentiate between a corrupted file and a wrong password
in a foolproof way.
As in other tests such as in
KeyStoreWrapperTests#testDecryptKeyStoreWithWrongPassword
we handle this by matching both possible exception messages.
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request May 19, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is
throwing(AEADBadTagException). Other implementations from other
Security Providers fail with a different exception and as such we
cannot differentiate between a corrupted file and a wrong password
in a foolproof way.
As in other tests such as in
KeyStoreWrapperTests#testDecryptKeyStoreWithWrongPassword
we handle this by matching both possible exception messages.
jkakavas added a commit that referenced this pull request May 19, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is
throwing(AEADBadTagException). Other implementations from other
Security Providers fail with a different exception and as such we
cannot differentiate between a corrupted file and a wrong password
in a foolproof way.
As in other tests such as in
KeyStoreWrapperTests#testDecryptKeyStoreWithWrongPassword
we handle this by matching both possible exception messages.
jkakavas added a commit that referenced this pull request May 19, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is
throwing(AEADBadTagException). Other implementations from other
Security Providers fail with a different exception and as such we
cannot differentiate between a corrupted file and a wrong password
in a foolproof way.
As in other tests such as in
KeyStoreWrapperTests#testDecryptKeyStoreWithWrongPassword
we handle this by matching both possible exception messages.
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request May 21, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers fail with a different exception and as such we cannot
differentiate between a corrupted file and a wrong passwordin a
foolproof way.
we handle this in this test by matching both possible
exception messages.

Relates: elastic#56889
jkakavas added a commit that referenced this pull request May 26, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers might cause decryption to fail in a different way and cause
us to throw a generic error message.
We handle this in this test by matching both possible
exception messages.

Relates: #56889
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request May 26, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers might cause decryption to fail in a different way and cause
us to throw a generic error message.
We handle this in this test by matching both possible
exception messages.

Relates: elastic#56889
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request May 26, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers might cause decryption to fail in a different way and cause
us to throw a generic error message.
We handle this in this test by matching both possible
exception messages.

Relates: elastic#56889
jkakavas added a commit to jkakavas/elasticsearch that referenced this pull request May 26, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers might cause decryption to fail in a different way and cause
us to throw a generic error message.
We handle this in this test by matching both possible
exception messages.

Relates: elastic#56889
jkakavas added a commit that referenced this pull request May 26, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers might cause decryption to fail in a different way and cause
us to throw a generic error message.
We handle this in this test by matching both possible
exception messages.

Relates: #56889
jkakavas added a commit that referenced this pull request May 26, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers might cause decryption to fail in a different way and cause
us to throw a generic error message.
We handle this in this test by matching both possible
exception messages.

Relates: #56889
jkakavas added a commit that referenced this pull request May 26, 2020
In KeystoreWrapper class we determine if the error to decrypt a
given keystore is caused by a wrong password based on the exception
that the SunJCE implementation of AES is throwing
(AEADBadTagException). Other implementations from other Security
Providers might cause decryption to fail in a different way and cause
us to throw a generic error message.
We handle this in this test by matching both possible
exception messages.

Relates: #56889
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Security Security issues without another label Team:Security Meta label for security team >test-failure Triaged test failures from CI v7.7.1 v7.8.1 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants