Features: Allow the TCP Input to receive events over a TLS connection #7056
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a valid error, current guest is don't write enough stuff to the socket to trigger the error and the message are in the buffer. |
kvch
reviewed
May 11, 2018
| CAs []string `config:"certificate_authorities"` | ||
| Certificate CertificateConfig `config:",inline"` | ||
| CurveTypes []tlsCurveType `config:"curve_types"` | ||
| ClientAuth tlsClientAuth `config:"client_authentification"` //`none`, `optional` or `required` |
There was a problem hiding this comment.
Could you rename it to client_authentication, so non-native English speakers like me don't need to check the dictionary? :D Also, according to English on StackExchange it's less idiomatic. :D https://english.stackexchange.com/questions/7844/is-authentification-a-real-word
kvch
reviewed
May 11, 2018
| @@ -260,7 +260,7 @@ def test_tcp_tls_with_a_plain_text_socket(self): | |||
| # The TLS handshake will close the connection, resulting in a broken pipe. | |||
| # no events should be written on disk. | |||
| with assert_raises(IOError): | |||
| for n in range(0, 100): | |||
| for n in range(0, 100000): | |||
There was a problem hiding this comment.
Why did you increase this number? How much time does it add to test execution time?
There was a problem hiding this comment.
Oh, it's outdated. Nevermind.
kvch
suggested changes
May 11, 2018
|
@kvch it should have been client_authentication, my brain is wired in
French for that word.
On Fri, May 11, 2018 at 6:33 AM Noémi Ványi ***@***.***> wrote:
***@***.**** requested changes on this pull request.
It's a nice refactor. However, the name of the new option should be
changed.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#7056 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAACgGSvTX-Uzt0ZPMBZYAipIYpwAoH7ks5txWjvgaJpZM4T4h0W>
.
--
ph
|
|
@kvch I will make the change in https://github.com/elastic/beats/pull/7055/files#diff-4742d75c33d67fa2d2390d1854041621R18 instead of this PR. |
ph
force-pushed
the
feature/add-tls-to-tcp
branch
from
0925e3d to
ff2e7cc
Compare
|
I have added the in progress label the failures on windows are a bit concerning. |
ph
force-pushed
the
feature/add-tls-to-tcp
branch
from
c53731d to
8cf0c38
Compare
|
@kvch Updated with latest change from the original TLS PR |
kvch
approved these changes
May 17, 2018
|
Could you rebase this branch, so moving stuff to |
ph
force-pushed
the
feature/add-tls-to-tcp
branch
from
8cf0c38 to
0996973
Compare
|
@kvch I've rebased the PR and its ready to get merged. :) |
We can now receive events on the TCP input with a TLS connection, the input uses existing type to make sure we have the same naming convention and code used by outputs that support TLS communication (Elasticsearch and Logstash). The configuration will look like this: ``` host: "localhost:9000" ssl.enabled: true ssl.verification_mode: full # default ssl.supported_protocols: [TLSv1.1] ssl.cipher_suites: [] ssl.certificate_authorities: ["/etc/cacert"] ssl.certificate: /etc/mycert.crt ssl.key: /etc/mycert.key ssl.client_authentication: required ``` One added configuration is `client_authentication`, this option is only used in the context of server and define how we will force the authentification, it support the three following options: - `required`: Assume that the client will provide a certificate and we will verify it. (default) - `optional`: If a certificate is given by the client. - `none`: We don't validate client certificate. *Note: This commit contains a script to generate certs from a self signed CA. Fixes: elastic#6873
ph
force-pushed
the
feature/add-tls-to-tcp
branch
from
0996973 to
d686b9b
Compare
|
jenkins test this please |
stevea78
pushed a commit
to stevea78/beats
that referenced
this pull request
May 20, 2018
…elastic#7056) We can now receive events on the TCP input with a TLS connection, the input uses existing type to make sure we have the same naming convention and code used by outputs that support TLS communication (Elasticsearch and Logstash). The configuration will look like this: ``` host: "localhost:9000" ssl.enabled: true ssl.verification_mode: full # default ssl.supported_protocols: [TLSv1.1] ssl.cipher_suites: [] ssl.certificate_authorities: ["/etc/cacert"] ssl.certificate: /etc/mycert.crt ssl.key: /etc/mycert.key ssl.client_authentication: required ``` One added configuration is `client_authentication`, this option is only used in the context of server and define how we will force the authentification, it support the three following options: - `required`: Assume that the client will provide a certificate and we will verify it. (default) - `optional`: If a certificate is given by the client. - `none`: We don't validate client certificate. *Note: This commit contains a script to generate certs from a self signed CA. Fixes: elastic#6873
stevea78
pushed a commit
to stevea78/beats
that referenced
this pull request
May 20, 2018
…elastic#7056) We can now receive events on the TCP input with a TLS connection, the input uses existing type to make sure we have the same naming convention and code used by outputs that support TLS communication (Elasticsearch and Logstash). The configuration will look like this: ``` host: "localhost:9000" ssl.enabled: true ssl.verification_mode: full # default ssl.supported_protocols: [TLSv1.1] ssl.cipher_suites: [] ssl.certificate_authorities: ["/etc/cacert"] ssl.certificate: /etc/mycert.crt ssl.key: /etc/mycert.key ssl.client_authentication: required ``` One added configuration is `client_authentication`, this option is only used in the context of server and define how we will force the authentification, it support the three following options: - `required`: Assume that the client will provide a certificate and we will verify it. (default) - `optional`: If a certificate is given by the client. - `none`: We don't validate client certificate. *Note: This commit contains a script to generate certs from a self signed CA. Fixes: elastic#6873
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We can now receive events on the TCP input with a TLS connection, the
input uses existing type to make sure we have the same naming convention
and code used by outputs that support TLS communication (Elasticsearch
and Logstash).
The configuration will look like this:
One added configuration is
client_authentication, this option isonly used in the context of server and define how we will force the
authentification, it support the three following options:
required: Assume that the client will provide a certificate and wewill verify it. (default)
optional: If a certificate is given by the client.none: We don't validate client certificate.*Note: This commit contains a script to generate certs from a self
signed CA.
Fixes: #6873
Depends on #7054