Skip to content

Conversation

@jkakavas
Copy link
Contributor

The Bouncy Castle FIPS provider that we use for running our tests
in fips mode has an issue with locale sensitive handling of Dates as
described in bcgit/bc-java#405

This causes certificate validation to fail if any given test that
includes some form of certificate validation happens to run in one
of the locales. This manifested earlier in #33081 which was
handled insufficiently in #33299

This change ensures that the problematic 3 locales

  • th-TH
  • ja-JP-u-ca-japanese-x-lvariant-JP
  • th-TH-u-nu-thai-x-lvariant-TH

will not be used when running our tests in a FIPS 140 JVM. It also
reverts #33299

The Bouncy Castle FIPS provider that we use for running our tests
in fips mode has an issue with locale sensitive handling of Dates as
described in bcgit/bc-java#405

This causes certificate validation to fail if any given test that
includes some form of certificate validation happens to run in one
of the locales. This manifested earlier in elastic#33081 which was
handled insufficiently in elastic#33299

This change ensures that the problematic 3 locales

* th-TH
* ja-JP-u-ca-japanese-x-lvariant-JP
* th-TH-u-nu-thai-x-lvariant-TH

will not be used when running our tests in a FIPS 140 JVM. It also
reverts elastic#33299
@jkakavas jkakavas added >test Issues or PRs that are addressing/adding tests :Delivery/Build Build or test infrastructure v7.0.0 v6.7.0 v8.0.0 labels Feb 15, 2019
@jkakavas jkakavas requested a review from jasontedor February 15, 2019 09:52
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Requests.INDEX_CONTENT_TYPE = XContentType.JSON;
}

@BeforeClass
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It feels a little too much of a blanket approach to do this in ESTestCase, but failing tests could be anywhere from classes implementing ESRestTestCase, or security related ones implementing ESTestCase. Doing this here, also protects us from future tests inadvertently causing this. I'm definitely open to suggestions to limit the scope if necessary but given the fact that the upstream bug makes using BCFipsProvider unusable in these 3 locales for users when dealing with certificates, I believe it's ok to not run the tests using these.

if (isUnusableLocale()) {
// See: https://github.com/bcgit/bc-java/issues/405
Logger logger = LogManager.getLogger(ESTestCase.class);
logger.warn("Attempting to run tests in an unusable locale in a FIPS JVM. Certificate expiration validation will fail, " +
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would be good to provide a link to the upstream bug so it's easier to check if this is still relevant ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the log message you mean ? Sure thing

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, move it to the log message rather than the comment.

Copy link
Contributor

@alpar-t alpar-t 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
Copy link
Contributor Author

If one of you could LGTM the change in 59ee5cb before I merge, that would be great. It basically stops restoring to the offending locale each time

@alpar-t
Copy link
Contributor

alpar-t commented Feb 19, 2019

still LGTM

@jkakavas jkakavas merged commit dc02278 into elastic:master Feb 19, 2019
jkakavas added a commit that referenced this pull request Feb 19, 2019
* Disable specific locales for tests in fips mode

The Bouncy Castle FIPS provider that we use for running our tests
in fips mode has an issue with locale sensitive handling of Dates as
described in bcgit/bc-java#405

This causes certificate validation to fail if any given test that
includes some form of certificate validation happens to run in one
of the locales. This manifested earlier in #33081 which was
handled insufficiently in #33299

This change ensures that the problematic 3 locales

* th-TH
* ja-JP-u-ca-japanese-x-lvariant-JP
* th-TH-u-nu-thai-x-lvariant-TH

will not be used when running our tests in a FIPS 140 JVM. It also
reverts #33299
jkakavas added a commit that referenced this pull request Feb 19, 2019
* Disable specific locales for tests in fips mode

The Bouncy Castle FIPS provider that we use for running our tests
in fips mode has an issue with locale sensitive handling of Dates as
described in bcgit/bc-java#405

This causes certificate validation to fail if any given test that
includes some form of certificate validation happens to run in one
of the locales. This manifested earlier in #33081 which was
handled insufficiently in #33299

This change ensures that the problematic 3 locales

* th-TH
* ja-JP-u-ca-japanese-x-lvariant-JP
* th-TH-u-nu-thai-x-lvariant-TH

will not be used when running our tests in a FIPS 140 JVM. It also
reverts #33299
jkakavas added a commit that referenced this pull request Feb 19, 2019
* Disable specific locales for tests in fips mode

The Bouncy Castle FIPS provider that we use for running our tests
in fips mode has an issue with locale sensitive handling of Dates as
described in bcgit/bc-java#405

This causes certificate validation to fail if any given test that
includes some form of certificate validation happens to run in one
of the locales. This manifested earlier in #33081 which was
handled insufficiently in #33299

This change ensures that the problematic 3 locales

* th-TH
* ja-JP-u-ca-japanese-x-lvariant-JP
* th-TH-u-nu-thai-x-lvariant-TH

will not be used when running our tests in a FIPS 140 JVM. It also
reverts #33299
@jkakavas jkakavas deleted the forbid-locale-fips branch February 19, 2019 15:44
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v6.7.0 v7.0.0-rc2 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants