Merge pull request #4340 from hashicorp/f-tls-parse-certs
Parse CA certificate to catch more specific errors
This commit is contained in:
Alex Dadgar
GitHub
commit
5aebd8d2d0
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
IMPROVEMENTS:
|
||||
* core: Updated serf library to improve how leave intents are handled [[GH-4278](https://github.com/hashicorp/nomad/issues/4278)]
|
||||
* core: Add more descriptive errors when parsing agent TLS certificates [[GH-4340](https://github.com/hashicorp/nomad/issues/4340)]
|
||||
* core: Added TLS configuration option to prefer server's ciphersuites over clients[[GH-4338](https://github.com/hashicorp/nomad/issues/4338)]
|
||||
* core: Add the option for operators to configure TLS versions and allowed
|
||||
cipher suites. Default is a subset of safe ciphers and TLS 1.2 [[GH-4269](https://github.com/hashicorp/nomad/pull/4269)]
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package tlsutil
|
|||
import (
|
||||
"crypto/tls"
|
||||
"crypto/x509"
|
||||
"encoding/pem"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"net"
|
||||
|
|
@ -153,8 +154,35 @@ func (c *Config) AppendCA(pool *x509.CertPool) error {
|
|||
return fmt.Errorf("Failed to read CA file: %v", err)
|
||||
}
|
||||
|
||||
block, rest := pem.Decode(data)
|
||||
if err := validateCertificate(block); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
for len(rest) > 0 {
|
||||
block, rest = pem.Decode(rest)
|
||||
if err := validateCertificate(block); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
if !pool.AppendCertsFromPEM(data) {
|
||||
return fmt.Errorf("Failed to parse any CA certificates")
|
||||
return fmt.Errorf("Failed to add any CA certificates")
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateCertificate checks to ensure a certificate is valid. If it is not,
|
||||
// return a descriptive error of why the certificate is invalid.
|
||||
func validateCertificate(block *pem.Block) error {
|
||||
if block == nil {
|
||||
return fmt.Errorf("Failed to decode CA file from pem format")
|
||||
}
|
||||
|
||||
// Parse the certificate to ensure that it is properly formatted
|
||||
if _, err := x509.ParseCertificates(block.Bytes); err != nil {
|
||||
return fmt.Errorf("Failed to parse CA file: %v", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import (
|
|||
"io"
|
||||
"io/ioutil"
|
||||
"net"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
|
|
@ -26,14 +27,142 @@ const (
|
|||
)
|
||||
|
||||
func TestConfig_AppendCA_None(t *testing.T) {
|
||||
require := require.New(t)
|
||||
|
||||
conf := &Config{}
|
||||
pool := x509.NewCertPool()
|
||||
err := conf.AppendCA(pool)
|
||||
if err != nil {
|
||||
t.Fatalf("err: %v", err)
|
||||
|
||||
require.Nil(err)
|
||||
}
|
||||
if len(pool.Subjects()) != 0 {
|
||||
t.Fatalf("bad: %v", pool.Subjects())
|
||||
|
||||
func TestConfig_AppendCA_Valid(t *testing.T) {
|
||||
require := require.New(t)
|
||||
|
||||
conf := &Config{
|
||||
CAFile: cacert,
|
||||
}
|
||||
pool := x509.NewCertPool()
|
||||
err := conf.AppendCA(pool)
|
||||
|
||||
require.Nil(err)
|
||||
}
|
||||
|
||||
func TestConfig_AppendCA_Valid_MultipleCerts(t *testing.T) {
|
||||
require := require.New(t)
|
||||
|
||||
tmpCAFile, err := ioutil.TempFile("/tmp", "test_ca_file")
|
||||
require.Nil(err)
|
||||
defer os.Remove(tmpCAFile.Name())
|
||||
|
||||
certs := `
|
||||
-----BEGIN CERTIFICATE-----
|
||||
MIICMzCCAdqgAwIBAgIUNZ9L86Xp9EuDH0/qyAesh599LXQwCgYIKoZIzj0EAwIw
|
||||
eDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNh
|
||||
biBGcmFuY2lzY28xEjAQBgNVBAoTCUhhc2hpQ29ycDEOMAwGA1UECxMFTm9tYWQx
|
||||
GDAWBgNVBAMTD25vbWFkLmhhc2hpY29ycDAeFw0xNjExMTAxOTQ4MDBaFw0yMTEx
|
||||
MDkxOTQ4MDBaMHgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYw
|
||||
FAYDVQQHEw1TYW4gRnJhbmNpc2NvMRIwEAYDVQQKEwlIYXNoaUNvcnAxDjAMBgNV
|
||||
BAsTBU5vbWFkMRgwFgYDVQQDEw9ub21hZC5oYXNoaWNvcnAwWTATBgcqhkjOPQIB
|
||||
BggqhkjOPQMBBwNCAARfJmTdHzYIMPD8SK+kj5Gc79fmpOcg6wnb4JNVwCqWw9O+
|
||||
uNdZJZWSi4Q/4HojM5FTSBqYxNgSrmY/o3oQrCPlo0IwQDAOBgNVHQ8BAf8EBAMC
|
||||
AQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUOjVq/BectnhcKn6EHUD4NJFm
|
||||
/UAwCgYIKoZIzj0EAwIDRwAwRAIgTemDJGSGtcQPXLWKiQNw4SKO9wAPhn/WoKW4
|
||||
Ln2ZUe8CIDsQswBQS7URbqnKYDye2Y4befJkr4fmhhmMQb2ex9A4
|
||||
-----END CERTIFICATE-----
|
||||
-----BEGIN CERTIFICATE-----
|
||||
MIICMzCCAdqgAwIBAgIUNZ9L86Xp9EuDH0/qyAesh599LXQwCgYIKoZIzj0EAwIw
|
||||
eDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNh
|
||||
biBGcmFuY2lzY28xEjAQBgNVBAoTCUhhc2hpQ29ycDEOMAwGA1UECxMFTm9tYWQx
|
||||
GDAWBgNVBAMTD25vbWFkLmhhc2hpY29ycDAeFw0xNjExMTAxOTQ4MDBaFw0yMTEx
|
||||
MDkxOTQ4MDBaMHgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYw
|
||||
FAYDVQQHEw1TYW4gRnJhbmNpc2NvMRIwEAYDVQQKEwlIYXNoaUNvcnAxDjAMBgNV
|
||||
BAsTBU5vbWFkMRgwFgYDVQQDEw9ub21hZC5oYXNoaWNvcnAwWTATBgcqhkjOPQIB
|
||||
BggqhkjOPQMBBwNCAARfJmTdHzYIMPD8SK+kj5Gc79fmpOcg6wnb4JNVwCqWw9O+
|
||||
uNdZJZWSi4Q/4HojM5FTSBqYxNgSrmY/o3oQrCPlo0IwQDAOBgNVHQ8BAf8EBAMC
|
||||
AQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUOjVq/BectnhcKn6EHUD4NJFm
|
||||
/UAwCgYIKoZIzj0EAwIDRwAwRAIgTemDJGSGtcQPXLWKiQNw4SKO9wAPhn/WoKW4
|
||||
Ln2ZUe8CIDsQswBQS7URbqnKYDye2Y4befJkr4fmhhmMQb2ex9A4
|
||||
-----END CERTIFICATE-----`
|
||||
|
||||
_, err = tmpCAFile.Write([]byte(certs))
|
||||
require.Nil(err)
|
||||
|
||||
conf := &Config{
|
||||
CAFile: tmpCAFile.Name(),
|
||||
}
|
||||
pool := x509.NewCertPool()
|
||||
err = conf.AppendCA(pool)
|
||||
|
||||
require.Nil(err)
|
||||
}
|
||||
|
||||
func TestConfig_AppendCA_InValid_MultipleCerts(t *testing.T) {
|
||||
require := require.New(t)
|
||||
|
||||
tmpCAFile, err := ioutil.TempFile("/tmp", "test_ca_file")
|
||||
require.Nil(err)
|
||||
defer os.Remove(tmpCAFile.Name())
|
||||
|
||||
certs := `
|
||||
-----BEGIN CERTIFICATE-----
|
||||
MIICMzCCAdqgAwIBAgIUNZ9L86Xp9EuDH0/qyAesh599LXQwCgYIKoZIzj0EAwIw
|
||||
eDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNh
|
||||
biBGcmFuY2lzY28xEjAQBgNVBAoTCUhhc2hpQ29ycDEOMAwGA1UECxMFTm9tYWQx
|
||||
GDAWBgNVBAMTD25vbWFkLmhhc2hpY29ycDAeFw0xNjExMTAxOTQ4MDBaFw0yMTEx
|
||||
MDkxOTQ4MDBaMHgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYw
|
||||
FAYDVQQHEw1TYW4gRnJhbmNpc2NvMRIwEAYDVQQKEwlIYXNoaUNvcnAxDjAMBgNV
|
||||
BAsTBU5vbWFkMRgwFgYDVQQDEw9ub21hZC5oYXNoaWNvcnAwWTATBgcqhkjOPQIB
|
||||
BggqhkjOPQMBBwNCAARfJmTdHzYIMPD8SK+kj5Gc79fmpOcg6wnb4JNVwCqWw9O+
|
||||
uNdZJZWSi4Q/4HojM5FTSBqYxNgSrmY/o3oQrCPlo0IwQDAOBgNVHQ8BAf8EBAMC
|
||||
AQYwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUOjVq/BectnhcKn6EHUD4NJFm
|
||||
/UAwCgYIKoZIzj0EAwIDRwAwRAIgTemDJGSGtcQPXLWKiQNw4SKO9wAPhn/WoKW4
|
||||
Ln2ZUe8CIDsQswBQS7URbqnKYDye2Y4befJkr4fmhhmMQb2ex9A4
|
||||
-----END CERTIFICATE-----
|
||||
-----BEGIN CERTIFICATE-----
|
||||
Invalid
|
||||
-----END CERTIFICATE-----`
|
||||
|
||||
_, err = tmpCAFile.Write([]byte(certs))
|
||||
require.Nil(err)
|
||||
|
||||
conf := &Config{
|
||||
CAFile: tmpCAFile.Name(),
|
||||
}
|
||||
pool := x509.NewCertPool()
|
||||
err = conf.AppendCA(pool)
|
||||
|
||||
require.NotNil(err)
|
||||
}
|
||||
|
||||
func TestConfig_AppendCA_Invalid(t *testing.T) {
|
||||
require := require.New(t)
|
||||
{
|
||||
conf := &Config{
|
||||
CAFile: "invalidFile",
|
||||
}
|
||||
pool := x509.NewCertPool()
|
||||
err := conf.AppendCA(pool)
|
||||
require.NotNil(err)
|
||||
require.Contains(err.Error(), "Failed to read CA file")
|
||||
require.Equal(len(pool.Subjects()), 0)
|
||||
}
|
||||
|
||||
{
|
||||
tmpFile, err := ioutil.TempFile("/tmp", "test_ca_file")
|
||||
require.Nil(err)
|
||||
defer os.Remove(tmpFile.Name())
|
||||
_, err = tmpFile.Write([]byte("Invalid CA Content!"))
|
||||
require.Nil(err)
|
||||
|
||||
conf := &Config{
|
||||
CAFile: tmpFile.Name(),
|
||||
}
|
||||
pool := x509.NewCertPool()
|
||||
err = conf.AppendCA(pool)
|
||||
require.NotNil(err)
|
||||
require.Contains(err.Error(), "Failed to decode CA file from pem format")
|
||||
require.Equal(len(pool.Subjects()), 0)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue