diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go index e938c008b..7f3b8285e 100644 --- a/internal/ingress/controller/config/config.go +++ b/internal/ingress/controller/config/config.go @@ -721,7 +721,7 @@ type TemplateConfig struct { IsSSLPassthroughEnabled bool NginxStatusIpv4Whitelist []string NginxStatusIpv6Whitelist []string - RedirectServers map[string]string + RedirectServers interface{} ListenPorts *ListenPorts PublishService *apiv1.Service DynamicCertificatesEnabled bool diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go index eb018cf11..6ce3fb3cd 100644 --- a/internal/ingress/controller/nginx.go +++ b/internal/ingress/controller/nginx.go @@ -38,6 +38,7 @@ import ( "github.com/eapache/channels" apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/scheme" v1core "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/record" @@ -498,39 +499,20 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { // https://trac.nginx.org/nginx/ticket/631 var longestName int var serverNameBytes int - redirectServers := make(map[string]string) + for _, srv := range ingressCfg.Servers { if longestName < len(srv.Hostname) { longestName = len(srv.Hostname) } serverNameBytes += len(srv.Hostname) - if srv.RedirectFromToWWW { - var n string - if strings.HasPrefix(srv.Hostname, "www.") { - n = strings.TrimPrefix(srv.Hostname, "www.") - } else { - n = fmt.Sprintf("www.%v", srv.Hostname) - } - klog.V(3).Infof("Creating redirect from %q to %q", srv.Hostname, n) - if _, ok := redirectServers[n]; !ok { - found := false - for _, esrv := range ingressCfg.Servers { - if esrv.Hostname == n { - found = true - break - } - } - if !found { - redirectServers[n] = srv.Hostname - } - } - } } + if cfg.ServerNameHashBucketSize == 0 { nameHashBucketSize := nginxHashBucketSize(longestName) klog.V(3).Infof("Adjusting ServerNameHashBucketSize variable to %d", nameHashBucketSize) cfg.ServerNameHashBucketSize = nameHashBucketSize } + serverNameHashMaxSize := nextPowerOf2(serverNameBytes) if cfg.ServerNameHashMaxSize < serverNameHashMaxSize { klog.V(3).Infof("Adjusting ServerNameHashMaxSize variable to %d", serverNameHashMaxSize) @@ -619,7 +601,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { IsIPV6Enabled: n.isIPV6Enabled && !cfg.DisableIpv6, NginxStatusIpv4Whitelist: cfg.NginxStatusIpv4Whitelist, NginxStatusIpv6Whitelist: cfg.NginxStatusIpv6Whitelist, - RedirectServers: redirectServers, + RedirectServers: buildRedirects(ingressCfg.Servers), IsSSLPassthroughEnabled: n.cfg.EnableSSLPassthrough, ListenPorts: n.cfg.ListenPorts, PublishService: n.GetPublishService(), @@ -1022,3 +1004,65 @@ func cleanTempNginxCfg() error { return nil } + +type redirect struct { + From string + To string + SSLCert ingress.SSLCert +} + +func buildRedirects(servers []*ingress.Server) []*redirect { + names := sets.String{} + redirectServers := make([]*redirect, 0) + + for _, srv := range servers { + if !srv.RedirectFromToWWW { + continue + } + + to := srv.Hostname + + var from string + if strings.HasPrefix(to, "www.") { + from = strings.TrimPrefix(to, "www.") + } else { + from = fmt.Sprintf("www.%v", to) + } + + if names.Has(to) { + continue + } + + klog.V(3).Infof("Creating redirect from %q to %q", from, to) + found := false + for _, esrv := range servers { + if esrv.Hostname == from { + found = true + break + } + } + + if found { + klog.Warningf("Already exists an Ingress with %q hostname. Skipping creation of redirection from %q to %q.", from, from, to) + continue + } + + r := &redirect{ + From: from, + To: to, + } + + if srv.SSLCert.PemSHA != "" { + if ssl.IsValidHostname(from, srv.SSLCert.CN) { + r.SSLCert = srv.SSLCert + } else { + klog.Warningf("the server %v has SSL configured but the SSL certificate does not contains a CN for %v. Redirects will not work for HTTPS to HTTPS", from, to) + } + } + + redirectServers = append(redirectServers, r) + names.Insert(to) + } + + return redirectServers +} diff --git a/internal/net/ssl/ssl.go b/internal/net/ssl/ssl.go index 4261342e9..cdcf6f4f0 100644 --- a/internal/net/ssl/ssl.go +++ b/internal/net/ssl/ssl.go @@ -30,6 +30,7 @@ import ( "math/big" "net" "strconv" + "strings" "time" "github.com/zakjan/cert-chain-resolver/certUtil" @@ -508,3 +509,21 @@ func FullChainCert(in string, fs file.Filesystem) ([]byte, error) { return certUtil.EncodeCertificates(certs), nil } + +// IsValidHostname checks if a hostname is valid in a list of common names +func IsValidHostname(hostname string, commonNames []string) bool { + for _, cn := range commonNames { + if strings.EqualFold(hostname, cn) { + return true + } + + labels := strings.Split(hostname, ".") + labels[0] = "*" + candidate := strings.Join(labels, ".") + if strings.EqualFold(candidate, cn) { + return true + } + } + + return false +} diff --git a/internal/net/ssl/ssl_test.go b/internal/net/ssl/ssl_test.go index 121b55411..04c7018fd 100644 --- a/internal/net/ssl/ssl_test.go +++ b/internal/net/ssl/ssl_test.go @@ -205,3 +205,39 @@ func newCA(name string) (*keyPair, error) { Cert: cert, }, nil } + +func TestIsValidHostname(t *testing.T) { + cases := map[string]struct { + Hostname string + CN []string + Valid bool + }{ + "when there is no common names": { + "foo.bar", + []string{}, + false, + }, + "when there is a match for foo.bar": { + "foo.bar", + []string{"foo.bar"}, + true, + }, + "when there is a wildcard match for foo.bar": { + "foo.bar", + []string{"*.bar"}, + true, + }, + "when there is a wrong wildcard for *.bar": { + "invalid.foo.bar", + []string{"*.bar"}, + false, + }, + } + + for k, tc := range cases { + valid := IsValidHostname(tc.Hostname, tc.CN) + if valid != tc.Valid { + t.Errorf("%s: expected '%v' but returned %v", k, tc.Valid, valid) + } + } +} diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 381098dbe..8bee4bf09 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -520,7 +520,8 @@ http { {{ end }} {{/* Build server redirects (from/to www) */}} - {{ range $hostname, $to := .RedirectServers }} + {{ range $redirect := .RedirectServers }} + ## start server {{ $redirect.From }} server { {{ range $address := $all.Cfg.BindAddressIpv4 }} listen {{ $address }}:{{ $all.ListenPorts.HTTP }}{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}; @@ -538,7 +539,25 @@ http { listen [::]:{{ if $all.IsSSLPassthroughEnabled }}{{ $all.ListenPorts.SSLProxy }} proxy_protocol{{ else }}{{ $all.ListenPorts.HTTPS }}{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}{{ end }}; {{ end }} {{ end }} - server_name {{ $hostname }}; + server_name {{ $redirect.From }}; + + {{ if not (empty $redirect.SSLCert.PemFileName) }} + {{/* comment PEM sha is required to detect changes in the generated configuration and force a reload */}} + # PEM sha: {{ $redirect.SSLCert.PemSHA }} + ssl_certificate {{ $redirect.SSLCert.PemFileName }}; + ssl_certificate_key {{ $redirect.SSLCert.PemFileName }}; + {{ if not (empty $redirect.SSLCert.FullChainPemFileName)}} + ssl_trusted_certificate {{ $redirect.SSLCert.FullChainPemFileName }}; + ssl_stapling on; + ssl_stapling_verify on; + {{ end }} + + {{ if $all.DynamicCertificatesEnabled}} + ssl_certificate_by_lua_block { + certificate.call() + } + {{ end }} + {{ end }} {{ if gt (len $cfg.BlockUserAgents) 0 }} if ($block_ua) { @@ -553,11 +572,12 @@ http { {{ if ne $all.ListenPorts.HTTPS 443 }} {{ $redirect_port := (printf ":%v" $all.ListenPorts.HTTPS) }} - return {{ $all.Cfg.HTTPRedirectCode }} $scheme://{{ $to }}{{ $redirect_port }}$request_uri; + return {{ $all.Cfg.HTTPRedirectCode }} $scheme://{{ $redirect.To }}{{ $redirect_port }}$request_uri; {{ else }} - return {{ $all.Cfg.HTTPRedirectCode }} $scheme://{{ $to }}$request_uri; + return {{ $all.Cfg.HTTPRedirectCode }} $scheme://{{ $redirect.To }}$request_uri; {{ end }} } + ## end server {{ $redirect.From }} {{ end }} {{ range $server := $servers }} diff --git a/test/e2e/annotations/fromtowwwredirect.go b/test/e2e/annotations/fromtowwwredirect.go index d5bc376c8..4aaa5909b 100644 --- a/test/e2e/annotations/fromtowwwredirect.go +++ b/test/e2e/annotations/fromtowwwredirect.go @@ -17,6 +17,7 @@ limitations under the License. package annotations import ( + "crypto/tls" "fmt" "net/http" "time" @@ -24,20 +25,21 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/parnurzeal/gorequest" + "k8s.io/ingress-nginx/test/e2e/framework" ) -var _ = framework.IngressNginxDescribe("Annotations - Fromtowwwredirect", func() { +var _ = framework.IngressNginxDescribe("Annotations - from-to-www-redirect", func() { f := framework.NewDefaultFramework("fromtowwwredirect") BeforeEach(func() { - f.NewEchoDeploymentWithReplicas(2) + f.NewEchoDeploymentWithReplicas(1) }) AfterEach(func() { }) - It("should redirect from www", func() { + It("should redirect from www HTTP to HTTP", func() { By("setting up server for redirect from www") host := "fromtowwwredirect.bar.com" @@ -67,4 +69,48 @@ var _ = framework.IngressNginxDescribe("Annotations - Fromtowwwredirect", func() Expect(resp.StatusCode).Should(Equal(http.StatusPermanentRedirect)) Expect(resp.Header.Get("Location")).Should(Equal("http://fromtowwwredirect.bar.com/foo")) }) + + It("should redirect from www HTTPS to HTTPS", func() { + By("setting up server for redirect from www") + host := "fromtowwwredirect.bar.com" + + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/from-to-www-redirect": "true", + } + + ing := framework.NewSingleIngressWithTLS(host, "/", host, []string{host, fmt.Sprintf("www.%v", host)}, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + _, err := framework.CreateIngressTLSSecret(f.KubeClientSet, + ing.Spec.TLS[0].Hosts, + ing.Spec.TLS[0].SecretName, + ing.Namespace) + Expect(err).ToNot(HaveOccurred()) + + f.WaitForNginxServer(fmt.Sprintf("www.%v", host), + func(server string) bool { + return Expect(server).Should(ContainSubstring(`server_name www.fromtowwwredirect.bar.com;`)) && + Expect(server).Should(ContainSubstring(fmt.Sprintf("/etc/ingress-controller/ssl/%v-fromtowwwredirect.bar.com.pem", f.IngressController.Namespace))) && + Expect(server).Should(ContainSubstring(`return 308 $scheme://fromtowwwredirect.bar.com$request_uri;`)) + }) + + By("sending request to www.fromtowwwredirect.bar.com") + + h := fmt.Sprintf("%s.%s", "www", host) + + resp, _, errs := gorequest.New(). + TLSClientConfig(&tls.Config{ + InsecureSkipVerify: true, + ServerName: h, + }). + Get(f.IngressController.HTTPSURL). + Retry(10, 1*time.Second, http.StatusNotFound). + RedirectPolicy(noRedirectPolicyFunc). + Set("host", h). + End() + + Expect(errs).Should(BeEmpty()) + Expect(resp.StatusCode).Should(Equal(http.StatusPermanentRedirect)) + Expect(resp.Header.Get("Location")).Should(Equal("https://fromtowwwredirect.bar.com/")) + }) })