Commit f1300da5 authored by Kamil Trzciński's avatar Kamil Trzciński

Remove TLSSkipVerify, we shouldn't advise bad practice

parent 1ef82ea7
Pipeline #373393 failed with stage
......@@ -40,10 +40,9 @@ type ParallelsConfig struct {
}
type RunnerCredentials struct {
URL string `toml:"url" json:"url" short:"u" long:"url" env:"CI_SERVER_URL" required:"true" description:"Runner URL"`
Token string `toml:"token" json:"token" short:"t" long:"token" env:"CI_SERVER_TOKEN" required:"true" description:"Runner token"`
TLSSkipVerify bool `toml:"tls-skip-verify" json:"tls-skip-verify" long:"tls-skip-verify" env:"CI_SERVER_TLS_SKIP_VERIFY" description:"Whether to verify the TLS certificate when using HTTPS (INSECURE)"`
TLSCAFile string `toml:"tls-ca-file" json:"tls-ca-file" long:"tls-ca-file" env:"CI_SERVER_TLS_CA_FILE" description:"File containing the certificates to verify the peer when using HTTPS"`
URL string `toml:"url" json:"url" short:"u" long:"url" env:"CI_SERVER_URL" required:"true" description:"Runner URL"`
Token string `toml:"token" json:"token" short:"t" long:"token" env:"CI_SERVER_TOKEN" required:"true" description:"Runner token"`
TLSCAFile string `toml:"tls-ca-file" json:"tls-ca-file" long:"tls-ca-file" env:"CI_SERVER_TLS_CA_FILE" description:"File containing the certificates to verify the peer when using HTTPS"`
}
type RunnerSettings struct {
......
......@@ -13,7 +13,7 @@ The GitLab Runner provides these options:
- `/etc/gitlab-runner/certs/hostname.crt` on *nix systems when gitlab-runner is executed as root.
- `~/.gitlab-runner/certs/hostname.crt` on *nix systems when gitlab-runner is executed as non-root,
- `./certs/hostname.crt` on other systems.
If address of your server is: `https://my.gitlab.server.com:8443/`.
Create the certificate file at: `/etc/gitlab-runner/certs/my.gitlab.server.com`.
......@@ -21,27 +21,9 @@ The GitLab Runner provides these options:
which allows you to specify custom file with certificates. This file will be read everytime when runner tries to
access the GitLab server.
4. GitLab Runner exposes `tls-skip-verify` option during registration and [`config.toml`](advanced-configuration.md)
which allows you to skip TLS verification when connecting to server.
**This approach is INSECURE! Use at your own risk!**
Anyone can eavesdrop your connection:
- see the runner token which is used to authenticate against GitLab,
- see tokens which are used to clone GitLab projects,
- see the secure variables that are passed to runner.
### Git cloning
Currently the certificates are only used to verify connections between GitLab Runner and GitLab server.
This doesn't affect git commands (ie. `git clone`).
You still will see build errors due to TLS validation failure.
To have maximum security for your builds you should configure `GIT_SSL_CAINFO` variable.
It allows you to define the file containing the certificates to verify the peer with when fetching or pushing over HTTPS
The runner injects missing certificates to build CA chain to build containers.
This allows the `git clone` and `artifacts` to work with servers that do not use publicly trusted certificates.
The other options is to disable TLS verification, but as said before this approach is insecure.
Add to your `.gitlab-ci.yml` this lines:
```
variables:
GIT_SSL_NO_VERIFY: "1"
```
This approach is secure, but makes the runner a single point of trust.
......@@ -193,9 +193,8 @@ func newClient(config common.RunnerCredentials) (c *client, err error) {
}
c = &client{
url: url,
skipVerify: config.TLSSkipVerify,
caFile: config.TLSCAFile,
url: url,
caFile: config.TLSCAFile,
}
if CertificateDirectory != "" && c.caFile == "" {
......
......@@ -119,19 +119,6 @@ func TestClientInvalidSSL(t *testing.T) {
assert.Contains(t, statusText, "certificate signed by unknown authority")
}
func TestClientSkipVerify(t *testing.T) {
s := httptest.NewTLSServer(http.HandlerFunc(clientHandler))
defer s.Close()
c, _ := newClient(RunnerCredentials{
URL: s.URL,
TLSSkipVerify: true,
})
statusCode, statusText, certificates := c.do("test/ok", "GET", 200, nil, nil)
assert.Equal(t, 200, statusCode, statusText)
assert.NotNil(t, certificates)
}
func TestClientTLSCAFile(t *testing.T) {
s := httptest.NewTLSServer(http.HandlerFunc(clientHandler))
defer s.Close()
......
......@@ -16,7 +16,7 @@ func (n *GitLabClient) getClient(runner RunnerCredentials) (c *client, err error
if n.clients == nil {
n.clients = make(map[string]*client)
}
key := fmt.Sprintf("%s_%d_%s", runner.URL, runner.TLSSkipVerify, runner.TLSCAFile)
key := fmt.Sprintf("%s_%s", runner.URL, runner.TLSCAFile)
c = n.clients[key]
if c == nil {
c, err = newClient(runner)
......
......@@ -26,10 +26,6 @@ func TestClients(t *testing.T) {
c2, _ := c.getClient(RunnerCredentials{
URL: "http://test2/",
})
c3, _ := c.getClient(RunnerCredentials{
URL: "http://test/",
TLSSkipVerify: true,
})
c4, _ := c.getClient(RunnerCredentials{
URL: "http://test/",
TLSCAFile: "ca_file",
......@@ -40,7 +36,6 @@ func TestClients(t *testing.T) {
})
c6, c6err := c.getClient(brokenCredentials)
assert.NotEqual(t, c1, c2)
assert.NotEqual(t, c1, c3)
assert.NotEqual(t, c1, c4)
assert.Equal(t, c4, c5)
assert.Nil(t, c6)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment