Skip to content

Commit

Permalink
Merge branch 'release/1.19.x' into backport/b-ui/prettify-ember-cli-b…
Browse files Browse the repository at this point in the history
…uild/namely-trusty-goldfish
  • Loading branch information
philrenaud authored Sep 13, 2024
2 parents d839e33 + b6784bf commit 791c7b1
Show file tree
Hide file tree
Showing 23 changed files with 266 additions and 99 deletions.
3 changes: 3 additions & 0 deletions .changelog/21703.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
jwt-provider: change dns lookup family from the default of AUTO which would prefer ipv6 to ALL if LOGICAL_DNS is used or PREFER_IPV4 if STRICT_DNS is used to gracefully handle transitions to ipv6.
```
3 changes: 3 additions & 0 deletions .changelog/21704.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:security
Explicitly set 'Content-Type' header to mitigate XSS vulnerability.
```
3 changes: 3 additions & 0 deletions .changelog/21711.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:security
Implement HTML sanitization for user-generated content to prevent XSS attacks in the UI.
```
3 changes: 3 additions & 0 deletions .changelog/21717.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:security
ui: Pin a newer resolution of Markdown-it
```
3 changes: 3 additions & 0 deletions .changelog/21726.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:security
UI: Remove codemirror linting due to package dependency
```
4 changes: 4 additions & 0 deletions .changelog/21729.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
```release-notes:security
Bump Dockerfile base image to `alpine:3.20`.
This resolves CVE-2024-7264 and CVE-2024-8096 (curl).
```
4 changes: 0 additions & 4 deletions .release/security-scan.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ container {
suppress {
# N.b. `vulnerabilites` is the correct spelling for this tool.
vulnerabilites = [
"CVE-2023-46218", # [email protected]
"CVE-2023-46219", # [email protected]
"CVE-2023-5678", # [email protected]
"CVE-2024-7264", # [email protected]
]
paths = [
"internal/tools/proto-gen-rpc-glue/e2e/consul/*",
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# Official docker image that includes binaries from releases.hashicorp.com. This
# downloads the release from releases.hashicorp.com and therefore requires that
# the release is published before building the Docker image.
FROM docker.mirror.hashicorp.services/alpine:3.19 as official
FROM docker.mirror.hashicorp.services/alpine:3.20 as official

# This is the release of Consul to pull in.
ARG VERSION
Expand Down Expand Up @@ -112,7 +112,7 @@ CMD ["agent", "-dev", "-client", "0.0.0.0"]

# Production docker image that uses CI built binaries.
# Remember, this image cannot be built locally.
FROM docker.mirror.hashicorp.services/alpine:3.19 as default
FROM docker.mirror.hashicorp.services/alpine:3.20 as default

ARG PRODUCT_VERSION
ARG BIN_NAME
Expand Down
9 changes: 9 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,14 @@ envoy-regen: ## Regenerating envoy golden files
@find "command/connect/envoy/testdata" -name '*.golden' -delete
@go test -tags '$(GOTAGS)' ./command/connect/envoy -update


##@ Changelog

.PHONY: gen-changelog
gen-changelog: ## Generate changelog entry for the current branch based on the currently open PR for that branch
@$(SHELL) $(CURDIR)/build-support/scripts/gen-changelog.sh


##@ Help

# The help target prints out all targets with their descriptions organized
Expand All @@ -635,3 +643,4 @@ envoy-regen: ## Regenerating envoy golden files
.PHONY: help
help: ## Display this help.
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z_0-9-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)

35 changes: 34 additions & 1 deletion agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package agent
import (
"encoding/json"
"fmt"
"github.com/hashicorp/go-hclog"
"io"
"net"
"net/http"
Expand Down Expand Up @@ -42,6 +43,11 @@ import (
"github.com/hashicorp/consul/proto/private/pbcommon"
)

const (
contentTypeHeader = "Content-Type"
plainContentType = "text/plain; charset=utf-8"
)

var HTTPSummaries = []prometheus.SummaryDefinition{
{
Name: []string{"api", "http"},
Expand Down Expand Up @@ -219,6 +225,7 @@ func (s *HTTPHandlers) handler() http.Handler {
// If enableDebug register wrapped pprof handlers
if !s.agent.enableDebug.Load() && s.checkACLDisabled() {
resp.WriteHeader(http.StatusNotFound)
resp.Header().Set(contentTypeHeader, plainContentType)
return
}

Expand All @@ -227,6 +234,7 @@ func (s *HTTPHandlers) handler() http.Handler {

authz, err := s.agent.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
if err != nil {
resp.Header().Set(contentTypeHeader, plainContentType)
resp.WriteHeader(http.StatusForbidden)
return
}
Expand All @@ -236,6 +244,7 @@ func (s *HTTPHandlers) handler() http.Handler {
// TODO(partitions): should this be possible in a partition?
// TODO(acl-error-enhancements): We should return error details somehow here.
if authz.OperatorRead(nil) != acl.Allow {
resp.Header().Set(contentTypeHeader, plainContentType)
resp.WriteHeader(http.StatusForbidden)
return
}
Expand Down Expand Up @@ -316,6 +325,8 @@ func (s *HTTPHandlers) handler() http.Handler {
}

h = withRemoteAddrHandler(h)
h = ensureContentTypeHeader(h, s.agent.logger)

s.h = &wrappedMux{
mux: mux,
handler: h,
Expand All @@ -336,6 +347,20 @@ func withRemoteAddrHandler(next http.Handler) http.Handler {
})
}

// Injects content type explicitly if not already set into response to prevent XSS
func ensureContentTypeHeader(next http.Handler, logger hclog.Logger) http.Handler {

return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
next.ServeHTTP(resp, req)

val := resp.Header().Get(contentTypeHeader)
if val == "" {
resp.Header().Set(contentTypeHeader, plainContentType)
logger.Debug("warning: content-type header not explicitly set.", "request-path", req.URL)
}
})
}

// nodeName returns the node name of the agent
func (s *HTTPHandlers) nodeName() string {
return s.agent.config.NodeName
Expand Down Expand Up @@ -379,6 +404,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc
"from", req.RemoteAddr,
"error", err,
)
//set response type to plain to prevent XSS
resp.Header().Set(contentTypeHeader, plainContentType)
resp.WriteHeader(http.StatusInternalServerError)
return
}
Expand Down Expand Up @@ -410,6 +437,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc
"from", req.RemoteAddr,
"error", errMsg,
)
//set response type to plain to prevent XSS
resp.Header().Set(contentTypeHeader, plainContentType)
resp.WriteHeader(http.StatusForbidden)
fmt.Fprint(resp, errMsg)
return
Expand Down Expand Up @@ -606,6 +635,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc
resp.Header().Add("X-Consul-Reason", errPayload.Reason)
}
} else {
//set response type to plain to prevent XSS
resp.Header().Set(contentTypeHeader, plainContentType)
handleErr(err)
return
}
Expand All @@ -617,6 +648,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc
if contentType == "application/json" {
buf, err = s.marshalJSON(req, obj)
if err != nil {
//set response type to plain to prevent XSS
resp.Header().Set(contentTypeHeader, plainContentType)
handleErr(err)
return
}
Expand All @@ -627,7 +660,7 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc
}
}
}
resp.Header().Set("Content-Type", contentType)
resp.Header().Set(contentTypeHeader, contentType)
resp.WriteHeader(httpCode)
resp.Write(buf)
}
Expand Down
32 changes: 28 additions & 4 deletions agent/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -639,14 +639,14 @@ func TestHTTPAPIResponseHeaders(t *testing.T) {
`)
defer a.Shutdown()

