Merge pull request #1492 from transcom/validate_certs

Check DOD certs on startup

Gopkg.toml

@@ -19,3 +19,7 @@ required = [
  [[constraint]]
   name = "github.com/trussworks/pdfcpu"
   branch = "afero"
+
+[[constraint]]
+  branch = "master"
+  name = "go.mozilla.org/pkcs7"

Makefile

@@ -176,11 +176,11 @@ webserver_test: server_generate
 ifndef TEST_ACC_ENV
 	@echo "Running acceptance tests for webserver using local environment."
 	@echo "* Use environment XYZ by setting environment variable to TEST_ACC_ENV=XYZ."
-	TEST_ACC_DATABASE=0 TEST_ACC_HONEYCOMB=0 \
+	TEST_ACC_DATABASE=0 TEST_ACC_DOD_CERTIFICATES=0 TEST_ACC_HONEYCOMB=0 \
 	go test -p 1 -count 1 -short $$(go list ./... | grep \\/cmd\\/webserver) 2> /dev/null
 else
 	@echo "Running acceptance tests for webserver with environment $$TEST_ACC_ENV."
-	TEST_ACC_DATABASE=0 TEST_ACC_HONEYCOMB=0 TEST_ACC_CWD=$$(PWD) \
+	TEST_ACC_DATABASE=0 TEST_ACC_DOD_CERTIFICATES=0 TEST_ACC_HONEYCOMB=0 TEST_ACC_CWD=$$(PWD) \
 	aws-vault exec $$AWS_PROFILE -- \
 	chamber exec app-$$TEST_ACC_ENV -- \
 	go test -p 1 -count 1 -short $$(go list ./... | grep \\/cmd\\/webserver) 2> /dev/null

cmd/webserver/main.go

@@ -88,6 +88,14 @@ func (e *errInvalidRegion) Error() string {
 	return fmt.Sprintf("invalid region %s", e.Region)
 }
 
+type errInvalidPKCS7 struct {
+	Path string
+}
+
+func (e *errInvalidPKCS7) Error() string {
+	return fmt.Sprintf("invalid DER encoded PKCS7 package: %s", e.Path)
+}
+
 func stringSliceContains(stringSlice []string, value string) bool {
 	for _, x := range stringSlice {
 		if value == x {
@@ -239,31 +247,58 @@ func initFlags(flag *pflag.FlagSet) {
 	flag.String("csrf-auth-key", "", "CSRF Auth Key, 32 byte long")
 }
 
-func initDODCertificates(v *viper.Viper, logger *zap.Logger) ([]server.TLSCert, *x509.CertPool, error) {
-
-	moveMilCerts := []server.TLSCert{
-		server.TLSCert{
-			//Append move.mil cert with CA certificate chain
-			CertPEMBlock: bytes.Join([][]byte{
-				[]byte(v.GetString("move-mil-dod-tls-cert")),
-				[]byte(v.GetString("move-mil-dod-ca-cert"))},
-				[]byte("\n"),
-			),
-			KeyPEMBlock: []byte(v.GetString("move-mil-dod-tls-key")),
+func initDODCertificates(v *viper.Viper, logger *zap.Logger) ([]tls.Certificate, *x509.CertPool, error) {
+
+	tlsCert := v.GetString("move-mil-dod-tls-cert")
+	if len(tlsCert) == 0 {
+		return make([]tls.Certificate, 0), nil, errors.Errorf("%s is missing", "move-mil-dod-tls-cert")
+	}
+
+	caCert := v.GetString("move-mil-dod-ca-cert")
+	if len(caCert) == 0 {
+		return make([]tls.Certificate, 0), nil, errors.Errorf("%s is missing", "move-mil-dod-ca-cert")
+	}
+
+	//Append move.mil cert with CA certificate chain
+	cert := bytes.Join(
+		[][]byte{
+			[]byte(tlsCert),
+			[]byte(caCert),
 		},
+		[]byte("\n"),
+	)
+
+	key := []byte(v.GetString("move-mil-dod-tls-key"))
+	if len(key) == 0 {
+		return make([]tls.Certificate, 0), nil, errors.Errorf("%s is missing", "move-mil-dod-tls-key")
+	}
+
+	keyPair, err := tls.X509KeyPair(cert, key)
+	if err != nil {
+		return make([]tls.Certificate, 0), nil, errors.Wrap(err, "failed to parse DOD keypair for server")
+	}
+
+	pathToPackage := v.GetString("dod-ca-package")
+	if len(pathToPackage) == 0 {
+		return make([]tls.Certificate, 0), nil, errors.Wrap(&errInvalidPKCS7{Path: pathToPackage}, fmt.Sprintf("%s is missing", "dod-ca-package"))
 	}
 
-	pkcs7Package, err := ioutil.ReadFile(v.GetString("dod-ca-package")) // #nosec
+	pkcs7Package, err := ioutil.ReadFile(pathToPackage) // #nosec
 	if err != nil {
-		return moveMilCerts, nil, errors.Wrap(err, "Failed to read DoD CA certificate package")
+		return make([]tls.Certificate, 0), nil, errors.Wrap(err, fmt.Sprintf("%s is invalid", "dod-ca-package"))
+	}
+
+	if len(pkcs7Package) == 0 {
+		return make([]tls.Certificate, 0), nil, errors.Wrap(&errInvalidPKCS7{Path: pathToPackage}, fmt.Sprintf("%s is an empty file", "dod-ca-package"))
 	}
 
 	dodCACertPool, err := server.LoadCertPoolFromPkcs7Package(pkcs7Package)
 	if err != nil {
-		return moveMilCerts, dodCACertPool, errors.Wrap(err, "Failed to parse DoD CA certificate package")
+		return make([]tls.Certificate, 0), dodCACertPool, errors.Wrap(err, "Failed to parse DoD CA certificate package")
 	}
 
-	return moveMilCerts, dodCACertPool, nil
+	return []tls.Certificate{keyPair}, dodCACertPool, nil
+
 }
 
 func initRoutePlanner(v *viper.Viper, logger *zap.Logger) route.Planner {
@@ -845,6 +880,9 @@ func main() {
 		logger.Fatal("Failed to initialize DOD certificates", zap.Error(err))
 	}
 
+	logger.Debug("Server DOD Key Pair Loaded")
+	logger.Debug("DOD Certificate Authorities", zap.Any("subjects", dodCACertPool.Subjects()))
+
 	listenInterface := v.GetString("interface")
 
 	go func() {

cmd/webserver/main_test.go

@@ -106,6 +106,12 @@ func (suite *webServerSuite) TestConfigStorage() {
 }
 
 func (suite *webServerSuite) TestDODCertificates() {
+
+	if os.Getenv("TEST_ACC_DOD_CERTIFICATES") != "1" {
+		suite.logger.Info("Skipping TestDODCertificates")
+		return
+	}
+
 	_, _, err := initDODCertificates(suite.viper, suite.logger)
 	suite.Nil(err)
 }

docs/how-to/run-go-tests.md

@@ -22,7 +22,7 @@ If you're adding a feature that requires new or modified configuration, it's a g
 $ TEST_ACC_ENV=experimental make webserver_test
 ```
 
-This command will first load the variables from the `config/env/*.env` file and then run `chamber exec` to pull the environments from AWS.  You can run acceptance tests for the database and honeycomb through environment variables with `TEST_ACC_DATABASE=1` and `TEST_ACC_HONEYCOMB=1`.
+This command will first load the variables from the `config/env/*.env` file and then run `chamber exec` to pull the environments from AWS.  You can run acceptance tests for the database, DOD certificates, honeycomb through environment variables with `TEST_ACC_DATABASE=1`, `TEST_ACC_DOD_CERTIFICATES=1`, and `TEST_ACC_HONEYCOMB=1`, respectively.
 
 ### Run All Tests in a Single Package
 

pkg/server/certificates.go

@@ -3,7 +3,7 @@ package server
 import (
 	"crypto/x509"
 
-	"github.com/fullsailor/pkcs7"
+	"go.mozilla.org/pkcs7"
 )
 
 // LoadCertPoolFromPkcs7Package reads the certificates in a DER-encoded PKCS7

pkg/server/server.go

@@ -28,13 +28,6 @@ var ErrUnparseableCACert = errors.New("unable to parse CA certificate")
 
 type serverFunc func(server *http.Server) error
 
-// TLSCert encapsulates a public certificate and private key.
-// Each are represented as a slice of bytes.
-type TLSCert struct {
-	CertPEMBlock []byte
-	KeyPEMBlock  []byte
-}
-
 // Server represents an http or https listening server. HTTPS listeners support
 // requiring client authentication with a provided CA.
 type Server struct {
@@ -44,7 +37,7 @@ type Server struct {
 	ListenAddress  string
 	Logger         *zap.Logger
 	Port           int
-	TLSCerts       []TLSCert
+	TLSCerts       []tls.Certificate
 }
 
 // addr generates an address:port string to be used in defining an http.Server
@@ -85,10 +78,8 @@ func (s Server) serverConfig(tlsConfig *tls.Config) (*http.Server, error) {
 	return serverConfig, err
 }
 
-// tlsConfig generates a new *tls.Config. It will
+// tlsConfig generates a new *tls.Config based on Mozilla's recommendations and returns an error, if any.
 func (s Server) tlsConfig() (*tls.Config, error) {
-	var tlsCerts []tls.Certificate
-	var err error
 
 	// Load client Certificate Authority (CA) if we are requiring client
 	// cert authentication.
@@ -99,16 +90,6 @@ func (s Server) tlsConfig() (*tls.Config, error) {
 		}
 	}
 
-	// Parse and append all of the TLSCerts to the tls.Config
-	for _, cert := range s.TLSCerts {
-		parsedCert, err := tls.X509KeyPair(cert.CertPEMBlock, cert.KeyPEMBlock)
-		if err != nil {
-			s.Logger.Error("failed to parse tls certificate", zap.Error(err))
-			return nil, err
-		}
-		tlsCerts = append(tlsCerts, parsedCert)
-	}
-
 	// Follow Mozilla's "modern" server side TLS recommendations
 	// https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility
 	// https://statics.tls.security.mozilla.org/server-side-tls-conf-4.0.json
@@ -123,7 +104,7 @@ func (s Server) tlsConfig() (*tls.Config, error) {
 			tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
 			tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
 		},
-		Certificates: tlsCerts,
+		Certificates: s.TLSCerts,
 		ClientAuth:   s.ClientAuthType,
 		ClientCAs:    s.CaCertPool,
 		CurvePreferences: []tls.CurveID{
@@ -136,12 +117,12 @@ func (s Server) tlsConfig() (*tls.Config, error) {
 	}
 
 	// Map certificates with the CommonName / DNSNames to support
-	// Subject Name Indication (SNI). In other words this will tell
+	// Server Name Indication (SNI). In other words this will tell
 	// the TLS listener to sever the appropriate certificate matching
 	// the requested hostname.
 	tlsConfig.BuildNameToCertificate()
 
-	return tlsConfig, err
+	return tlsConfig, nil
 }
 
 // ListenAndServeTLS returns a TLS Listener function for serving HTTPS requests