From 5f60d0e2ac8695cf85933939901c81786f29e581 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Mon, 21 Oct 2019 17:30:21 -0400 Subject: [PATCH 1/7] first draft --- config/config.go | 3 +++ router/router.go | 8 +++++++- ssl/ssl.go | 22 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 7a5a72e9df0..57b967548fd 100644 --- a/config/config.go +++ b/config/config.go @@ -60,6 +60,8 @@ type Configuration struct { BlacklistedAcctMap map[string]bool // Is publisher/account ID required to be submitted in the OpenRTB2 request AccountRequired bool `mapstructure:"account_required"` + // Local private file containing SSL certificates + PemCertsFile string `mapstructure:"certificates_file"` } const MIN_COOKIE_SIZE_BYTES = 500 @@ -705,6 +707,7 @@ func SetupViper(v *viper.Viper, filename string) { v.SetDefault("blacklisted_apps", []string{""}) v.SetDefault("blacklisted_accts", []string{""}) v.SetDefault("account_required", false) + v.SetDefault("certificates_file", "") // Set environment variable support: v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) diff --git a/router/router.go b/router/router.go index 15478f7b38a..8090cbeb010 100644 --- a/router/router.go +++ b/router/router.go @@ -175,12 +175,18 @@ func New(cfg *config.Configuration, rateConvertor *currencies.RateConverter) (r r = &Router{ Router: httprouter.New(), } + + // For bid processing, we need both the hardcoded certificates and the certificates found in container's + // local file system + certPool := ssl.GetRootCAPool() + ssl.AppendPEMFileToRootCAPool(certPool, cfg.PemCertsFile) + theClient := &http.Client{ Transport: &http.Transport{ MaxIdleConns: cfg.Client.MaxIdleConns, MaxIdleConnsPerHost: cfg.Client.MaxIdleConnsPerHost, IdleConnTimeout: time.Duration(cfg.Client.IdleConnTimeout) * time.Second, - TLSClientConfig: &tls.Config{RootCAs: ssl.GetRootCAPool()}, + TLSClientConfig: &tls.Config{RootCAs: certPool}, }, } // Hack because of how legacy handles districtm diff --git a/ssl/ssl.go b/ssl/ssl.go index bafd91613c7..96050e576f3 100644 --- a/ssl/ssl.go +++ b/ssl/ssl.go @@ -16,6 +16,28 @@ func GetRootCAPool() *x509.CertPool { return pool } +// Appends certificates to the `x509.CertPool` from a `.pem` private local file. On many Linux +// systems, /etc/ssl/cert.pem will contain the system wide set but in our case, we'll pull +// the certificate file path from the `Configuration` struct +func AppendPEMFileToRootCAPool(certPool *x509.CertPool, pemFileName string) error { + if certPool == nil { + certPool = x509.NewCertPool() + } + if pemFileName != "" { + //read file and place it's contents in `pemCerts` + pemCerts, err := ioutil.ReadFile(pemFileName) + if err != nil { + return nil, fmt.Errorf("Failed to read file %s: %v", pemFileName, err) + } + + //`pemCerts` has been obtained, append to certPool + certPool.AppendCertsFromPEM(pemCerts) + } else { + return fmt.Errorf("Path to local PEM file containing SSL certificates is\n") + } + return nil +} + var pemCerts = []byte(` -----BEGIN CERTIFICATE----- MIIDzzCCAregAwIBAgIDAWweMA0GCSqGSIb3DQEBBQUAMIGNMQswCQYDVQQGEwJB From 87ee5d2988a33e6b22009b6436610a59f5c06070 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 22 Oct 2019 15:20:11 -0400 Subject: [PATCH 2/7] Test cases --- config/config_test.go | 4 ++++ ssl/mockcertificates/mock-certs.pem | 11 +++++++++++ ssl/ssl.go | 4 +++- ssl/ssl_test.go | 21 +++++++++++++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 ssl/mockcertificates/mock-certs.pem create mode 100644 ssl/ssl_test.go diff --git a/config/config_test.go b/config/config_test.go index 17c8c5e3f6b..11dc601bf3a 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -32,6 +32,8 @@ func TestDefaults(t *testing.T) { cmpBools(t, "account_required", cfg.AccountRequired, false) cmpInts(t, "metrics.influxdb.collection_rate_seconds", cfg.Metrics.Influxdb.MetricSendInterval, 20) cmpBools(t, "account_adapter_details", cfg.Metrics.Disabled.AccountAdapterDetails, false) + cmpStrings(t, "adapters.pubmatic.endpoint", cfg.Adapters[string(openrtb_ext.BidderPubmatic)].Endpoint, "http://hbopenbid.pubmatic.com/translator?source=prebid-server") + cmpStrings(t, "certificates_file", cfg.PemCertsFile, "") } var fullConfig = []byte(` @@ -105,6 +107,7 @@ adapters: usersync_url: https://tag.adkernel.com/syncr?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&r= blacklisted_apps: ["spamAppID","sketchy-app-id"] account_required: true +certificates_file: /etc/ssl/cert.pem `) var adapterExtraInfoConfig = []byte(` @@ -266,6 +269,7 @@ func TestFullConfig(t *testing.T) { cmpStrings(t, "adapters.rhythmone.usersync_url", cfg.Adapters[string(openrtb_ext.BidderRhythmone)].UserSyncURL, "https://sync.1rx.io/usersync2/rmphb?gdpr={{.GDPR}}&gdpr_consent={{.GDPRConsent}}&redir=http%3A%2F%2Fprebid-server.prebid.org%2F%2Fsetuid%3Fbidder%3Drhythmone%26gdpr%3D{{.GDPR}}%26gdpr_consent%3D{{.GDPRConsent}}%26uid%3D%5BRX_UUID%5D") cmpBools(t, "account_required", cfg.AccountRequired, true) cmpBools(t, "account_adapter_details", cfg.Metrics.Disabled.AccountAdapterDetails, true) + cmpStrings(t, "certificates_file", cfg.PemCertsFile, "/etc/ssl/cert.pem") } func TestUnmarshalAdapterExtraInfo(t *testing.T) { diff --git a/ssl/mockcertificates/mock-certs.pem b/ssl/mockcertificates/mock-certs.pem new file mode 100644 index 00000000000..e0bf7db58ff --- /dev/null +++ b/ssl/mockcertificates/mock-certs.pem @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw +DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow +EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d +7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B +5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr +BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1 +NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l +Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc +6MF9+Yw1Yy0t +-----END CERTIFICATE----- diff --git a/ssl/ssl.go b/ssl/ssl.go index 96050e576f3..82cce57c81a 100644 --- a/ssl/ssl.go +++ b/ssl/ssl.go @@ -2,6 +2,8 @@ package ssl import ( "crypto/x509" + "fmt" + "io/ioutil" ) // from https://medium.com/@kelseyhightower/optimizing-docker-images-for-static-binaries-b5696e26eb07 @@ -27,7 +29,7 @@ func AppendPEMFileToRootCAPool(certPool *x509.CertPool, pemFileName string) erro //read file and place it's contents in `pemCerts` pemCerts, err := ioutil.ReadFile(pemFileName) if err != nil { - return nil, fmt.Errorf("Failed to read file %s: %v", pemFileName, err) + return fmt.Errorf("Failed to read file %s: %v", pemFileName, err) } //`pemCerts` has been obtained, append to certPool diff --git a/ssl/ssl_test.go b/ssl/ssl_test.go new file mode 100644 index 00000000000..a2fbf851820 --- /dev/null +++ b/ssl/ssl_test.go @@ -0,0 +1,21 @@ +package ssl + +import ( + //"crypto/x509" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAppendPEMFileToRootCAPool(t *testing.T) { + certPool := GetRootCAPool() + subjects := certPool.Subjects() + + hardCodedSubNum := len(subjects) + assert.True(t, hardCodedSubNum > 0) + + AppendPEMFileToRootCAPool(certPool, "mockcertificates/mock-certs.pem") + subjects = certPool.Subjects() + subNumIncludingFile := len(subjects) + assert.True(t, subNumIncludingFile > hardCodedSubNum, "subNumIncludingFile should be greater than hardCodedSubNum") +} From d419894aeedbc0af79a029ed4e79aec2b298fa7c Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 22 Oct 2019 16:10:13 -0400 Subject: [PATCH 3/7] Expanding ssl_test.go --- ssl/ssl.go | 8 ++++---- ssl/ssl_test.go | 41 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/ssl/ssl.go b/ssl/ssl.go index 82cce57c81a..5b19acf30f8 100644 --- a/ssl/ssl.go +++ b/ssl/ssl.go @@ -21,7 +21,7 @@ func GetRootCAPool() *x509.CertPool { // Appends certificates to the `x509.CertPool` from a `.pem` private local file. On many Linux // systems, /etc/ssl/cert.pem will contain the system wide set but in our case, we'll pull // the certificate file path from the `Configuration` struct -func AppendPEMFileToRootCAPool(certPool *x509.CertPool, pemFileName string) error { +func AppendPEMFileToRootCAPool(certPool *x509.CertPool, pemFileName string) (*x509.CertPool, error) { if certPool == nil { certPool = x509.NewCertPool() } @@ -29,15 +29,15 @@ func AppendPEMFileToRootCAPool(certPool *x509.CertPool, pemFileName string) erro //read file and place it's contents in `pemCerts` pemCerts, err := ioutil.ReadFile(pemFileName) if err != nil { - return fmt.Errorf("Failed to read file %s: %v", pemFileName, err) + return certPool, fmt.Errorf("Failed to read file %s: %v", pemFileName, err) } //`pemCerts` has been obtained, append to certPool certPool.AppendCertsFromPEM(pemCerts) } else { - return fmt.Errorf("Path to local PEM file containing SSL certificates is\n") + return certPool, fmt.Errorf("Path to local PEM file containing SSL certificates is\n") } - return nil + return certPool, nil } var pemCerts = []byte(` diff --git a/ssl/ssl_test.go b/ssl/ssl_test.go index a2fbf851820..2e7a4440995 100644 --- a/ssl/ssl_test.go +++ b/ssl/ssl_test.go @@ -1,21 +1,54 @@ package ssl import ( - //"crypto/x509" + "crypto/x509" "testing" "github.com/stretchr/testify/assert" ) -func TestAppendPEMFileToRootCAPool(t *testing.T) { +func TestCertsFromFilePoolExists(t *testing.T) { + // Load hardcoded certificates found in ssl.go certPool := GetRootCAPool() - subjects := certPool.Subjects() + // Assert loaded certificates by looking at the lenght of the subjects array of strings + subjects := certPool.Subjects() hardCodedSubNum := len(subjects) assert.True(t, hardCodedSubNum > 0) - AppendPEMFileToRootCAPool(certPool, "mockcertificates/mock-certs.pem") + // Load certificates from file + certificatesFile := "mockcertificates/mock-certs.pem" + certPool, err := AppendPEMFileToRootCAPool(certPool, certificatesFile) + + // Assert loaded certificates by looking at the lenght of the subjects array of strings + assert.NoError(t, err, "Error thrown by AppendPEMFileToRootCAPool while loading file %s: %v", certificatesFile, err) subjects = certPool.Subjects() subNumIncludingFile := len(subjects) assert.True(t, subNumIncludingFile > hardCodedSubNum, "subNumIncludingFile should be greater than hardCodedSubNum") } + +func TestCertsFromFilePoolDontExist(t *testing.T) { + // Empty certpool + var certPool *x509.CertPool = nil + + // Load certificates from file + certificatesFile := "mockcertificates/mock-certs.pem" + certPool, err := AppendPEMFileToRootCAPool(certPool, certificatesFile) + + // Assert loaded certificates by looking at the lenght of the subjects array of strings + assert.NoError(t, err, "Error thrown by AppendPEMFileToRootCAPool while loading file %s: %v", certificatesFile, err) + subjects := certPool.Subjects() + assert.Equal(t, len(subjects), 1, "We only loaded one vertificate from the file, len(subjects) should equal 1") +} + +func TestAppendPEMFileToRootCAPoolFail(t *testing.T) { + // Empty certpool + var certPool *x509.CertPool + + // Load certificates from file + fakeCertificatesFile := "mockcertificates/NO-FILE.pem" + certPool, err := AppendPEMFileToRootCAPool(certPool, fakeCertificatesFile) + + // Assert loaded certificates by looking at the lenght of the subjects array of strings + assert.Errorf(t, err, "AppendPEMFileToRootCAPool should throw an error by while loading fake file %s \n", fakeCertificatesFile) +} From 20fded48200f3e61d6858c233987bbf80b22a399 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 22 Oct 2019 16:41:01 -0400 Subject: [PATCH 4/7] Corrected router.go --- router/router.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/router/router.go b/router/router.go index 8090cbeb010..db5804ac6d9 100644 --- a/router/router.go +++ b/router/router.go @@ -179,7 +179,11 @@ func New(cfg *config.Configuration, rateConvertor *currencies.RateConverter) (r // For bid processing, we need both the hardcoded certificates and the certificates found in container's // local file system certPool := ssl.GetRootCAPool() - ssl.AppendPEMFileToRootCAPool(certPool, cfg.PemCertsFile) + var readCertErr Error + certPool, readCertErr = ssl.AppendPEMFileToRootCAPool(certPool, cfg.PemCertsFile) + if readCertErr != nil { + glog.Infof("Could not read certificates file: %s \n", readCertErr.Error()) + } theClient := &http.Client{ Transport: &http.Transport{ From 6dca2fa8251ffef5df9e809a1c1d4519ffde5933 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Tue, 22 Oct 2019 16:47:11 -0400 Subject: [PATCH 5/7] Corrected entry inside config/config_test.go --- config/config_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/config/config_test.go b/config/config_test.go index 11dc601bf3a..14eda7496a7 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -32,7 +32,6 @@ func TestDefaults(t *testing.T) { cmpBools(t, "account_required", cfg.AccountRequired, false) cmpInts(t, "metrics.influxdb.collection_rate_seconds", cfg.Metrics.Influxdb.MetricSendInterval, 20) cmpBools(t, "account_adapter_details", cfg.Metrics.Disabled.AccountAdapterDetails, false) - cmpStrings(t, "adapters.pubmatic.endpoint", cfg.Adapters[string(openrtb_ext.BidderPubmatic)].Endpoint, "http://hbopenbid.pubmatic.com/translator?source=prebid-server") cmpStrings(t, "certificates_file", cfg.PemCertsFile, "") } From 79f211bca4f7a3365173a10c005cb84bf4318069 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Fri, 25 Oct 2019 11:02:17 -0400 Subject: [PATCH 6/7] removed error return in ssl.go --- router/router.go | 2 +- ssl/ssl.go | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/router/router.go b/router/router.go index db5804ac6d9..854b7fcdb94 100644 --- a/router/router.go +++ b/router/router.go @@ -179,7 +179,7 @@ func New(cfg *config.Configuration, rateConvertor *currencies.RateConverter) (r // For bid processing, we need both the hardcoded certificates and the certificates found in container's // local file system certPool := ssl.GetRootCAPool() - var readCertErr Error + var readCertErr error certPool, readCertErr = ssl.AppendPEMFileToRootCAPool(certPool, cfg.PemCertsFile) if readCertErr != nil { glog.Infof("Could not read certificates file: %s \n", readCertErr.Error()) diff --git a/ssl/ssl.go b/ssl/ssl.go index 5b19acf30f8..d05c90154b9 100644 --- a/ssl/ssl.go +++ b/ssl/ssl.go @@ -34,8 +34,6 @@ func AppendPEMFileToRootCAPool(certPool *x509.CertPool, pemFileName string) (*x5 //`pemCerts` has been obtained, append to certPool certPool.AppendCertsFromPEM(pemCerts) - } else { - return certPool, fmt.Errorf("Path to local PEM file containing SSL certificates is\n") } return certPool, nil } From 4ba89dffabdd4cf868a0481c6f5c047886ba0822 Mon Sep 17 00:00:00 2001 From: Gus Carreon Date: Mon, 4 Nov 2019 16:17:22 -0500 Subject: [PATCH 7/7] Correct typos in commented lines --- ssl/ssl_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ssl/ssl_test.go b/ssl/ssl_test.go index 2e7a4440995..c4c29d149ef 100644 --- a/ssl/ssl_test.go +++ b/ssl/ssl_test.go @@ -11,7 +11,7 @@ func TestCertsFromFilePoolExists(t *testing.T) { // Load hardcoded certificates found in ssl.go certPool := GetRootCAPool() - // Assert loaded certificates by looking at the lenght of the subjects array of strings + // Assert loaded certificates by looking at the length of the subjects array of strings subjects := certPool.Subjects() hardCodedSubNum := len(subjects) assert.True(t, hardCodedSubNum > 0) @@ -20,7 +20,7 @@ func TestCertsFromFilePoolExists(t *testing.T) { certificatesFile := "mockcertificates/mock-certs.pem" certPool, err := AppendPEMFileToRootCAPool(certPool, certificatesFile) - // Assert loaded certificates by looking at the lenght of the subjects array of strings + // Assert loaded certificates by looking at the length of the subjects array of strings assert.NoError(t, err, "Error thrown by AppendPEMFileToRootCAPool while loading file %s: %v", certificatesFile, err) subjects = certPool.Subjects() subNumIncludingFile := len(subjects) @@ -35,7 +35,7 @@ func TestCertsFromFilePoolDontExist(t *testing.T) { certificatesFile := "mockcertificates/mock-certs.pem" certPool, err := AppendPEMFileToRootCAPool(certPool, certificatesFile) - // Assert loaded certificates by looking at the lenght of the subjects array of strings + // Assert loaded certificates by looking at the length of the subjects array of strings assert.NoError(t, err, "Error thrown by AppendPEMFileToRootCAPool while loading file %s: %v", certificatesFile, err) subjects := certPool.Subjects() assert.Equal(t, len(subjects), 1, "We only loaded one vertificate from the file, len(subjects) should equal 1") @@ -45,10 +45,10 @@ func TestAppendPEMFileToRootCAPoolFail(t *testing.T) { // Empty certpool var certPool *x509.CertPool - // Load certificates from file + // In this test we are going to pass a file that does not exist as value of second argument fakeCertificatesFile := "mockcertificates/NO-FILE.pem" certPool, err := AppendPEMFileToRootCAPool(certPool, fakeCertificatesFile) - // Assert loaded certificates by looking at the lenght of the subjects array of strings + // Assert AppendPEMFileToRootCAPool correctly throws an error when trying to load an nonexisting file assert.Errorf(t, err, "AppendPEMFileToRootCAPool should throw an error by while loading fake file %s \n", fakeCertificatesFile) }