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.

Sign up for GitHub

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

feat(config/server): Add option for TLS and mTLS server #508

Merged
merged 2 commits into from
Jun 14, 2023
Merged

feat(config/server): Add option for TLS and mTLS server #508

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2bfc699
feat(config/server): Add option for TLS and mTLS server
annadorottya Jun 12, 2023
a4d7c82
feat(config/mutualtls): Add separate options for cert files for mutua…
annadorottya Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,13 @@ customfields: # custom fields are added to falco events, if the value starts wit
templatedfields: # templated fields are added to falco events and metrics, it uses Go template + output_fields values
# Dkey: '{{ or (index . "k8s.ns.labels.foo") "bar" }}'
# bracketreplacer: "_" # if not empty, replace the brackets in keys of Output Fields
mutualtlsfilespath: "/etc/certs" # folder which will used to store client.crt, client.key and ca.crt files for mutual tls (default: "/etc/certs")
mutualtlsfilespath: "/etc/certs" # folder which will used to store client.crt, client.key and ca.crt files for mutual tls for outputs (default: "/etc/certs")
tlsserver:
deploy: false # if true, TLS server will be deployed instead of HTTP
certfile: "/etc/certs/server.crt" # server certification file
keyfile: "/etc/certs/server.key" # server key
mutualtls: false # if true, mTLS server will be deployed instead of TLS, deploy also has to be true
cacertfile: "/etc/certs/ca.crt" # for client certification if mutualtls is true

