Auto Discover Servers: recover from bad configuration #44282
Conversation
| return nil | ||
| } | ||
|
|
||
| func checksum(filename string) (string, error) { | ||
| f, err := utils.OpenFileAllowingUnsafeLinks(filename) |
There was a problem hiding this comment.
I see that tctl already has a similar function, but the usage of utils.OpenFileAllowingUnsafeLinks does raise my eyebrow. Would it be possible to use utils.OpenFileNoUnsafeLinks instead? If not, then I think it'd be a good idea to leave a comment explaining why following symlinks is warranted in this case. I don't have enough context on this code to assess this.
There was a problem hiding this comment.
I'll change it to use utils.OpenFileNoUnsafeLinks.
This happens for two files and those files are written by teleport node configure which creates the files and no symlinks.
Also, we expect no manual intervention in these files because teleport is meant to be installed automatically.
There was a problem hiding this comment.
@ravicious @marcoandredinis is it really necessary to block symlinks here? this breaks the tests on my macbook where /var is a symlink. How do we know /etc won't be a symlink and why should we care?
IIRC the reason to avoid reading files from symlinks is to avoid reading them as root and leaking info about files owned by other users to an unprivileged user who created a malicious symlink
--- FAIL: TestAutoDiscoverNode/gcp_adds_a_label_with_the_project_id (0.01s)
autodiscover_test.go:369:
Error Trace: /Users/nic/src/teleport/lib/srv/server/installer/autodiscover_test.go:369
Error: Received unexpected error:
opening file /var/folders/x_/33kp49n52rg8hs97k5m96rnw0000gn/T/TestAutoDiscoverNodegcp_adds_a_label_with_the_project_id462103172/001/etc/teleport.yaml, symlink not allowed in path: /var
Test: TestAutoDiscoverNode/gcp_adds_a_label_with_the_project_id
There was a problem hiding this comment.
I'll change it back to utils.OpenFileAllowingUnsafeLinks
#46600
marcoandredinis
force-pushed
the
marco/autodiscoverserver_recover_badconfig
branch
2 times, most recently
from
3b02d29 to
5a041ab
Compare
marcoandredinis
force-pushed
the
marco/autodiscoverserver_recover_badconfig
branch
from
5a041ab to
f3be074
Compare
|
@marcoandredinis See the table below for backport results.
|
The auto discover flow will run a command for all the target servers.
If the server joins teleport, then the instance is skipped.
When running the auto discover on the target instance, if teleport is already installed, the installation procedure exits.
It might happen that server cannot join the cluster because it was configured to join with an invalid token (non existent, wrong roles, ...).
Even if the user fixes the Matcher's installation params, the new configuration won't be applied, because, as said above, the procedure exits.
This PR changes the installation procedure use to auto-enroll servers. If the target configuration is different than the existing one, then teleport server is restarted.
If it stays the same, then it will no restart. This is to prevent unnecessary restarts of the service.
Demo after fixing the token
Context: #37620
changelog: Re-create teleport configuration when the node fails to join the cluster during Server Auto Discover.