From bd682bf881b80639251f7a2ab4f7af2a50a1627b Mon Sep 17 00:00:00 2001 From: Nate Mueller Date: Wed, 3 Jan 2018 13:19:15 -0800 Subject: [PATCH 1/2] Native SSL support Add two new flags for atlantis server: --ssl-key-file for the key --ssl-cert-file for the server and CA certs You either need to use both or neither. You'll get an error if you accidentally use only one. Currently defaults to non-SSL mode. I think you could make a good argument for SSL by default but since that would be a breaking change I left it for the maintainers to decide. Removed docs for setting up an NGINX frontend. --- README.md | 2 +- cmd/server.go | 16 +++++++++- docs/nginx-ssl-proxy.md | 66 ----------------------------------------- server/server.go | 16 +++++++++- 4 files changed, 31 insertions(+), 69 deletions(-) delete mode 100644 docs/nginx-ssl-proxy.md diff --git a/README.md b/README.md index f5d7f26e..3ea7c72f 100644 --- a/README.md +++ b/README.md @@ -481,7 +481,7 @@ However, if you were to lose the data, all you would need to do is run `atlantis **Q: How to add SSL to Atlantis server?** -A: Atlantis currently only supports HTTP. In order to add SSL you will need to front Atlantis server with NGINX or HAProxy. Follow the document [here](./docs/nginx-ssl-proxy.md) to use configure NGINX with SSL as a reverse proxy. +A: Pass the `--ssl` option to enable SSL for incoming connections. You will need to get a trusted certificate and pass it into Atlantis server with the `--ssl-key-file` and `--ssl-cert-file` options. ## Contributing diff --git a/cmd/server.go b/cmd/server.go index 497d70b6..01abcf61 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -31,6 +31,8 @@ const ( LogLevelFlag = "log-level" PortFlag = "port" RequireApprovalFlag = "require-approval" + SSLCertFileFlag = "ssl-cert-file" + SSLKeyFileFlag = "ssl-key-file" ) var stringFlags = []stringFlag{ @@ -94,6 +96,14 @@ var stringFlags = []stringFlag{ description: "Log level. Either debug, info, warn, or error.", value: "info", }, + { + name: SSLCertFileFlag, + description: "File containing x509 Certificate used for serving HTTPS. If the cert is signed by a CA, the file should be the concatenation of the server's certificate, any intermediates, and the CA's certificate.", + }, + { + name: SSLKeyFileFlag, + description: fmt.Sprintf("File containing x509 private key matching --%s.", SSLCertFileFlag), + }, } var boolFlags = []boolFlag{ { @@ -248,13 +258,17 @@ func (s *ServerCmd) validate(config server.Config) error { if logLevel != "debug" && logLevel != "info" && logLevel != "warn" && logLevel != "error" { return errors.New("invalid log level: not one of debug, info, warn, error") } - vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GitlabUserFlag, GitlabTokenFlag) + + if (config.SSLKeyFile == "") != (config.SSLCertFile == "") { + return fmt.Errorf("%s and %s are required for ssl", SSLKeyFileFlag, SSLCertFileFlag) + } // The following combinations are valid. // 1. github user and token // 2. gitlab user and token // 3. all 4 set // We validate using contradiction (I think). + vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GitlabUserFlag, GitlabTokenFlag) if config.GithubUser != "" && config.GithubToken == "" || config.GithubToken != "" && config.GithubUser == "" { return vcsErr } diff --git a/docs/nginx-ssl-proxy.md b/docs/nginx-ssl-proxy.md deleted file mode 100644 index 7d1d0354..00000000 --- a/docs/nginx-ssl-proxy.md +++ /dev/null @@ -1,66 +0,0 @@ -# NGINX SSL Proxy -This document shows how to configure Nginx with SSL as a reverse proxy for Atlantis server. - -* Install NGINX - -```bash -sudo apt-get update -sudo apt-get install nginx -``` - -* Install a SSL Certificate -This certificate can be purchased or generated. Here is a example of generating a self signed SSL certificate. - -```bash -cd /etc/nginx -sudo openssl req -x509 -nodes -days 365 -newkey rsa:2048 -keyout /etc/nginx/cert.key -out /etc/nginx/cert.crt -``` -You will be prompted to enter some information about the certificate. Fill those as you like. - -* Edit NGINX Config - -```bash -sudo vim /etc/nginx/sites-enabled/default -server { - listen 80; - return 301 https://$host$request_uri; -} - -server { - - listen 443; - server_name atlantis.domain.com; - - ssl_certificate /etc/nginx/cert.crt; - ssl_certificate_key /etc/nginx/cert.key; - - ssl on; - ssl_session_cache builtin:1000 shared:SSL:10m; - ssl_protocols TLSv1 TLSv1.1 TLSv1.2; - ssl_ciphers HIGH:!aNULL:!eNULL:!EXPORT:!CAMELLIA:!DES:!MD5:!PSK:!RC4; - ssl_prefer_server_ciphers on; - - access_log /var/log/nginx/atlantis.access.log; - - location / { - - proxy_set_header Host $host; - proxy_set_header X-Real-IP $remote_addr; - proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $scheme; - - # Fixes the “It appears that your reverse proxy set up is broken" error. - proxy_pass http://localhost:4141; - proxy_read_timeout 90; - - proxy_redirect http://localhost:4141 https://atlantis.domain.com; - } - } - ``` - - * Restart NGINX - - ```bash - sudo service nginx restart - - ``` \ No newline at end of file diff --git a/server/server.go b/server/server.go index 7281c863..4fb58e1e 100644 --- a/server/server.go +++ b/server/server.go @@ -45,6 +45,8 @@ type Server struct { EventsController *EventsController IndexTemplate TemplateWriter LockDetailTemplate TemplateWriter + SSLCertFile string + SSLKeyFile string } // Config configures Server. @@ -67,6 +69,8 @@ type Config struct { // allowing terraform apply's to be run. RequireApproval bool `mapstructure:"require-approval"` SlackToken string `mapstructure:"slack-token"` + SSLCertFile string `mapstructure:"ssl-cert-file"` + SSLKeyFile string `mapstructure:"ssl-key-file"` Webhooks []WebhookConfig `mapstructure:"webhooks"` } @@ -214,6 +218,8 @@ func NewServer(config Config) (*Server, error) { EventsController: eventsController, IndexTemplate: indexTemplate, LockDetailTemplate: lockTemplate, + SSLKeyFile: config.SSLKeyFile, + SSLCertFile: config.SSLCertFile, }, nil } @@ -249,7 +255,15 @@ func (s *Server) Start() error { server := &http.Server{Addr: fmt.Sprintf(":%d", s.Port), Handler: n} go func() { s.Logger.Warn("Atlantis started - listening on port %v", s.Port) - if err := server.ListenAndServe(); err != nil { + + var err error + if s.SSLCertFile != "" && s.SSLKeyFile != "" { + err = server.ListenAndServeTLS(s.SSLCertFile, s.SSLKeyFile) + } else { + err = server.ListenAndServe() + } + + if err != nil { // When shutdown safely, there will be no error. s.Logger.Err(err.Error()) } From 2b9f42dd549397e29b0edf7ccbb1882175e8859b Mon Sep 17 00:00:00 2001 From: Luke Kysow Date: Tue, 23 Jan 2018 11:21:14 -0800 Subject: [PATCH 2/2] Add tests to ssl and clean up errors --- README.md | 4 +++- cmd/server.go | 14 +++++-------- cmd/server_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 3ea7c72f..a47069ec 100644 --- a/README.md +++ b/README.md @@ -481,7 +481,9 @@ However, if you were to lose the data, all you would need to do is run `atlantis **Q: How to add SSL to Atlantis server?** -A: Pass the `--ssl` option to enable SSL for incoming connections. You will need to get a trusted certificate and pass it into Atlantis server with the `--ssl-key-file` and `--ssl-cert-file` options. +A: First, you'll need to get a public/private key pair to serve over SSL. +These need to be in a directory accessible by Atlantis. Then start `atlantis server` with the `--ssl-cert-file` and `--ssl-key-file` flags. +See `atlantis server --help` for more information. ## Contributing diff --git a/cmd/server.go b/cmd/server.go index 01abcf61..7b977214 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -260,23 +260,19 @@ func (s *ServerCmd) validate(config server.Config) error { } if (config.SSLKeyFile == "") != (config.SSLCertFile == "") { - return fmt.Errorf("%s and %s are required for ssl", SSLKeyFileFlag, SSLCertFileFlag) + return fmt.Errorf("--%s and --%s are both required for ssl", SSLKeyFileFlag, SSLCertFileFlag) } // The following combinations are valid. - // 1. github user and token - // 2. gitlab user and token + // 1. github user and token set + // 2. gitlab user and token set // 3. all 4 set - // We validate using contradiction (I think). vcsErr := fmt.Errorf("--%s/--%s or --%s/--%s must be set", GHUserFlag, GHTokenFlag, GitlabUserFlag, GitlabTokenFlag) - if config.GithubUser != "" && config.GithubToken == "" || config.GithubToken != "" && config.GithubUser == "" { - return vcsErr - } - if config.GitlabUser != "" && config.GitlabToken == "" || config.GitlabToken != "" && config.GitlabUser == "" { + if ((config.GithubUser == "") != (config.GithubToken == "")) || ((config.GitlabUser == "") != (config.GitlabToken == "")) { return vcsErr } // At this point, we know that there can't be a single user/token without - // its pair, but we haven't checked if any user/token is set at all. + // its partner, but we haven't checked if any user/token is set at all. if config.GithubUser == "" && config.GitlabUser == "" { return vcsErr } diff --git a/cmd/server_test.go b/cmd/server_test.go index 56357d2f..b3fdc4d4 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -102,6 +102,58 @@ func TestExecute_ValidateLogLevel(t *testing.T) { Equals(t, "invalid log level: not one of debug, info, warn, error", err.Error()) } +func TestExecute_ValidateSSLConfig(t *testing.T) { + expErr := "--ssl-key-file and --ssl-cert-file are both required for ssl" + cases := []struct { + description string + flags map[string]interface{} + expectError bool + }{ + { + "neither option set", + make(map[string]interface{}), + false, + }, + { + "just ssl-key-file set", + map[string]interface{}{ + cmd.SSLKeyFileFlag: "file", + }, + true, + }, + { + "just ssl-cert-file set", + map[string]interface{}{ + cmd.SSLCertFileFlag: "flag", + }, + true, + }, + { + "both flags set", + map[string]interface{}{ + cmd.SSLCertFileFlag: "cert", + cmd.SSLKeyFileFlag: "key", + }, + false, + }, + } + for _, testCase := range cases { + t.Log("Should validate ssl config when " + testCase.description) + // Add in required flags. + testCase.flags[cmd.GHUserFlag] = "user" + testCase.flags[cmd.GHTokenFlag] = "token" + + c := setup(testCase.flags) + err := c.Execute() + if testCase.expectError { + Assert(t, err != nil, "should be an error") + Equals(t, expErr, err.Error()) + } else { + Ok(t, err) + } + } +} + func TestExecute_ValidateVCSConfig(t *testing.T) { expErr := "--gh-user/--gh-token or --gitlab-user/--gitlab-token must be set" cases := []struct {