slack:
webhookurl: "" # Slack WebhookURL (ex: https://hooks.slack.com/services/XXXX/YYYY/ZZZZ), if not empty, Slack output is enabled
Expand Down Expand Up @@ -643,6 +649,11 @@ care of lower/uppercases**) : `yaml: a.b --> envvar: A_B` :
- **TEMPLATEDFIELDS** : templated fields are added to falco events and metrics, it uses Go template + output_fields values
- **BRACKETREPLACER** : if not empty, the brackets in keys of Output Fields are replaced
- **MUTUALTLSFILESPATH**: path which will be used to stored certs and key for mutual tls authentication (default: "/etc/certs")
- **TLSSERVER_DEPLOY**: if _true_ TLS server will be deployed instead of HTTP
- **TLSSERVER_CERTFILE**: server certification file for TLS Server (default: "/etc/certs/server.crt")
- **TLSSERVER_KEYFILE**: server key file for TLS Server (default: "/etc/certs/server.key")
- **TLSSERVER_MUTUALTLS**: if _true_ mTLS server will be deployed instead of TLS, deploy also has to be true
- **TLSSERVER_CACERTFILE**: CA certification file for client certification (default: "/etc/certs/ca.crt")
- **SLACK_WEBHOOKURL** : Slack Webhook URL (ex: https://hooks.slack.com/services/XXXX/YYYY/ZZZZ)
- **SLACK_CHANNEL** : Slack Channel (optionnal)
- **SLACK_FOOTER** : Slack footer
Expand Down
6 changes: 6 additions & 0 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ func getConfig() *types.Configuration {
v.SetDefault("BracketReplacer", "")
v.SetDefault("MutualTlsFilesPath", "/etc/certs")

v.SetDefault("TLSServer.Deploy", false)
v.SetDefault("TLSServer.CertFile", "/etc/certs/server.crt")
v.SetDefault("TLSServer.KeyFile", "/etc/certs/server.key")
v.SetDefault("TLSServer.MutualTLS", false)
v.SetDefault("TLSServer.CaCertFile", "/etc/certs/ca.crt")

v.SetDefault("Slack.WebhookURL", "")
v.SetDefault("Slack.Footer", "https://github.com/falcosecurity/falcosidekick")
v.SetDefault("Slack.Username", "Falcosidekick")
Expand Down
8 changes: 7 additions & 1 deletion config_example.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ customfields: # custom fields are added to falco events and metrics, if the valu
templatedfields: # templated fields are added to falco events and metrics, it uses Go template + output_fields values
# Dkey: '{{ or (index . "k8s.ns.labels.foo") "bar" }}'
# bracketreplacer: "_" # if not empty, the brackets in keys of Output Fields are replaced
mutualtlsfilespath: "/etc/certs" # folder which will used to store client.crt, client.key and ca.crt files for mutual tls (default: "/etc/certs")
mutualtlsfilespath: "/etc/certs" # folder which will used to store client.crt, client.key and ca.crt files for mutual tls for outputs (default: "/etc/certs")
tlsserver:
deploy: false # if true, TLS server will be deployed instead of HTTP
certfile: "/etc/certs/server.crt" # server certification file
keyfile: "/etc/certs/server.key" # server key
mutualtls: false # if true, mTLS server will be deployed instead of TLS, deploy also has to be true
cacertfile: "/etc/certs/ca.crt" # for client certification if mutualtls is true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid it will create confusion for end users. We have 2 different mTLS:

  • the mTLS used to authenticate falcosidekick with its outputs
  • the mTLS used by the falcosidekick's webserver
    So we can't use the same folder as they might have conflicts with the ca.crt , wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it could be confusing that the default path is the same folder. They could rename the ca.crt for the server to something else if they need two different ones though.
One solution I see is that we could use etc/certs/server for the new ones and /etc/certs/client for the already existing ones - this sounds more logical in my mind but this would mean breaking change for those who used the default option.
Other path name that I would suggest is /etc/tls/server and leave the client one as is. Just to have something different and in the end everyone can set it to whatever they want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me propose you something, it's mostly to avoid to break anything:

  • /etc/certs/ remains as is, for client and not break anything
  • /etc/certs/client/ is for client too, with client.key, client.crt and ca.crt
  • /etc/certs/server/ is for server, with server.key, server.crt and ca.crt

I'll add a message in the changelog and blog post to explain the /etc/certs is obsolete and will be replaced by /etc/certs/client/ in the falcosidekick-ui in future

Copy link
Contributor Author

@annadorottya annadorottya Jun 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, you mean something like this?

mutualtlsfilespath: "/etc/certs" # folder which will used to store client.crt, client.key and ca.crt files for mutual tls for outputs, will be deprecated in the future (default: "/etc/certs")
mutualtlsclient: # takes priority over mutualtlsfilespath if not emtpy
  certfile: "/etc/certs/client/client.crt" # client certification file
  keyfile: "/etc/certs/client/client.key" # client key
  cacertfile: "/etc/certs/client/ca.crt" # for server certification
tlsserver:
  deploy: false # if true, TLS server will be deployed instead of HTTP
  certfile: "/etc/certs/server/server.crt" # server certification file
  keyfile: "/etc/certs/server/server.key" # server key
  mutualtls: false # if true, mTLS server will be deployed instead of TLS, deploy also has to be true
  cacertfile: "/etc/certs/server/ca.crt" # for client certification if mutualtls is true

Not sure how to resolve the issue of which one gets priority in this case, this is what I could come up with:
By not setting default value for mutualtlsclient file values for now, it will be set to those values if they are set manually and the mutualtlsfilespath + hardcoded file names if mutualtlsfilespath is set or if it's empty (thus backfalls to the default /etc/certs value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok with your solution.

For the mutualtls, do you think we could enforce deploy if it's true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

I think that would be confusing eg. if I turn off the deploy for debugging, then I would expect that the whole TLS part is turned off, but it would still be on. Maybe I could set a warning log message for the deploy == false && mutualtls == true case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for the warning, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the modifications in a new commit, let me know if it's okay with you or I should change something!



slack:
Expand Down
40 changes: 38 additions & 2 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package main

import (
"context"
"crypto/tls"
"crypto/x509"
"fmt"
"io/ioutil"
"log"
"net/http"
"os"
Expand Down Expand Up @@ -729,7 +732,40 @@ func main() {
IdleTimeout: 60 * time.Second,
}

if err := server.ListenAndServe(); err != nil {
log.Fatalf("[ERROR] : %v", err.Error())
if config.TLSServer.Deploy {
if config.TLSServer.MutualTLS {
if config.Debug {
log.Printf("[DEBUG] : running mTLS server")
}

caCert, err := ioutil.ReadFile(config.TLSServer.CaCertFile)
if err != nil {
log.Printf("[ERROR] : %v\n", err.Error())
}
caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caCert)

server.TLSConfig = &tls.Config{
ClientAuth: tls.RequireAndVerifyClientCert,
RootCAs: caCertPool,
MinVersion: tls.VersionTLS12,
}
}

if config.Debug && !config.TLSServer.MutualTLS {
log.Printf("[DEBUG] : running TLS server")
}

if err := server.ListenAndServeTLS(config.TLSServer.CertFile, config.TLSServer.KeyFile); err != nil {
log.Fatalf("[ERROR] : %v", err.Error())
}
} else {
if config.Debug {
log.Printf("[DEBUG] : running HTTP server")
}

if err := server.ListenAndServe(); err != nil {
log.Fatalf("[ERROR] : %v", err.Error())
}
}
}
10 changes: 10 additions & 0 deletions types/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func (f FalcoPayload) Check() bool {
// Configuration is a struct to store configuration
type Configuration struct {
MutualTLSFilesPath string
TLSServer TLSServer
Debug bool
ListenAddress string
ListenPort int
Expand Down Expand Up @@ -104,6 +105,15 @@ type Configuration struct {
N8N N8NConfig
}

// TLSServer represents parameters for TLS Server
type TLSServer struct {
Deploy bool
CertFile string
KeyFile string
MutualTLS bool
CaCertFile string
}

// SlackOutputConfig represents parameters for Slack
type SlackOutputConfig struct {
WebhookURL string
Expand Down