requireHasHeadersSet(t, a, "/v1/agent/self")
requireHasHeadersSet(t, a, "/v1/agent/self", "application/json")

// Check the Index page that just renders a simple message with UI disabled
// also gets the right headers.
requireHasHeadersSet(t, a, "/")
requireHasHeadersSet(t, a, "/", "text/plain; charset=utf-8")
}

func requireHasHeadersSet(t *testing.T, a *TestAgent, path string) {
func requireHasHeadersSet(t *testing.T, a *TestAgent, path string, contentType string) {
t.Helper()

resp := httptest.NewRecorder()
Expand All @@ -661,6 +661,9 @@ func requireHasHeadersSet(t *testing.T, a *TestAgent, path string) {

require.Equal(t, "1; mode=block", hdrs.Get("X-XSS-Protection"),
"X-XSS-Protection header value incorrect")

require.Equal(t, contentType, hdrs.Get("Content-Type"),
"")
}

func TestUIResponseHeaders(t *testing.T) {
Expand All @@ -680,7 +683,28 @@ func TestUIResponseHeaders(t *testing.T) {
`)
defer a.Shutdown()

requireHasHeadersSet(t, a, "/ui")
//response header for the UI appears to be being handled by the UI itself.
requireHasHeadersSet(t, a, "/ui", "text/plain; charset=utf-8")
}

func TestErrorContentTypeHeaderSet(t *testing.T) {
if testing.Short() {
t.Skip("too slow for testing.Short")
}

t.Parallel()
a := NewTestAgent(t, `
http_config {
response_headers = {
"Access-Control-Allow-Origin" = "*"
"X-XSS-Protection" = "1; mode=block"
"X-Frame-Options" = "SAMEORIGIN"
}
}
`)
defer a.Shutdown()

requireHasHeadersSet(t, a, "/fake-path-doesn't-exist", "text/plain; charset=utf-8")
}

func TestAcceptEncodingGzip(t *testing.T) {
Expand Down
22 changes: 21 additions & 1 deletion agent/xds/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,12 @@ func makeJWTProviderCluster(p *structs.JWTProviderConfigEntry) (*envoy_cluster_v
return nil, err
}

discoveryType := makeJWKSDiscoveryClusterType(p.JSONWebKeySet.Remote)
lookupFamily := makeJWKSClusterDNSLookupFamilyType(discoveryType)
cluster := &envoy_cluster_v3.Cluster{
Name: makeJWKSClusterName(p.Name),
ClusterDiscoveryType: makeJWKSDiscoveryClusterType(p.JSONWebKeySet.Remote),
ClusterDiscoveryType: discoveryType,
DnsLookupFamily: lookupFamily,
LoadAssignment: &envoy_endpoint_v3.ClusterLoadAssignment{
ClusterName: makeJWKSClusterName(p.Name),
Endpoints: []*envoy_endpoint_v3.LocalityLbEndpoints{
Expand Down Expand Up @@ -278,6 +281,23 @@ func makeJWKSDiscoveryClusterType(r *structs.RemoteJWKS) *envoy_cluster_v3.Clust
return ct
}

func makeJWKSClusterDNSLookupFamilyType(r *envoy_cluster_v3.Cluster_Type) envoy_cluster_v3.Cluster_DnsLookupFamily {
// When using LOGICAL_DNS we want to use the Cluster_ALL lookup family which will fetch all the ip addresses for a given hostname and then
// try to connect to each one and will create the cluster based on the first one that passes.
// When using STRICT_DNS we want to use the CLUSTER_V4_PREFERRED lookup family which will prefer
// creating clusters using ipv4 addresses if those are available.
// Otherwise we fallback to Cluser_AUTO which will use the default behavior, and will be ignored as per the documentation.
// https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-enum-config-cluster-v3-cluster-dnslookupfamily
switch r.Type {
case envoy_cluster_v3.Cluster_LOGICAL_DNS:
return envoy_cluster_v3.Cluster_ALL
case envoy_cluster_v3.Cluster_STRICT_DNS:
return envoy_cluster_v3.Cluster_V4_PREFERRED
default:
return envoy_cluster_v3.Cluster_AUTO
}
}

func makeJWTCertValidationContext(p *structs.JWKSCluster) *envoy_tls_v3.CertificateValidationContext {
vc := &envoy_tls_v3.CertificateValidationContext{}
if p == nil || p.TLSCertificates == nil {
Expand Down
50 changes: 50 additions & 0 deletions agent/xds/clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,56 @@ func TestMakeJWKSDiscoveryClusterType(t *testing.T) {
}
}

func TestMakeJWKSClusterDNSLookupFamilyType(t *testing.T) {
tests := map[string]struct {
clusterType *envoy_cluster_v3.Cluster_Type
expectedDNSLookupFamily envoy_cluster_v3.Cluster_DnsLookupFamily
}{
// strict dns and logical dns are the only ones that are different
"jwks with strict dns": {
clusterType: &envoy_cluster_v3.Cluster_Type{
Type: envoy_cluster_v3.Cluster_STRICT_DNS,
},
expectedDNSLookupFamily: envoy_cluster_v3.Cluster_V4_PREFERRED,
},
"jwks with logical dns": {
clusterType: &envoy_cluster_v3.Cluster_Type{
Type: envoy_cluster_v3.Cluster_LOGICAL_DNS,
},
expectedDNSLookupFamily: envoy_cluster_v3.Cluster_ALL,
},
// all should be auto from here down
"jwks with cluster EDS": {
clusterType: &envoy_cluster_v3.Cluster_Type{
Type: envoy_cluster_v3.Cluster_EDS,
},
expectedDNSLookupFamily: envoy_cluster_v3.Cluster_AUTO,
},
"jwks with static dns": {
clusterType: &envoy_cluster_v3.Cluster_Type{
Type: envoy_cluster_v3.Cluster_STATIC,
},
expectedDNSLookupFamily: envoy_cluster_v3.Cluster_AUTO,
},

"jwks with original dst": {
clusterType: &envoy_cluster_v3.Cluster_Type{
Type: envoy_cluster_v3.Cluster_ORIGINAL_DST,
},
expectedDNSLookupFamily: envoy_cluster_v3.Cluster_AUTO,
},
}

for name, tt := range tests {
tt := tt
t.Run(name, func(t *testing.T) {
actualDNSLookupFamily := makeJWKSClusterDNSLookupFamilyType(tt.clusterType)

require.Equal(t, tt.expectedDNSLookupFamily, actualDNSLookupFamily)
})
}
}

func TestParseJWTRemoteURL(t *testing.T) {
tests := map[string]struct {
uri string
Expand Down
Loading

0 comments on commit 791c7b1

Please sign in to comment.