Check and complete intermediate SSL certificates

This commit is contained in:
Manuel Alejandro de Brito Fontes 2019-07-04 18:06:55 -04:00
parent edf2b03c22
commit 8807db9748
No known key found for this signature in database
GPG key ID: 786136016A8BA02A
13 changed files with 132 additions and 214 deletions

View file

@ -30,6 +30,13 @@ import (
"k8s.io/ingress-nginx/internal/runtime"
)
var (
// EnableSSLChainCompletion Autocomplete SSL certificate chains with missing intermediate CA certificates.
EnableSSLChainCompletion = false
// EnableDynamicCertificates Dynamically update SSL certificates instead of reloading NGINX
EnableDynamicCertificates = true
)
const (
// http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size
// Sets the maximum allowed size of the client request body
@ -755,25 +762,25 @@ func (cfg Configuration) BuildLogFormatUpstream() string {
// TemplateConfig contains the nginx configuration to render the file nginx.conf
type TemplateConfig struct {
ProxySetHeaders map[string]string
AddHeaders map[string]string
BacklogSize int
Backends []*ingress.Backend
PassthroughBackends []*ingress.SSLPassthroughBackend
Servers []*ingress.Server
TCPBackends []ingress.L4Service
UDPBackends []ingress.L4Service
HealthzURI string
Cfg Configuration
IsIPV6Enabled bool
IsSSLPassthroughEnabled bool
NginxStatusIpv4Whitelist []string
NginxStatusIpv6Whitelist []string
RedirectServers interface{}
ListenPorts *ListenPorts
PublishService *apiv1.Service
DynamicCertificatesEnabled bool
EnableMetrics bool
ProxySetHeaders map[string]string
AddHeaders map[string]string
BacklogSize int
Backends []*ingress.Backend
PassthroughBackends []*ingress.SSLPassthroughBackend
Servers []*ingress.Server
TCPBackends []ingress.L4Service
UDPBackends []ingress.L4Service
HealthzURI string
Cfg Configuration
IsIPV6Enabled bool
IsSSLPassthroughEnabled bool
NginxStatusIpv4Whitelist []string
NginxStatusIpv6Whitelist []string
RedirectServers interface{}
ListenPorts *ListenPorts
PublishService *apiv1.Service
EnableDynamicCertificates bool
EnableMetrics bool
PID string
StatusSocket string

View file

@ -84,14 +84,10 @@ type Configuration struct {
EnableMetrics bool
MetricsPerHost bool
EnableSSLChainCompletion bool
FakeCertificate *ingress.SSLCert
SyncRateLimit float32
DynamicCertificatesEnabled bool
DisableCatchAll bool
ValidationWebhook string
@ -171,7 +167,7 @@ func (n *NGINXController) syncIngress(interface{}) error {
}
err := wait.ExponentialBackoff(retry, func() (bool, error) {
err := configureDynamically(pcfg, n.cfg.DynamicCertificatesEnabled)
err := configureDynamically(pcfg)
if err == nil {
klog.V(2).Infof("Dynamic reconfiguration succeeded.")
return true, nil
@ -890,7 +886,7 @@ func (n *NGINXController) serviceEndpoints(svcKey, backendPort string) ([]ingres
return upstreams, nil
}
// overridePemFileNameAndPemSHA should only be called when DynamicCertificatesEnabled
// overridePemFileNameAndPemSHA should only be called when EnableDynamicCertificates
// ideally this function should not exist, the only reason why we use it is that
// we rely on PemFileName in nginx.tmpl to configure SSL directives
// and PemSHA to force reload
@ -940,7 +936,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
certificate, err := n.store.GetLocalSSLCert(n.cfg.DefaultSSLCertificate)
if err == nil {
defaultCertificate = certificate
if n.cfg.DynamicCertificatesEnabled {
if ngx_config.EnableDynamicCertificates {
n.overridePemFileNameAndPemSHA(defaultCertificate)
}
} else {
@ -1123,7 +1119,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
}
}
if n.cfg.DynamicCertificatesEnabled {
if ngx_config.EnableDynamicCertificates {
n.overridePemFileNameAndPemSHA(cert)
}

View file

@ -1116,7 +1116,7 @@ func newNGINXController(t *testing.T) *NGINXController {
t.Fatalf("error: %v", err)
}
storer := store.New(true,
storer := store.New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
@ -1126,7 +1126,6 @@ func newNGINXController(t *testing.T) *NGINXController {
clientSet,
fs,
channels.NewRingChannel(10),
false,
pod,
false)

View file

@ -129,7 +129,6 @@ func NewNGINXController(config *Configuration, mc metric.Collector, fs file.File
n.podInfo = pod
n.store = store.New(
config.EnableSSLChainCompletion,
config.Namespace,
config.ConfigMapName,
config.TCPConfigMapName,
@ -139,7 +138,6 @@ func NewNGINXController(config *Configuration, mc metric.Collector, fs file.File
config.Client,
fs,
n.updateCh,
config.DynamicCertificatesEnabled,
pod,
config.DisableCatchAll)
@ -598,24 +596,24 @@ func (n NGINXController) generateTemplate(cfg ngx_config.Configuration, ingressC
cfg.SSLDHParam = sslDHParam
tc := ngx_config.TemplateConfig{
ProxySetHeaders: setHeaders,
AddHeaders: addHeaders,
BacklogSize: sysctlSomaxconn(),
Backends: ingressCfg.Backends,
PassthroughBackends: ingressCfg.PassthroughBackends,
Servers: ingressCfg.Servers,
TCPBackends: ingressCfg.TCPEndpoints,
UDPBackends: ingressCfg.UDPEndpoints,
Cfg: cfg,
IsIPV6Enabled: n.isIPV6Enabled && !cfg.DisableIpv6,
NginxStatusIpv4Whitelist: cfg.NginxStatusIpv4Whitelist,
NginxStatusIpv6Whitelist: cfg.NginxStatusIpv6Whitelist,
RedirectServers: buildRedirects(ingressCfg.Servers),
IsSSLPassthroughEnabled: n.cfg.EnableSSLPassthrough,
ListenPorts: n.cfg.ListenPorts,
PublishService: n.GetPublishService(),
DynamicCertificatesEnabled: n.cfg.DynamicCertificatesEnabled,
EnableMetrics: n.cfg.EnableMetrics,
ProxySetHeaders: setHeaders,
AddHeaders: addHeaders,
BacklogSize: sysctlSomaxconn(),
Backends: ingressCfg.Backends,
PassthroughBackends: ingressCfg.PassthroughBackends,
Servers: ingressCfg.Servers,
TCPBackends: ingressCfg.TCPEndpoints,
UDPBackends: ingressCfg.UDPEndpoints,
Cfg: cfg,
IsIPV6Enabled: n.isIPV6Enabled && !cfg.DisableIpv6,
NginxStatusIpv4Whitelist: cfg.NginxStatusIpv4Whitelist,
NginxStatusIpv6Whitelist: cfg.NginxStatusIpv6Whitelist,
RedirectServers: buildRedirects(ingressCfg.Servers),
IsSSLPassthroughEnabled: n.cfg.EnableSSLPassthrough,
ListenPorts: n.cfg.ListenPorts,
PublishService: n.GetPublishService(),
EnableDynamicCertificates: ngx_config.EnableDynamicCertificates,
EnableMetrics: n.cfg.EnableMetrics,
HealthzURI: nginx.HealthPath,
PID: nginx.PID,
@ -851,7 +849,7 @@ func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configurati
copyOfRunningConfig.ControllerPodsCount = 0
copyOfPcfg.ControllerPodsCount = 0
if n.cfg.DynamicCertificatesEnabled {
if ngx_config.EnableDynamicCertificates {
clearCertificates(&copyOfRunningConfig)
clearCertificates(&copyOfPcfg)
}
@ -861,7 +859,7 @@ func (n *NGINXController) IsDynamicConfigurationEnough(pcfg *ingress.Configurati
// configureDynamically encodes new Backends in JSON format and POSTs the
// payload to an internal HTTP endpoint handled by Lua.
func configureDynamically(pcfg *ingress.Configuration, isDynamicCertificatesEnabled bool) error {
func configureDynamically(pcfg *ingress.Configuration) error {
backends := make([]*ingress.Backend, len(pcfg.Backends))
for i, backend := range pcfg.Backends {
@ -949,7 +947,7 @@ func configureDynamically(pcfg *ingress.Configuration, isDynamicCertificatesEnab
return fmt.Errorf("unexpected error code: %d", statusCode)
}
if isDynamicCertificatesEnabled {
if ngx_config.EnableDynamicCertificates {
err = configureCertificates(pcfg)
if err != nil {
return err

View file

@ -32,10 +32,14 @@ import (
apiv1 "k8s.io/api/core/v1"
"k8s.io/ingress-nginx/internal/ingress"
ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
"k8s.io/ingress-nginx/internal/nginx"
)
func TestIsDynamicConfigurationEnough(t *testing.T) {
ngx_config.EnableDynamicCertificates = false
defer func() { ngx_config.EnableDynamicCertificates = true }()
backends := []*ingress.Backend{{
Name: "fakenamespace-myapp-80",
Endpoints: []ingress.Endpoint{
@ -73,9 +77,7 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
Backends: backends,
Servers: servers,
},
cfg: &Configuration{
DynamicCertificatesEnabled: false,
},
cfg: &Configuration{},
}
newConfig := commonConfig
@ -95,12 +97,13 @@ func TestIsDynamicConfigurationEnough(t *testing.T) {
Backends: []*ingress.Backend{{Name: "a-backend-8080"}},
Servers: servers,
}
ngx_config.EnableDynamicCertificates = true
if !n.IsDynamicConfigurationEnough(newConfig) {
t.Errorf("Expected to be dynamically configurable when only backends change")
}
n.cfg.DynamicCertificatesEnabled = true
newServers := []*ingress.Server{{
Hostname: "myapp1.fake",
Locations: []*ingress.Location{
@ -243,7 +246,10 @@ func TestConfigureDynamically(t *testing.T) {
ControllerPodsCount: 2,
}
err = configureDynamically(commonConfig, false)
ngx_config.EnableDynamicCertificates = false
defer func() { ngx_config.EnableDynamicCertificates = true }()
err = configureDynamically(commonConfig)
if err != nil {
t.Errorf("unexpected error posting dynamic configuration: %v", err)
}

View file

@ -20,16 +20,14 @@ import (
"fmt"
"strings"
"github.com/imdario/mergo"
"k8s.io/klog"
apiv1 "k8s.io/api/core/v1"
networking "k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-nginx/internal/file"
"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/k8s"
ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
"k8s.io/ingress-nginx/internal/net/ssl"
)
@ -103,7 +101,7 @@ func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error
return nil, fmt.Errorf("unexpected error creating SSL Cert: %v", err)
}
if !s.isDynamicCertificatesEnabled || len(ca) > 0 {
if !ngx_config.EnableDynamicCertificates || len(ca) > 0 {
err = ssl.StoreSSLCertOnDisk(s.filesystem, nsSecName, sslCert)
if err != nil {
return nil, fmt.Errorf("error while storing certificate and key: %v", err)
@ -152,57 +150,6 @@ func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error
return sslCert, nil
}
func (s *k8sStore) checkSSLChainIssues() {
for _, item := range s.ListLocalSSLCerts() {
secrKey := k8s.MetaNamespaceKey(item)
secret, err := s.GetLocalSSLCert(secrKey)
if err != nil {
continue
}
if secret.FullChainPemFileName != "" {
// chain already checked
continue
}
data, err := ssl.FullChainCert(secret.PemFileName, s.filesystem)
if err != nil {
klog.Errorf("Error generating CA certificate chain for Secret %q: %v", secrKey, err)
continue
}
fullChainPemFileName := fmt.Sprintf("%v/%v-%v-full-chain.pem", file.DefaultSSLDirectory, secret.Namespace, secret.Name)
file, err := s.filesystem.Create(fullChainPemFileName)
if err != nil {
klog.Errorf("Error creating SSL certificate file for Secret %q: %v", secrKey, err)
continue
}
_, err = file.Write(data)
if err != nil {
klog.Errorf("Error creating SSL certificate for Secret %q: %v", secrKey, err)
continue
}
dst := &ingress.SSLCert{}
err = mergo.MergeWithOverwrite(dst, secret)
if err != nil {
klog.Errorf("Error creating SSL certificate for Secret %q: %v", secrKey, err)
continue
}
dst.FullChainPemFileName = fullChainPemFileName
klog.Infof("Updating local copy of SSL certificate %q with missing intermediate CA certs", secrKey)
s.sslStore.Update(secrKey, dst)
// this update must trigger an update
// (like an update event from a change in Ingress)
s.sendDummyEvent()
}
}
// sendDummyEvent sends a dummy event to trigger an update
// This is used in when a secret change
func (s *k8sStore) sendDummyEvent() {

View file

@ -35,7 +35,6 @@ import (
"k8s.io/apimachinery/pkg/labels"
k8sruntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/informers"
clientset "k8s.io/client-go/kubernetes"
@ -187,8 +186,6 @@ func (i *Informer) Run(stopCh chan struct{}) {
// k8sStore internal Storer implementation using informers and thread safe stores
type k8sStore struct {
isOCSPCheckEnabled bool
// backendConfig contains the running configuration from the configmap
// this is required because this rarely changes but is a very expensive
// operation to execute in each OnUpdate invocation
@ -224,36 +221,31 @@ type k8sStore struct {
defaultSSLCertificate string
isDynamicCertificatesEnabled bool
pod *k8s.PodInfo
}
// New creates a new object store to be used in the ingress controller
func New(checkOCSP bool,
func New(
namespace, configmap, tcp, udp, defaultSSLCertificate string,
resyncPeriod time.Duration,
client clientset.Interface,
fs file.Filesystem,
updateCh *channels.RingChannel,
isDynamicCertificatesEnabled bool,
pod *k8s.PodInfo,
disableCatchAll bool) Storer {
store := &k8sStore{
isOCSPCheckEnabled: checkOCSP,
informers: &Informer{},
listers: &Lister{},
sslStore: NewSSLCertTracker(),
filesystem: fs,
updateCh: updateCh,
backendConfig: ngx_config.NewDefault(),
syncSecretMu: &sync.Mutex{},
backendConfigMu: &sync.RWMutex{},
secretIngressMap: NewObjectRefMap(),
defaultSSLCertificate: defaultSSLCertificate,
isDynamicCertificatesEnabled: isDynamicCertificatesEnabled,
pod: pod,
informers: &Informer{},
listers: &Lister{},
sslStore: NewSSLCertTracker(),
filesystem: fs,
updateCh: updateCh,
backendConfig: ngx_config.NewDefault(),
syncSecretMu: &sync.Mutex{},
backendConfigMu: &sync.RWMutex{},
secretIngressMap: NewObjectRefMap(),
defaultSSLCertificate: defaultSSLCertificate,
pod: pod,
}
eventBroadcaster := record.NewBroadcaster()
@ -878,10 +870,6 @@ func (s *k8sStore) setConfig(cmap *corev1.ConfigMap) {
func (s *k8sStore) Run(stopCh chan struct{}) {
// start informers
s.informers.Run(stopCh)
if s.isOCSPCheckEnabled {
go wait.Until(s.checkSSLChainIssues, 60*time.Second, stopCh)
}
}
// GetRunningControllerPodsCount returns the number of Running ingress-nginx controller Pods

View file

@ -41,6 +41,7 @@ import (
"k8s.io/ingress-nginx/internal/file"
"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
"k8s.io/ingress-nginx/internal/k8s"
"k8s.io/ingress-nginx/test/e2e/framework"
)
@ -87,7 +88,7 @@ func TestStore(t *testing.T) {
}(updateCh)
fs := newFS(t)
storer := New(true,
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
@ -97,7 +98,6 @@ func TestStore(t *testing.T) {
clientSet,
fs,
updateCh,
false,
pod,
false)
@ -168,7 +168,7 @@ func TestStore(t *testing.T) {
}(updateCh)
fs := newFS(t)
storer := New(true,
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
@ -178,7 +178,6 @@ func TestStore(t *testing.T) {
clientSet,
fs,
updateCh,
false,
pod,
false)
@ -319,7 +318,7 @@ func TestStore(t *testing.T) {
}(updateCh)
fs := newFS(t)
storer := New(true,
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
@ -329,7 +328,6 @@ func TestStore(t *testing.T) {
clientSet,
fs,
updateCh,
false,
pod,
false)
@ -426,7 +424,7 @@ func TestStore(t *testing.T) {
}(updateCh)
fs := newFS(t)
storer := New(true,
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
@ -436,7 +434,6 @@ func TestStore(t *testing.T) {
clientSet,
fs,
updateCh,
false,
pod,
false)
@ -516,7 +513,7 @@ func TestStore(t *testing.T) {
}(updateCh)
fs := newFS(t)
storer := New(true,
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
@ -526,7 +523,6 @@ func TestStore(t *testing.T) {
clientSet,
fs,
updateCh,
false,
pod,
false)
@ -628,7 +624,7 @@ func TestStore(t *testing.T) {
}(updateCh)
fs := newFS(t)
storer := New(true,
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
@ -638,7 +634,6 @@ func TestStore(t *testing.T) {
clientSet,
fs,
updateCh,
false,
pod,
false)
@ -708,6 +703,9 @@ func TestStore(t *testing.T) {
}
t.Run("should exists a secret in the local store and filesystem", func(t *testing.T) {
ngx_config.EnableDynamicCertificates = false
defer func() { ngx_config.EnableDynamicCertificates = true }()
err := framework.WaitForSecretInNamespace(clientSet, ns, name)
if err != nil {
t.Errorf("error waiting for secret: %v", err)

View file

@ -32,9 +32,6 @@ type SSLCert struct {
CAFileName string `json:"caFileName"`
// PemFileName contains the path to the file with the certificate and key concatenated
PemFileName string `json:"pemFileName"`
// FullChainPemFileName contains the path to the file with the certificate and key concatenated
// This certificate contains the full chain (ca + intermediates + cert)
FullChainPemFileName string `json:"fullChainPemFileName"`
// PemSHA contains the sha1 of the pem file.
// This is used to detect changes in the secret that contains the certificates
PemSHA string `json:"pemSha"`

View file

@ -523,9 +523,6 @@ func (s1 *SSLCert) Equal(s2 *SSLCert) bool {
if !s1.ExpireTime.Equal(s2.ExpireTime) {
return false
}
if s1.FullChainPemFileName != s2.FullChainPemFileName {
return false
}
if s1.PemCertKey != s2.PemCertKey {
return false
}

View file

@ -38,6 +38,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/ingress-nginx/internal/file"
"k8s.io/ingress-nginx/internal/ingress"
ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config"
"k8s.io/ingress-nginx/internal/watch"
"k8s.io/klog"
)
@ -74,8 +75,18 @@ func verifyPemCertAgainstRootCA(pemCert *x509.Certificate, ca []byte) error {
// CreateSSLCert validates cert and key, extracts common names and returns corresponding SSLCert object
func CreateSSLCert(cert, key []byte) (*ingress.SSLCert, error) {
var pemCertBuffer bytes.Buffer
pemCertBuffer.Write(cert)
if ngx_config.EnableSSLChainCompletion {
data, err := fullChainCert(cert)
if err != nil {
klog.Errorf("Error generating certificate chain for Secret: %v", err)
} else {
pemCertBuffer.Reset()
pemCertBuffer.Write(data)
}
}
pemCertBuffer.Write([]byte("\n"))
pemCertBuffer.Write(key)
@ -376,7 +387,6 @@ func GetFakeSSLCert(fs file.Filesystem) *ingress.SSLCert {
}
func getFakeHostSSLCert(host string) ([]byte, []byte) {
var priv interface{}
var err error
@ -423,16 +433,11 @@ func getFakeHostSSLCert(host string) ([]byte, []byte) {
return cert, key
}
// FullChainCert checks if a certificate file contains issues in the intermediate CA chain
// fullChainCert checks if a certificate file contains issues in the intermediate CA chain
// Returns a new certificate with the intermediate certificates.
// If the certificate does not contains issues with the chain it return an empty byte array
func FullChainCert(in string, fs file.Filesystem) ([]byte, error) {
data, err := fs.ReadFile(in)
if err != nil {
return nil, err
}
cert, err := certUtil.DecodeCertificate(data)
func fullChainCert(in []byte) ([]byte, error) {
cert, err := certUtil.DecodeCertificate(in)
if err != nil {
return nil, err
}
@ -452,11 +457,6 @@ func FullChainCert(in string, fs file.Filesystem) ([]byte, error) {
return nil, err
}
certs, err = certUtil.AddRootCA(certs)
if err != nil {
return nil, err
}
return certUtil.EncodeCertificates(certs), nil
}