Skip to content

[TEST] Make SSL restrictions update atomic#31050

Merged
tvernum merged 6 commits intoelastic:masterfrom
tvernum:fix/29989-trust-restric-tests
Jun 7, 2018
Merged

[TEST] Make SSL restrictions update atomic#31050
tvernum merged 6 commits intoelastic:masterfrom
tvernum:fix/29989-trust-restric-tests

Conversation

@tvernum
Copy link
Copy Markdown
Contributor

@tvernum tvernum commented Jun 4, 2018

SSLTrustRestrictionsTests updates the restrictions YML file during the test run to change the set of restrictions. This update was small, but it wasn't atomic.
If the yml file is reloaded while empty or invalid, then it causes all SSL certificates to be considered invalid (until it is reloaded again), which could break the sniffing/administrative client that runs underneath the tests.

Relates: #29989

@tvernum tvernum requested a review from jkakavas June 4, 2018 00:39
@tvernum tvernum added >test Issues or PRs that are addressing/adding tests v7.0.0 v6.3.0 v6.2.5 :Security/TLS SSL/TLS, Certificates v6.4.0 labels Jun 4, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security

@tvernum tvernum added the review label Jun 4, 2018
try {
Files.write(restrictionsPath, Collections.singleton("trust.subject_name: \"" + trustedPattern + "\""));
Files.write(restrictionsTmpPath, Collections.singleton("trust.subject_name: \"" + trustedPattern + "\""));
Files.move(restrictionsTmpPath, restrictionsPath, StandardCopyOption.ATOMIC_MOVE);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should add REPLACE_EXISTING and handling of AtomicMoveNotSupportedException. SecurityFiles has code for this.

@tvernum
Copy link
Copy Markdown
Contributor Author

tvernum commented Jun 6, 2018

@jkakavas @jaymode
The PR has been updated and is ready for review.

Copy link
Copy Markdown
Contributor

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM

@tvernum tvernum merged commit bd3aaba into elastic:master Jun 7, 2018
tvernum added a commit that referenced this pull request Jun 15, 2018
SSLTrustRestrictionsTests updates the restrictions YML file during the test run to change the set of restrictions. This update was small, but it wasn't atomic.
If the yml file is reloaded while empty or invalid, then it causes all SSL certificates to be considered invalid (until it is reloaded again), which could break the sniffing/administrative client that runs underneath the tests.
tlrx added a commit that referenced this pull request Jun 15, 2018
* 6.x:
  6d711fa (origin/6.x, 6.x) Uncouple persistent task state and status (#31031)
  f0f16b7 [TEST] Make SSL restrictions update atomic (#31050)
  652193f Describe how to add a plugin in Dockerfile (#31340)
  bb568ab TEST: getCapturedRequestsAndClear should be atomic (#31312)
  6f94914 Do not set vm.max_map_count when unnecessary (#31285)
  c12f3c7 Painless: Fix bug for static method calls on interfaces (#31348)
  21128e2 QA: Fix resolution of default distribution (#31351)
  df17a83 Build: Fix the license in the pom zip and tar (#31336)
  3e76b15 Treat ack timeout more like a publish timeout (#31303)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Security/TLS SSL/TLS, Certificates >test Issues or PRs that are addressing/adding tests v6.4.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants