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

TLS not working with go 1.15 #20

Closed
simitt opened this issue Dec 3, 2020 · 13 comments
Closed

TLS not working with go 1.15 #20

simitt opened this issue Dec 3, 2020 · 13 comments
Labels
bug Something isn't working

Comments

@simitt
Copy link

simitt commented Dec 3, 2020

Summary

Compiling e.g. APM Server with go 1.15 and trying to run it via elastic agent results in following error:

{"level":"error","timestamp":"2020-12-03T10:36:56.840+0100","logger":"centralmgmt.fleet","caller":"fleet/manager.go:187","message":"elastic-agent-client got error: rpc error: code = Unavailable desc = connection error: desc = \"transport: authentication handshake failed: x509: certificate relies on legacy Common Name field, use SANs or temporarily enable Common Name matching with GODEBUG=x509ignoreCN=0\""}

The TLS related code is the same accross beats.

Related golang issues: https://github.com/golang/go/issues/39568 and https://github.com/golang/go/issues/40748

How to reproduce

  • run go test ./pkg/client with go 1.15 leads to a timeout
    OR
  • compile e.g. APM Server with go 1.15 and try running it under elastic agent (currently only possible with manual plumbing) results in above mentioned error
@ph ph added the bug Something isn't working label Dec 3, 2020
@ph
Copy link
Contributor

ph commented Dec 3, 2020

@blakerouse Can you take a look at this, master is not on 1.15 yet, I think it's planned for 1.15. cc @urso interesting to know.

@ph ph added the v7.12.0 label Dec 3, 2020
@urso
Copy link

urso commented Dec 3, 2020

go1.15 changes Certificate validation. @kvch is already looking into updating master to go1.15. The plan is to reenable host checking via Common Field if possible.

For agent or whoever generates the certificates, should use SANs. Even before go 1.15 comes

@ph
Copy link
Contributor

ph commented Dec 3, 2020

@urso Can you confirm that 1.15 is planned in 7.12?

@urso
Copy link

urso commented Dec 3, 2020

@urso Can you confirm that 1.15 is planned in 7.12?

Actually we were thinking 7.11

@ph
Copy link
Contributor

ph commented Dec 3, 2020

@urso Can we hold that one? it's close on the FF? cc @andresrc

@urso
Copy link

urso commented Dec 3, 2020

PR upgrading to go1.15: elastic/beats#22495

Looks like we still have a few issues. Checking the 1.15 release notes it looks like there are potentially a few other gotchas. This one is my favorite (time to scan/update code and dependencies?):

Creating a derived Context using a nil parent is now explicitly disallowed. Any attempt to do so with the WithValue, WithDeadline, or WithCancel functions will cause a panic.

I don't mind if we postpone this after until FF.

@kvch
Copy link

kvch commented Dec 3, 2020

@urso I checked our code and dependencies for possible context-related issues, but I haven't found anything which would cause problems. I checked in our deps with go mod vendor && git add vendor locally, and it did not show any incorrect context usage.

@ph
Copy link
Contributor

ph commented Dec 3, 2020

@urso @kvch If we can postpone the update and do it 7.12, I would appreciate it. I want to make sure we don't introduce issue like this with the basic communication channel.

@urso
Copy link

urso commented Dec 8, 2020

If we can postpone the update and do it 7.12, I would appreciate it. I want to make sure we don't introduce issue like this with the basic communication channel.

Yeah, I'm fine with that.

@michalpristas
Copy link
Contributor

as for elastic agent this affects IPC and agent to fleet communication.
IPC fix is easy and already targeted in PR elastic/beats#22495
other use cases agent to fleet or beat to output is handled using that PR as well and as soon as it defaults to backward compatible way there's nothing we should do agent side.

we could do two things here:

@ph
Copy link
Contributor

ph commented Jan 19, 2021

I am +1 to move to strict for the IPC, we can create the PR and sync with @ferullo's team for merging it.

@ph ph removed the v7.12.0 label Feb 2, 2021
@ph
Copy link
Contributor

ph commented Feb 2, 2021

Removed from iteration, with the workaround in go 1.15.7, we think its sufficient.

@ph
Copy link
Contributor

ph commented Feb 19, 2021

fixed by elastic/beats#22495

@ph ph closed this as completed Feb 19, 2021
v1v pushed a commit to v1v/elastic-agent-client that referenced this issue Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants