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

[7.11] [actions] for simplistic email servers, set rejectUnauthorized to false (#91760) #93133

Merged
merged 1 commit into from
Mar 1, 2021

Conversation

pmuellr
Copy link
Member

@pmuellr pmuellr commented Mar 1, 2021

Backports the following commits to 7.11:

…se (elastic#91760)

resolves elastic#91686

The poor email action has not had great success in setting TLS options
correctly.  Prior to 7.11, it was basically always setting `rejectUnauthorized`
to false, so was never validating certificates.  Starting in 7.11.0, it
started respecting TLS certificates, but there are some simple/test servers
in use that use self-signed certificates.

The real fix for this will be the resolution of issue
elastic#80120 , but until then, this PR
does a special-case check if the `secure` option is off (so the email client
connects with a plain socket and then upgrades to TLS via STARTTLS) and both
the user and password for the server are not set, then it will use
`rejectUnauthorized: false`.  Otherwise, it uses the global configured value
of this setting.

This also changes some other cases, where `secure: true` often did not
set any `rejectUnauthorized` property at all, and so did not get verified.
Now in all cases, `rejectUnauthorized` will be set, and the value will
correspond to the globally configured value, except for the special case
checked here, and when a proxy is in use (that logic did not change).

So it is possible this would break customers, who were using insecure servers
and email action worked, but with this fix the connections will be rejected.
They should have been rejected all this time though.

The work-around for this problem, if we don't implement a fix like this, is
that customers will need to set the global `rejectUnauthorized` to `false`,
which means NONE of their TLS connections for any actions will be verified.
Which seems extreme.
@pmuellr pmuellr enabled auto-merge (squash) March 1, 2021 21:33
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
triggersActionsUi 1.5MB 1.5MB -26.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
triggersActionsUi 165.1KB 165.2KB +102.0B
Unknown metric groups

async chunk count

id before after diff
triggersActionsUi 31 32 +1

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@pmuellr pmuellr merged commit 9cbbe9c into elastic:7.11 Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants