-
Notifications
You must be signed in to change notification settings - Fork 760
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.
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
Add SSL cert for accessing stored request API #1087
Changes from 6 commits
5f60d0e
87ee5d2
d419894
20fded4
8e992f2
6dca2fa
79f211b
4ba89df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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----- |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ package ssl | |
|
||
import ( | ||
"crypto/x509" | ||
"fmt" | ||
"io/ioutil" | ||
) | ||
|
||
// from https://medium.com/@kelseyhightower/optimizing-docker-images-for-static-binaries-b5696e26eb07 | ||
|
@@ -16,6 +18,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) (*x509.CertPool, 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 certPool, fmt.Errorf("Failed to read file %s: %v", pemFileName, err) | ||
} | ||
|
||
//`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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This error message is confusing ... is it really an error to fail to read a cert file when none was given? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed |
||
} | ||
return certPool, nil | ||
} | ||
|
||
var pemCerts = []byte(` | ||
-----BEGIN CERTIFICATE----- | ||
MIIDzzCCAregAwIBAgIDAWweMA0GCSqGSIb3DQEBBQUAMIGNMQswCQYDVQQGEwJB | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
package ssl | ||
|
||
import ( | ||
"crypto/x509" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: lenght |
||
subjects := certPool.Subjects() | ||
hardCodedSubNum := len(subjects) | ||
assert.True(t, hardCodedSubNum > 0) | ||
|
||
// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: lenght |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: lenght |
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is likely a copy/paste from the other tests, but doesn't make sense in this instance because an error is expected and the length is never checked. |
||
assert.Errorf(t, err, "AppendPEMFileToRootCAPool should throw an error by while loading fake file %s \n", fakeCertificatesFile) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a check if the string is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
AppendPEMFileToRootCAPool(certPool *x509.CertPool, pemFileName string)
function checks for an empty string but I removed the error it was returning when the string is empty because as of right now, we have no certificates file and it seems odd to return an error if the certificates file has not been defined in the configuration.Do you prefer the calling function to do the empty string check?