From 1c190b001b95e57d21dc63c01ae3c6de2a3fec57 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 2 Apr 2020 14:20:30 -0600 Subject: httpcaddyfile: Refactor site key parsing; detect conflicting schemes We now store the parsed site/server block keys with the server block, rather than parsing the addresses every time we read them. Also detect conflicting schemes, i.e. TLS and non-TLS cannot be served from the same server (natively -- modules could be built for it). Also do not add site subroutes (subroutes generated specifically from site blocks in the Caddyfile) that are empty. --- caddyconfig/httpcaddyfile/addresses.go | 12 ++- caddyconfig/httpcaddyfile/directives.go | 45 +++++++- caddyconfig/httpcaddyfile/httptype.go | 176 ++++++++++++++++++-------------- caddyconfig/httpcaddyfile/tlsapp.go | 23 +---- 4 files changed, 155 insertions(+), 101 deletions(-) (limited to 'caddyconfig') diff --git a/caddyconfig/httpcaddyfile/addresses.go b/caddyconfig/httpcaddyfile/addresses.go index f5fd6b6..6545585 100644 --- a/caddyconfig/httpcaddyfile/addresses.go +++ b/caddyconfig/httpcaddyfile/addresses.go @@ -106,12 +106,22 @@ func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBloc // server block are only the ones which use the address; but // the contents (tokens) are of course the same for addr, keys := range addrToKeys { + // parse keys so that we only have to do it once + parsedKeys := make([]Address, 0, len(keys)) + for _, key := range keys { + addr, err := ParseAddress(key) + if err != nil { + return nil, fmt.Errorf("parsing key '%s': %v", key, err) + } + parsedKeys = append(parsedKeys, addr.Normalize()) + } sbmap[addr] = append(sbmap[addr], serverBlock{ block: caddyfile.ServerBlock{ Keys: keys, Segments: sblock.block.Segments, }, pile: sblock.pile, + keys: parsedKeys, }) } } @@ -165,7 +175,7 @@ func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key str // figure out the HTTP and HTTPS ports; either // use defaults, or override with user config - httpPort, httpsPort := strconv.Itoa(certmagic.HTTPPort), strconv.Itoa(certmagic.HTTPSPort) + httpPort, httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPPort), strconv.Itoa(caddyhttp.DefaultHTTPSPort) if hport, ok := options["http_port"]; ok { httpPort = strconv.Itoa(hport.(int)) } diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go index 4c2b2d9..8fa48cd 100644 --- a/caddyconfig/httpcaddyfile/directives.go +++ b/caddyconfig/httpcaddyfile/directives.go @@ -16,7 +16,9 @@ package httpcaddyfile import ( "encoding/json" + "net" "sort" + "strconv" "strings" "github.com/caddyserver/caddy/v2" @@ -381,12 +383,49 @@ func parseSegmentAsSubroute(h Helper) (caddyhttp.MiddlewareHandler, error) { return buildSubroute(allResults, h.groupCounter) } -// serverBlock pairs a Caddyfile server block -// with a "pile" of config values, keyed by class -// name. +// serverBlock pairs a Caddyfile server block with +// a "pile" of config values, keyed by class name, +// as well as its parsed keys for convenience. type serverBlock struct { block caddyfile.ServerBlock pile map[string][]ConfigValue // config values obtained from directives + keys []Address +} + +// hostsFromKeys returns a list of all the non-empty hostnames found in +// the keys of the server block sb, unless allowEmpty is true, in which +// case a key with no host (e.g. ":443") will be added to the list as an +// empty string. Otherwise, if allowEmpty is false, and if sb has a key +// that omits the hostname (i.e. is a catch-all/empty host), then the returned +// list is empty, because the server block effectively matches ALL hosts. +// The list may not be in a consistent order. If includePorts is true, then +// any non-empty, non-standard ports will be included. +func (sb serverBlock) hostsFromKeys(allowEmpty, includePorts bool) []string { + // first get each unique hostname + hostMap := make(map[string]struct{}) + for _, addr := range sb.keys { + if addr.Host == "" && !allowEmpty { + // server block contains a key like ":443", i.e. the host portion + // is empty / catch-all, which means to match all hosts + return []string{} + } + if includePorts && + addr.Port != "" && + addr.Port != strconv.Itoa(caddyhttp.DefaultHTTPPort) && + addr.Port != strconv.Itoa(caddyhttp.DefaultHTTPSPort) { + hostMap[net.JoinHostPort(addr.Host, addr.Port)] = struct{}{} + } else { + hostMap[addr.Host] = struct{}{} + } + } + + // convert map to slice + sblockHosts := make([]string, 0, len(hostMap)) + for host := range hostMap { + sblockHosts = append(sblockHosts, host) + } + + return sblockHosts } type ( diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go index 4df5421..7fbf724 100644 --- a/caddyconfig/httpcaddyfile/httptype.go +++ b/caddyconfig/httpcaddyfile/httptype.go @@ -17,7 +17,6 @@ package httpcaddyfile import ( "encoding/json" "fmt" - "net" "reflect" "sort" "strconv" @@ -320,47 +319,6 @@ func (ServerType) evaluateGlobalOptionsBlock(serverBlocks []serverBlock, options return serverBlocks[1:], nil } -// hostsFromServerBlockKeys returns a list of all the non-empty hostnames -// found in the keys of the server block sb, unless allowEmpty is true, in -// which case a key with no host (e.g. ":443") will be added to the list as -// an empty string. Otherwise, if allowEmpty is false, and if sb has a key -// that omits the hostname (i.e. is a catch-all/empty host), then the returned -// list is empty, because the server block effectively matches ALL hosts. -// The list may not be in a consistent order. If includePorts is true, then -// any non-empty, non-standard ports will be included. -func (st *ServerType) hostsFromServerBlockKeys(sb caddyfile.ServerBlock, allowEmpty, includePorts bool) ([]string, error) { - // first get each unique hostname - hostMap := make(map[string]struct{}) - for _, sblockKey := range sb.Keys { - addr, err := ParseAddress(sblockKey) - if err != nil { - return nil, fmt.Errorf("parsing server block key: %v", err) - } - addr = addr.Normalize() - if addr.Host == "" && !allowEmpty { - // server block contains a key like ":443", i.e. the host portion - // is empty / catch-all, which means to match all hosts - return []string{}, nil - } - if includePorts && - addr.Port != "" && - addr.Port != strconv.Itoa(caddyhttp.DefaultHTTPPort) && - addr.Port != strconv.Itoa(caddyhttp.DefaultHTTPSPort) { - hostMap[net.JoinHostPort(addr.Host, addr.Port)] = struct{}{} - } else { - hostMap[addr.Host] = struct{}{} - } - } - - // convert map to slice - sblockHosts := make([]string, 0, len(hostMap)) - for host := range hostMap { - sblockHosts = append(sblockHosts, host) - } - - return sblockHosts, nil -} - // serversFromPairings creates the servers for each pairing of addresses // to server blocks. Each pairing is essentially a server definition. func (st *ServerType) serversFromPairings( @@ -384,11 +342,10 @@ func (st *ServerType) serversFromPairings( // descending sort by length of host then path sort.SliceStable(p.serverBlocks, func(i, j int) bool { // TODO: we could pre-process the specificities for efficiency, - // but I don't expect many blocks will have SO many keys... + // but I don't expect many blocks will have THAT many keys... var iLongestPath, jLongestPath string var iLongestHost, jLongestHost string - for _, key := range p.serverBlocks[i].block.Keys { - addr, _ := ParseAddress(key) + for _, addr := range p.serverBlocks[i].keys { if specificity(addr.Host) > specificity(iLongestHost) { iLongestHost = addr.Host } @@ -396,8 +353,7 @@ func (st *ServerType) serversFromPairings( iLongestPath = addr.Path } } - for _, key := range p.serverBlocks[j].block.Keys { - addr, _ := ParseAddress(key) + for _, addr := range p.serverBlocks[j].keys { if specificity(addr.Host) > specificity(jLongestHost) { jLongestHost = addr.Host } @@ -415,15 +371,12 @@ func (st *ServerType) serversFromPairings( // create a subroute for each site in the server block for _, sblock := range p.serverBlocks { - matcherSetsEnc, err := st.compileEncodedMatcherSets(sblock.block) + matcherSetsEnc, err := st.compileEncodedMatcherSets(sblock) if err != nil { return nil, fmt.Errorf("server block %v: compiling matcher sets: %v", sblock.block.Keys, err) } - hosts, err := st.hostsFromServerBlockKeys(sblock.block, false, false) - if err != nil { - return nil, err - } + hosts := sblock.hostsFromKeys(false, false) // tls: connection policies if cpVals, ok := sblock.pile["tls.connection_policy"]; ok { @@ -455,12 +408,7 @@ func (st *ServerType) serversFromPairings( // exclude any hosts that were defined explicitly with // "http://" in the key from automated cert management (issue #2998) - for _, key := range sblock.block.Keys { - addr, err := ParseAddress(key) - if err != nil { - return nil, err - } - addr = addr.Normalize() + for _, addr := range sblock.keys { if addr.Scheme == "http" && addr.Host != "" { if srv.AutoHTTPS == nil { srv.AutoHTTPS = new(caddyhttp.AutoHTTPSConfig) @@ -500,16 +448,19 @@ func (st *ServerType) serversFromPairings( LoggerNames: make(map[string]string), } } - hosts, err := st.hostsFromServerBlockKeys(sblock.block, true, true) - if err != nil { - return nil, err - } - for _, h := range hosts { + for _, h := range sblock.hostsFromKeys(true, true) { srv.Logs.LoggerNames[h] = ncl.name } } } + // a server cannot (natively) serve both HTTP and HTTPS at the + // same time, so make sure the configuration isn't in conflict + err := detectConflictingSchemes(srv, p.serverBlocks, options) + if err != nil { + return nil, err + } + // a catch-all TLS conn policy is necessary to ensure TLS can // be offered to all hostnames of the server; even though only // one policy is needed to enable TLS for the server, that @@ -527,7 +478,6 @@ func (st *ServerType) serversFromPairings( } // tidy things up a bit - var err error srv.TLSConnPolicies, err = consolidateConnPolicies(srv.TLSConnPolicies) if err != nil { return nil, fmt.Errorf("consolidating TLS connection policies for server %d: %v", i, err) @@ -540,6 +490,64 @@ func (st *ServerType) serversFromPairings( return servers, nil } +func detectConflictingSchemes(srv *caddyhttp.Server, serverBlocks []serverBlock, options map[string]interface{}) error { + httpPort := strconv.Itoa(caddyhttp.DefaultHTTPPort) + if hp, ok := options["http_port"].(int); ok { + httpPort = strconv.Itoa(hp) + } + httpsPort := strconv.Itoa(caddyhttp.DefaultHTTPSPort) + if hsp, ok := options["https_port"].(int); ok { + httpsPort = strconv.Itoa(hsp) + } + + var httpOrHTTPS string + checkAndSetHTTP := func(addr Address) error { + if httpOrHTTPS == "HTTPS" { + errMsg := fmt.Errorf("server listening on %v is configured for HTTPS and cannot natively multiplex HTTP and HTTPS: %s", + srv.Listen, addr.Original) + if addr.Scheme == "" && addr.Host == "" { + errMsg = fmt.Errorf("%s (try specifying https:// in the address)", errMsg) + } + return errMsg + } + if len(srv.TLSConnPolicies) > 0 { + // any connection policies created for an HTTP server + // is a logical conflict, as it would enable HTTPS + return fmt.Errorf("server listening on %v is HTTP, but attempts to configure TLS connection policies", srv.Listen) + } + httpOrHTTPS = "HTTP" + return nil + } + checkAndSetHTTPS := func(addr Address) error { + if httpOrHTTPS == "HTTP" { + return fmt.Errorf("server listening on %v is configured for HTTP and cannot natively multiplex HTTP and HTTPS: %s", + srv.Listen, addr.Original) + } + httpOrHTTPS = "HTTPS" + return nil + } + + for _, sblock := range serverBlocks { + for _, addr := range sblock.keys { + if addr.Scheme == "http" || addr.Port == httpPort { + if err := checkAndSetHTTP(addr); err != nil { + return err + } + } else if addr.Scheme == "https" || addr.Port == httpsPort { + if err := checkAndSetHTTPS(addr); err != nil { + return err + } + } else if addr.Host == "" { + if err := checkAndSetHTTP(addr); err != nil { + return err + } + } + } + } + + return nil +} + // consolidateConnPolicies combines TLS connection policies that are the same, // for a cleaner overall output. func consolidateConnPolicies(cps caddytls.ConnectionPolicies) (caddytls.ConnectionPolicies, error) { @@ -664,18 +672,34 @@ func appendSubrouteToRouteList(routeList caddyhttp.RouteList, matcherSetsEnc []caddy.ModuleMap, p sbAddrAssociation, warnings *[]caddyconfig.Warning) caddyhttp.RouteList { + + // nothing to do if... there's nothing to do + if len(matcherSetsEnc) == 0 && len(subroute.Routes) == 0 && subroute.Errors == nil { + return routeList + } + if len(matcherSetsEnc) == 0 && len(p.serverBlocks) == 1 { // no need to wrap the handlers in a subroute if this is // the only server block and there is no matcher for it routeList = append(routeList, subroute.Routes...) } else { - routeList = append(routeList, caddyhttp.Route{ - MatcherSetsRaw: matcherSetsEnc, - HandlersRaw: []json.RawMessage{ + route := caddyhttp.Route{ + // the semantics of a site block in the Caddyfile dictate + // that only the first matching one is evaluated, since + // site blocks do not cascade nor inherit + Terminal: true, + } + if len(matcherSetsEnc) > 0 { + route.MatcherSetsRaw = matcherSetsEnc + } + if len(subroute.Routes) > 0 || subroute.Errors != nil { + route.HandlersRaw = []json.RawMessage{ caddyconfig.JSONModuleObject(subroute, "handler", "subroute", warnings), - }, - Terminal: true, // only first matching site block should be evaluated - }) + } + } + if len(route.MatcherSetsRaw) > 0 || len(route.HandlersRaw) > 0 { + routeList = append(routeList, route) + } } return routeList } @@ -822,7 +846,7 @@ func matcherSetFromMatcherToken( return nil, false, nil } -func (st *ServerType) compileEncodedMatcherSets(sblock caddyfile.ServerBlock) ([]caddy.ModuleMap, error) { +func (st *ServerType) compileEncodedMatcherSets(sblock serverBlock) ([]caddy.ModuleMap, error) { type hostPathPair struct { hostm caddyhttp.MatchHost pathm caddyhttp.MatchPath @@ -832,13 +856,7 @@ func (st *ServerType) compileEncodedMatcherSets(sblock caddyfile.ServerBlock) ([ var matcherPairs []*hostPathPair var catchAllHosts bool - for _, key := range sblock.Keys { - addr, err := ParseAddress(key) - if err != nil { - return nil, fmt.Errorf("server block %v: parsing and standardizing address '%s': %v", sblock.Keys, key, err) - } - addr = addr.Normalize() - + for _, addr := range sblock.keys { // choose a matcher pair that should be shared by this // server block; if none exists yet, create one var chosenMatcherPair *hostPathPair @@ -905,7 +923,7 @@ func (st *ServerType) compileEncodedMatcherSets(sblock caddyfile.ServerBlock) ([ for _, ms := range matcherSets { msEncoded, err := encodeMatcherSet(ms) if err != nil { - return nil, fmt.Errorf("server block %v: %v", sblock.Keys, err) + return nil, fmt.Errorf("server block %v: %v", sblock.block.Keys, err) } matcherSetsEnc = append(matcherSetsEnc, msEncoded) } diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go index db3d13b..6214d61 100644 --- a/caddyconfig/httpcaddyfile/tlsapp.go +++ b/caddyconfig/httpcaddyfile/tlsapp.go @@ -43,26 +43,16 @@ func (st ServerType) buildTLSApp( hostsSharedWithHostlessKey := make(map[string]struct{}) for _, pair := range pairings { for _, sb := range pair.serverBlocks { - for _, key := range sb.block.Keys { - addr, err := ParseAddress(key) - if err != nil { - return nil, warnings, err - } - addr = addr.Normalize() + for _, addr := range sb.keys { if addr.Host == "" { serverBlocksWithHostlessKey++ // this server block has a hostless key, now // go through and add all the hosts to the set - for _, otherKey := range sb.block.Keys { - if otherKey == key { + for _, otherAddr := range sb.keys { + if otherAddr.Original == addr.Original { continue } - addr, err := ParseAddress(otherKey) - if err != nil { - return nil, warnings, err - } - addr = addr.Normalize() - if addr.Host != "" { + if otherAddr.Host != "" { hostsSharedWithHostlessKey[addr.Host] = struct{}{} } } @@ -82,10 +72,7 @@ func (st ServerType) buildTLSApp( // get values that populate an automation policy for this block var ap *caddytls.AutomationPolicy - sblockHosts, err := st.hostsFromServerBlockKeys(sblock.block, false, false) - if err != nil { - return nil, warnings, err - } + sblockHosts := sblock.hostsFromKeys(false, false) if len(sblockHosts) == 0 { ap = catchAllAP } -- cgit v1.2.3