fix: restrict autossl hosts

Makefile

@@ -55,10 +55,10 @@ coverage-server: build-server build-ssr ## Run all server tests (with code cover
 watch:
 	make -j4 watch-server watch-ui
 
-watch-server: build-server migrate ## Build and run server in watch mode
+watch-server: migrate ## Build and run server in watch mode
 	air -c air.conf
 
-watch-ui: clean ## Build and run server in watch mode
+watch-ui: ## Build and run server in watch mode
 	npx webpack-cli -w
 
 

app/pkg/web/autocert_test.go

@@ -1,34 +1,47 @@
 package web
 
 import (
+	"context"
 	"crypto/tls"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/getfider/fider/app/models/query"
 	"github.com/getfider/fider/app/pkg/bus"
 	"github.com/getfider/fider/app/services/blob/fs"
 
 	. "github.com/getfider/fider/app/pkg/assert"
 )
 
-func Test_UseAutoCert(t *testing.T) {
+func mockIsCNAMEAvailable(ctx context.Context, q *query.IsCNAMEAvailable) error {
+	if q.CNAME == "ideas.app.com" || q.CNAME == "feedback.mysite.com" {
+		q.Result = false
+	} else {
+		q.Result = true
+	}
+	return nil
+}
+
+func TestUseAutoCert_WhenCNAMEAreRegistered(t *testing.T) {
 	RegisterT(t)
 	bus.Init(fs.Service{})
+	bus.AddHandler(mockIsCNAMEAvailable)
 
 	manager, err := NewCertificateManager("", "")
 	Expect(err).IsNil()
 
-	invalidServerNames := []string{"ideas.app.com", "feedback.mysite.com"}
+	invalidServerNames := []string{"ideas.app.com", "FEEDBACK.mysite.COM"}
 
 	for _, serverName := range invalidServerNames {
 		cert, err := manager.GetCertificate(&tls.ClientHelloInfo{
 			ServerName: serverName,
 		})
 		Expect(err.Error()).ContainsSubstring(`acme/autocert: unable to satisfy`)
-		Expect(err.Error()).ContainsSubstring(`for domain "` + serverName + `": no viable challenge type found`)
+		Expect(err.Error()).ContainsSubstring(`for domain "` + strings.ToLower(serverName) + `": no viable challenge type found`)
 		Expect(cert).IsNil()
 	}
 
 	// GetCertificate starts a fire and forget go routine to delete items from cache, give it 2sec to complete it
-	time.Sleep(2*time.Second)
+	time.Sleep(2 * time.Second)
 }

app/pkg/web/ssl.go

@@ -1,13 +1,19 @@
 package web
 
 import (
+	"context"
 	"crypto/tls"
 	"crypto/x509"
 	"net/http"
 	"strings"
 
 	"golang.org/x/crypto/acme"
+	"golang.org/x/net/idna"
 
+	"github.com/getfider/fider/app"
+	"github.com/getfider/fider/app/models/query"
+	"github.com/getfider/fider/app/pkg/bus"
+	"github.com/getfider/fider/app/pkg/dbx"
 	"github.com/getfider/fider/app/pkg/env"
 	"github.com/getfider/fider/app/pkg/errors"
 	"golang.org/x/crypto/acme/autocert"
@@ -17,16 +23,46 @@ func getDefaultTLSConfig() *tls.Config {
 	return &tls.Config{
 		MinVersion: tls.VersionTLS12,
 		CipherSuites: []uint16{
-				tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
-				tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
-				tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
-				tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
-				tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
-				tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
+			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+			tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+			tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+			tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
+			tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
 		},
 	}
 }
 
+func isValidHostName(ctx context.Context, host string) error {
+	if host == "" {
+		return errors.New("host cannot be empty.")
+	}
+
+	if env.IsSingleHostMode() {
+		if env.Config.HostDomain == host {
+			return nil
+		}
+		return errors.New("server name mismatch")
+	}
+
+	trx, err := dbx.BeginTx(ctx)
+	if err != nil {
+		return errors.Wrap(err, "failed start new transaction")
+	}
+	defer trx.MustCommit()
+
+	isAvailable := &query.IsCNAMEAvailable{CNAME: host}
+	newCtx := context.WithValue(ctx, app.TransactionCtxKey, trx)
+	if err := bus.Dispatch(newCtx, isAvailable); err != nil {
+		return errors.Wrap(err, "failed to find tenant by cname")
+	}
+
+	if isAvailable.Result {
+		return errors.New("no tenants found with cname %s", host)
+	}
+	return nil
+}
+
 //CertificateManager is used to manage SSL certificates
 type CertificateManager struct {
 	cert    tls.Certificate
@@ -38,9 +74,10 @@ type CertificateManager struct {
 func NewCertificateManager(certFile, keyFile string) (*CertificateManager, error) {
 	manager := &CertificateManager{
 		autossl: autocert.Manager{
-			Prompt: autocert.AcceptTOS,
-			Cache:  NewAutoCertCache(),
-			Client: acmeClient(),
+			Prompt:     autocert.AcceptTOS,
+			Cache:      NewAutoCertCache(),
+			Client:     acmeClient(),
+			HostPolicy: isValidHostName,
 		},
 	}
 
@@ -65,7 +102,11 @@ func NewCertificateManager(certFile, keyFile string) (*CertificateManager, error
 //Otherwise fallsback to a automatically generated certificate by Let's Encrypt
 func (m *CertificateManager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, error) {
 	if m.leaf != nil {
-		serverName := strings.Trim(strings.ToLower(hello.ServerName), ".")
+		serverName, err := idna.Lookup.ToASCII(hello.ServerName)
+		if err != nil {
+			return nil, err
+		}
+		serverName = strings.Trim(serverName, ".")
 
 		// If ServerName is empty or does't contain a dot, just return the certificate
 		if serverName == "" || !strings.Contains(serverName, ".") {
@@ -75,6 +116,13 @@ func (m *CertificateManager) GetCertificate(hello *tls.ClientHelloInfo) (*tls.Ce
 		if env.IsSingleHostMode() || m.leaf.VerifyHostname(serverName) == nil {
 			return &m.cert, nil
 		}
+
+		// throw an error if it doesn't match the leaf certificate but still ends with current hostname, example:
+		// hostdomain is myserver.com and the certificate is *.myserver.com
+		// serverName is something.else.myserver.com, it should throw an error
+		if strings.HasSuffix(serverName, "."+env.Config.HostDomain) {
+			return nil, errors.New("invalid ServerName used: %s", serverName)
+		}
 	}
 
 	return m.autossl.GetCertificate(hello)

app/pkg/web/ssl_test.go

@@ -3,13 +3,16 @@ package web
 import (
 	"crypto/tls"
 	"testing"
+	"time"
 
+	"github.com/getfider/fider/app/pkg/bus"
 	"github.com/getfider/fider/app/pkg/env"
+	"github.com/getfider/fider/app/services/blob/fs"
 
 	. "github.com/getfider/fider/app/pkg/assert"
 )
 
-func Test_GetCertificate(t *testing.T) {
+func TestGetCertificate(t *testing.T) {
 	RegisterT(t)
 
 	var testCases = []struct {
@@ -41,3 +44,68 @@ func Test_GetCertificate(t *testing.T) {
 		Expect(cert.Certificate).Equals(wildcardCert.Certificate)
 	}
 }
+
+func TestGetCertificate_WhenCNAMEAreInvalid(t *testing.T) {
+	RegisterT(t)
+	bus.Init(fs.Service{})
+	bus.AddHandler(mockIsCNAMEAvailable)
+
+	manager, err := NewCertificateManager("", "")
+	Expect(err).IsNil()
+
+	invalidServerNames := []string{"feedback.heyworld.com"}
+
+	for _, serverName := range invalidServerNames {
+		cert, err := manager.GetCertificate(&tls.ClientHelloInfo{
+			ServerName: serverName,
+		})
+		Expect(err.Error()).ContainsSubstring(`no tenants found with cname ` + serverName)
+		Expect(cert).IsNil()
+	}
+
+	// GetCertificate starts a fire and forget go routine to delete items from cache, give it 2sec to complete it
+	time.Sleep(2 * time.Second)
+}
+
+func TestGetCertificate_ServerNameMatchesCertificate_ShouldReturnIt(t *testing.T) {
+	RegisterT(t)
+	bus.Init(fs.Service{})
+	bus.AddHandler(mockIsCNAMEAvailable)
+
+	certFile := env.Etc("dev-fider-io.crt")
+	certKey := env.Etc("dev-fider-io.key")
+	manager, err := NewCertificateManager(certFile, certKey)
+	Expect(err).IsNil()
+
+	serverNames := []string{"dev.fider.io", "feedback.dev.fider.io", "anything.dev.fider.io", "IDEAS.DEV.fider.io", ".feedback.DEV.fider.io"}
+
+	for _, serverName := range serverNames {
+		cert, err := manager.GetCertificate(&tls.ClientHelloInfo{
+			ServerName: serverName,
+		})
+		Expect(err).IsNil()
+		Expect(cert).IsNotNil()
+	}
+}
+
+func TestGetCertificate_ServerNameDoesntMatchCertificate_ButEndsWithHostName_ShouldThrow(t *testing.T) {
+	RegisterT(t)
+	bus.Init(fs.Service{})
+	bus.AddHandler(mockIsCNAMEAvailable)
+
+	env.Config.HostDomain = "dev.fider.io"
+	certFile := env.Etc("dev-fider-io.crt")
+	certKey := env.Etc("dev-fider-io.key")
+	manager, err := NewCertificateManager(certFile, certKey)
+	Expect(err).IsNil()
+
+	serverNames := []string{"sub.feedback.dev.fider.io"}
+
+	for _, serverName := range serverNames {
+		cert, err := manager.GetCertificate(&tls.ClientHelloInfo{
+			ServerName: serverName,
+		})
+		Expect(err.Error()).ContainsSubstring("invalid ServerName used: " + serverName)
+		Expect(cert).IsNil()
+	}
+}

app/services/sqlstore/postgres/tenant.go

@@ -86,7 +86,12 @@ func (t *dbEmailVerification) toModel() *entity.EmailVerification {
 
 func isCNAMEAvailable(ctx context.Context, q *query.IsCNAMEAvailable) error {
 	return using(ctx, func(trx *dbx.Trx, tenant *entity.Tenant, user *entity.User) error {
-		exists, err := trx.Exists("SELECT id FROM tenants WHERE cname = $1 AND id <> $2", q.CNAME, tenant.ID)
+		tenantID := 0
+		if tenant != nil {
+			tenantID = tenant.ID
+		}
+
+		exists, err := trx.Exists("SELECT id FROM tenants WHERE cname = $1 AND id <> $2", q.CNAME, tenantID)
 		if err != nil {
 			q.Result = false
 			return errors.Wrap(err, "failed to check if tenant exists with CNAME '%s'", q.CNAME)

app/services/sqlstore/postgres/tenant_test.go

@@ -139,6 +139,36 @@ func TestTenantStorage_GetByDomain_CNAME(t *testing.T) {
 	Expect(getByDomain.Result.Status).Equals(enum.TenantActive)
 }
 
+func TestTenantStorage_IsCNAMEAvailable_ExistingCNAME(t *testing.T) {
+	ctx := SetupDatabaseTest(t)
+	defer TeardownDatabaseTest()
+
+	err := bus.Dispatch(demoTenantCtx, &cmd.UpdateTenantSettings{
+		Title: "My Domain Inc.",
+		CNAME: "feedback.mycompany.com",
+		Logo:  &dto.ImageUpload{},
+	})
+	Expect(err).IsNil()
+
+	// when ctx doesn't have a tenant, CNAME is unavailable
+	isAvailable := &query.IsCNAMEAvailable{CNAME: "feedback.mycompany.com"}
+	err = bus.Dispatch(ctx, isAvailable)
+	Expect(err).IsNil()
+	Expect(isAvailable.Result).IsFalse()
+
+	// when CNAME belongs to tenant in context, it's still available
+	isAvailable = &query.IsCNAMEAvailable{CNAME: "feedback.mycompany.com"}
+	err = bus.Dispatch(demoTenantCtx, isAvailable)
+	Expect(err).IsNil()
+	Expect(isAvailable.Result).IsTrue()
+
+	// unused CNAMES are always available
+	isAvailable = &query.IsCNAMEAvailable{CNAME: "ideas.mycompany.com"}
+	err = bus.Dispatch(ctx, isAvailable)
+	Expect(err).IsNil()
+	Expect(isAvailable.Result).IsTrue()
+}
+
 func TestTenantStorage_IsSubdomainAvailable_ExistingDomain(t *testing.T) {
 	ctx := SetupDatabaseTest(t)
 	defer TeardownDatabaseTest()

etc/dev-fider-io.crt

@@ -1,18 +1,22 @@
 -----BEGIN CERTIFICATE-----
-MIICzzCCAbegAwIBAgIJAOfxz5w/17rGMA0GCSqGSIb3DQEBBQUAMBgxFjAUBgNV
-BAMTDS5kZXYuZmlkZXIuaW8wHhcNMjEwNjE0MTYyOTMwWhcNMzEwNjEyMTYyOTMw
-WjAYMRYwFAYDVQQDEw0uZGV2LmZpZGVyLmlvMIIBIjANBgkqhkiG9w0BAQEFAAOC
-AQ8AMIIBCgKCAQEA7ip+VpaIFYGV+7ga9YLctBg8SkPDzItDZPV0JSnMhVvh6rKC
-wYlNOY4XLXHzwdsgZY8Fdbm9SFbvlKSecjd+iYU71NiIr79hZVXZRcDbp1ztOLOU
-ekx8jqfJnHYJP3kyM34Jl3QkwR07kMCsPTBSSDsxT+i1tSMPpe5qcDta7VRayFrE
-o8YO30wDiQqt5/2JmHxeHHC991Z0/ebjul4Z8ZXtHqiUgwUlPuq9pfc7wcKnf38+
-W2JGZKot0xwWiFoBsaAip0qJxc0pN7eJd6GKMiRJ4c9MJxPl0vn98+w47vrOycOb
-9mA3q6540mgsjSwBQBGvMLxJMyGUzDc910e06wIDAQABoxwwGjAYBgNVHREEETAP
-gg0uZGV2LmZpZGVyLmlvMA0GCSqGSIb3DQEBBQUAA4IBAQDYinFkDmEJ4cmycTuv
-hXiILuRdfHyuUf75q3ewzRjmCciBaKEqj4s0XFwrlmmzaYmzcHkoY2M61URYbJQn
-nvO4CLwcYf879EuAqWBmvtQgpMlQIIfFA8JDG6RFmFlNAUcA/Kv4APw14Ynkn5D/
-S3otvQYdZMO+l2PuHLthUVj+Qy2pvwOC993+25sB/wJCWHl4aH/27bkmL3iUKovu
-HFKLoc3M+Fumv54GTWcTlPMHAhm81EhDUPmJl9QTUewmztUH2O4tfsMZ1yqDaux/
-MxcITfsP6WTDikP0iOMjOFF3bPsgSwrK3kfdSPQk9cibwnLOdaeEVoTfgo66lwnq
-mjZr
------END CERTIFICATE-----
+MIIDjTCCAnWgAwIBAgIUKJS9U7YzQOJJQrWsMXpWFM+JcXgwDQYJKoZIhvcNAQEL
+BQAwMDEXMBUGA1UEAwwOKi5kZXYuZmlkZXIuaW8xFTATBgNVBAMMDGRldi5maWRl
+ci5pbzAeFw0yMTA2MjIxNjI2MzFaFw0zMTA2MjAxNjI2MzFaMDAxFzAVBgNVBAMM
+DiouZGV2LmZpZGVyLmlvMRUwEwYDVQQDDAxkZXYuZmlkZXIuaW8wggEiMA0GCSqG
+SIb3DQEBAQUAA4IBDwAwggEKAoIBAQCTrS8FQ+xt4XupAQTrQk0NVIHoLY34E2kE
+IxlYfONPTBDOhiAtkTsXXancRXWtklrrIpjBHm9u1/iW8/1hqITDf40dgmupt2RT
+SVeXsAjgfHDy9v6oCo64tWqAkWb81rxrB0isLyrP6PGMXSTWJUukgx3KNeF+VN1w
+quNrlGFT1d6eMFWFufRVMZV6EnOOSCD21u8GM5CobnC/rqM4gNB2KHc5dgdERMP7
+uFw3Y6bPEeBzLv+9+QrMGmWf7//7kLdk6o7shWjkzgRig+bepvcOaafVL8hqaA+z
+BqIrxfEQnV30Tnr2jxNAEd3cSt1RgGF3as+VNQb12yBXATzgU4DhAgMBAAGjgZ4w
+gZswHQYDVR0OBBYEFI7FJZddSC7hu8COWlb/ZOzptR7IMB8GA1UdIwQYMBaAFI7F
+JZddSC7hu8COWlb/ZOzptR7IMA4GA1UdDwEB/wQEAwIFoDAgBgNVHSUBAf8EFjAU
+BggrBgEFBQcDAQYIKwYBBQUHAwIwJwYDVR0RBCAwHoIOKi5kZXYuZmlkZXIuaW+C
+DGRldi5maWRlci5pbzANBgkqhkiG9w0BAQsFAAOCAQEAYlWqm+rM/h0lCjXGjY0u
+HMRm84qequBDUO6pv3eE1qsf1Qc8XeziimDlKf6yrWe0cHmTVXHVOrH1uVo+e9UW
+L0t3rSCNH9jrSYwYx/TYqJXlvDMjnkdrzne6UcQR26H39q0oYzk/XfCFRwM1kAfF
+riuNU3hbCmrNKyhO3lkRyTVczUnbc0Yb1PtT6bLepZ0CVtYDb5El4N9IGIjLfPGY
+C4UG1JdJy33PAyticxFBsK14ios15Gp2po8G0sp6GP+3RM3ChoS38HDlKyqE0Pq9
+v4pw//KbFfCghxFYTupJJRbTYoU8U7Lh0Ou4SGQScuqK5jqBdk5LgmPIDNbv+2fQ
+9g==
+-----END CERTIFICATE-----