Skip to content

Commit

Permalink
fix tls communication with lapi and user/pw auth (backport) (#1955)
Browse files Browse the repository at this point in the history
allow self-signed TLS encryption with user/pw auth

docker:
 - remove defaults for certificate file locations
 - new envvar INSECURE_SKIP_VERIFY
 - register agent before TLS settings (cscli machine add removes them
   from the credentials file)
  • Loading branch information
mmetc authored Dec 29, 2022
1 parent 6ef4217 commit 1b28792
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 46 deletions.
6 changes: 3 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,17 @@ ENV AGENT_PASSWORD=
# TLS setup ----------------------------------- #

ENV USE_TLS=false
ENV INSECURE_SKIP_VERIFY=

ENV CACERT_FILE=

ENV LAPI_CACERT_FILE=
ENV LAPI_CERT_FILE=
ENV LAPI_KEY_FILE=

ENV CLIENT_CACERT_FILE=
ENV CLIENT_CERT_FILE=
ENV CLIENT_KEY_FILE=

# deprecated in favor of LAPI_*
ENV CACERT_FILE=
ENV CERT_FILE=
ENV KEY_FILE=

Expand Down
6 changes: 3 additions & 3 deletions Dockerfile.debian
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,17 @@ ENV AGENT_PASSWORD=
# TLS setup ----------------------------------- #

ENV USE_TLS=false
ENV INSECURE_SKIP_VERIFY=

ENV CACERT_FILE=

ENV LAPI_CACERT_FILE=
ENV LAPI_CERT_FILE=
ENV LAPI_KEY_FILE=

ENV CLIENT_CACERT_FILE=
ENV CLIENT_CERT_FILE=
ENV CLIENT_KEY_FILE=

# deprecated in favor of LAPI_*
ENV CACERT_FILE=
ENV CERT_FILE=
ENV KEY_FILE=

Expand Down
12 changes: 6 additions & 6 deletions docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,12 @@ Using binds rather than named volumes ([complete explanation here](https://docs.
| | | |
| __TLS Auth/encryption | | |
| `USE_TLS` | false | Enable TLS on the LAPI |
| `CLIENT_CERT_FILE` | /etc/ssl/cert.pem | Client TLS Certificate path |
| `CLIENT_KEY_FILE` | /etc/ssl/key.pem | Client TLS Key path |
| `CLIENT_CACERT_FILE` | | Client CA certificate bundle |
| `LAPI_CERT_FILE` | /etc/ssl/cert.pem | LAPI TLS Certificate path |
| `LAPI_KEY_FILE` | /etc/ssl/key.pem | LAPI TLS Key path |
| `LAPI_CACERT_FILE` | | LAPI CA certificate bundle |
| `CACERT_FILE` | | CA certificate bundle (optional) |
| `INSECURE_SKIP_VERIFY` | | Skip LAPI certificate validation |
| `CLIENT_CERT_FILE` | | Client TLS Certificate path (enables TLS auth) |
| `CLIENT_KEY_FILE` | | Client TLS Key path |
| `LAPI_CERT_FILE` | | LAPI TLS Certificate path (required if USE_TLS) |
| `LAPI_KEY_FILE` | | LAPI TLS Key path (required if USE_TLS) |
| `AGENTS_ALLOWED_OU` | agent-ou | OU values allowed for agents, separated by comma |
| `BOUNCERS_ALLOWED_OU` | bouncer-ou | OU values allowed for bouncers, separated by comma |
| | | |
Expand Down
46 changes: 22 additions & 24 deletions docker/docker_start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,14 @@ cscli_if_clean() {

#-----------------------------------#

if [ -n "$CERT_FILE" ] || [ -n "$KEY_FILE" ] || [ -n "$CACERT_FILE" ]; then
if [ -n "$CERT_FILE" ] || [ -n "$KEY_FILE" ] ; then
printf '%b' '\033[0;33m'
echo "Warning: the variables CERT_FILE, KEY_FILE and CACERT_FILE have been deprecated." >&2
echo "Please use LAPI_CERT_FILE, LAPI_KEY_FILE and LAPI_CACERT_FILE insted." >&2
echo "Warning: the variables CERT_FILE and KEY_FILE have been deprecated." >&2
echo "Please use LAPI_CERT_FILE and LAPI_KEY_FILE insted." >&2
echo "The old variables will be removed in a future release." >&2
printf '%b' '\033[0m'
LAPI_CERT_FILE=${LAPI_CERT_FILE:-$CERT_FILE}
LAPI_KEY_FILE=${LAPI_KEY_FILE:-$KEY_FILE}
LAPI_CACERT_FILE=${LAPI_CACERT_FILE:-$CACERT_FILE}
fi

# Check and prestage databases
Expand Down Expand Up @@ -141,8 +140,24 @@ if isfalse "$DISABLE_LOCAL_API" && isfalse "$USE_TLS"; then
cscli machines add "$CUSTOM_HOSTNAME" --auto --url "$LOCAL_API_URL"
fi

if isfalse "$DISABLE_LOCAL_API"; then
echo "Check if lapi needs to automatically register an agent"

# pre-registration is not needed with TLS authentication, but we can have TLS transport with user/pw
if [ "$AGENT_USERNAME" != "" ] && [ "$AGENT_PASSWORD" != "" ] ; then
# re-register because pw may have been changed
# doing this generates local_api_credentials.yaml from scratch, removing any tls setting
cscli machines add "$AGENT_USERNAME" --password "$AGENT_PASSWORD" --url "$LOCAL_API_URL" --force
echo "Agent registered to lapi"
elif isfalse "$USE_TLS"; then
echo "TLS is disabled, the AGENT_USERNAME and AGENT_PASSWORD must be set" >&2
fi
fi

lapi_credentials_path=$(conf_get '.api.client.credentials_path')

conf_set 'with(select(strenv(INSECURE_SKIP_VERIFY)!=""); .api.client.insecure_skip_verify = env(INSECURE_SKIP_VERIFY))'

# we only use the envvars that are actually defined
# in case of persistent configuration
conf_set '
Expand All @@ -153,12 +168,9 @@ conf_set '

if istrue "$USE_TLS"; then
conf_set '
with(select(strenv(CLIENT_CACERT_FILE)!=""); .ca_cert_path = strenv(CLIENT_CACERT_FILE)) |
with(select(.ca_cert_path=="" or .ca_cert_path==null); .ca_cert_path = "/etc/ssl/crowdsec-client/ca.pem") |
with(select(strenv(CACERT_FILE)!=""); .ca_cert_path = strenv(CACERT_FILE)) |
with(select(strenv(CLIENT_KEY_FILE)!=""); .key_path = strenv(CLIENT_KEY_FILE)) |
with(select(.key_path=="" or .key_path==null); .key_path = "/etc/ssl/crowdsec-client/key.pem") |
with(select(strenv(CLIENT_CERT_FILE)!=""); .cert_path = strenv(CLIENT_CERT_FILE)) |
with(select(.cert_path=="" or .cert_path==null); .cert_path = "/etc/ssl/crowdsec-client/cert.pem")
with(select(strenv(CLIENT_CERT_FILE)!=""); .cert_path = strenv(CLIENT_CERT_FILE))
' "$lapi_credentials_path"
else
conf_set '
Expand All @@ -168,17 +180,6 @@ else
' "$lapi_credentials_path"
fi

if isfalse "$DISABLE_LOCAL_API"; then
echo "Check if lapi needs to automatically register an agent"

# pre-registration is not needed with TLS
if isfalse "$USE_TLS" && [ "$AGENT_USERNAME" != "" ] && [ "$AGENT_PASSWORD" != "" ] ; then
# re-register because pw may have been changed
cscli machines add "$AGENT_USERNAME" --password "$AGENT_PASSWORD" --url "$LOCAL_API_URL" --force
echo "Agent registered to lapi"
fi
fi

# registration to online API for signal push
if isfalse "$DISABLE_ONLINE_API" && [ "$CONFIG_FILE" == "/etc/crowdsec/config.yaml" ] ; then
config_exists=$(conf_get '.api.server.online_client | has("credentials_path")')
Expand Down Expand Up @@ -217,12 +218,9 @@ if istrue "$USE_TLS"; then
agents_allowed_yaml=$(csv2yaml "$AGENTS_ALLOWED_OU") \
bouncers_allowed_yaml=$(csv2yaml "$BOUNCERS_ALLOWED_OU") \
conf_set '
with(select(strenv(LAPI_CACERT_FILE)!=""); .api.server.tls.ca_cert_path = strenv(LAPI_CACERT_FILE)) |
with(select(.api.server.tls.ca_cert_path=="" or .api.server.tls.ca_cert_path==null); .api.server.tls.ca_cert_path = "/etc/ssl/crowdsec-lapi/ca.pem") |
with(select(strenv(CACERT_FILE)!=""); .api.server.tls.ca_cert_path = strenv(CACERT_FILE)) |
with(select(strenv(LAPI_CERT_FILE)!=""); .api.server.tls.cert_file = strenv(LAPI_CERT_FILE)) |
with(select(.api.server.tls.cert_file=="" or .api.server.tls.cert_file==null); .api.server.tls.cert_file = "/etc/ssl/crowdsec-lapi/cert.pem") |
with(select(strenv(LAPI_KEY_FILE)!=""); .api.server.tls.key_file = strenv(LAPI_KEY_FILE)) |
with(select(.api.server.tls.key_file=="" or .api.server.tls.key_file==null); .api.server.tls.key_file = "/etc/ssl/crowdsec-lapi/key.pem") |
with(select(strenv(BOUNCERS_ALLOWED_OU)!=""); .api.server.tls.bouncers_allowed_ou = env(bouncers_allowed_yaml)) |
with(select(strenv(AGENTS_ALLOWED_OU)!=""); .api.server.tls.agents_allowed_ou = env(agents_allowed_yaml)) |
... comments=""
Expand Down
4 changes: 2 additions & 2 deletions pkg/apiclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ func NewClient(config *Config) (*ApiClient, error) {
UpdateScenario: config.UpdateScenario,
}
tlsconfig := tls.Config{InsecureSkipVerify: InsecureSkipVerify}
tlsconfig.RootCAs = CaCertPool
if Cert != nil {
tlsconfig.RootCAs = CaCertPool
tlsconfig.Certificates = []tls.Certificate{*Cert}
}
http.DefaultTransport.(*http.Transport).TLSClientConfig = &tlsconfig
Expand All @@ -75,8 +75,8 @@ func NewDefaultClient(URL *url.URL, prefix string, userAgent string, client *htt
client = &http.Client{}
if ht, ok := http.DefaultTransport.(*http.Transport); ok {
tlsconfig := tls.Config{InsecureSkipVerify: InsecureSkipVerify}
tlsconfig.RootCAs = CaCertPool
if Cert != nil {
tlsconfig.RootCAs = CaCertPool
tlsconfig.Certificates = []tls.Certificate{*Cert}
}
ht.TLSClientConfig = &tlsconfig
Expand Down
19 changes: 11 additions & 8 deletions pkg/csconfig/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (l *LocalApiClientCfg) Load() error {
}
}

if l.Credentials.Login != "" && (l.Credentials.CACertPath != "" || l.Credentials.CertPath != "" || l.Credentials.KeyPath != "") {
if l.Credentials.Login != "" && (l.Credentials.CertPath != "" || l.Credentials.KeyPath != "") {
return fmt.Errorf("user/password authentication and TLS authentication are mutually exclusive")
}

Expand All @@ -91,23 +91,26 @@ func (l *LocalApiClientCfg) Load() error {
apiclient.InsecureSkipVerify = *l.InsecureSkipVerify
}

if l.Credentials.CACertPath != "" && l.Credentials.CertPath != "" && l.Credentials.KeyPath != "" {
cert, err := tls.LoadX509KeyPair(l.Credentials.CertPath, l.Credentials.KeyPath)
if err != nil {
return errors.Wrapf(err, "failed to load api client certificate")
}

if l.Credentials.CACertPath != "" {
caCert, err := os.ReadFile(l.Credentials.CACertPath)
if err != nil {
return errors.Wrapf(err, "failed to load cacert")
}

caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caCert)
apiclient.CaCertPool = caCertPool
}

if l.Credentials.CertPath != "" && l.Credentials.KeyPath != "" {
cert, err := tls.LoadX509KeyPair(l.Credentials.CertPath, l.Credentials.KeyPath)
if err != nil {
return errors.Wrapf(err, "failed to load api client certificate")
}

apiclient.Cert = &cert
apiclient.CaCertPool = caCertPool
}

return nil
}

Expand Down

0 comments on commit 1b28792

Please sign in to comment.