Ability to disable Envoy adding server headers to responses

[youngnick]

From #3692, @LiddersUK:

My security team and the external pen testers continue to raise this as an (admittedly low impact) issue as if an attacker knows the technology that we employ then it gives them an attack vector.
Their recommendation is for us to remove any server banners from response headers.

According to envoyproxy/envoy#14421, it seems like we will need to set the server_header_transformation param on the HTTPConnectionManager top PASS_THROUGH.

This will require a new global config boolean that defaults to true that will allow disabling of Envoy's server header responses.

[github-actions]

The Contour project currently lacks enough contributors to adequately respond to all Issues.

This bot triages Issues according to the following rules:

• After 60d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, the Issue is closed

You can:

• Mark this Issue as fresh by commenting
• Close this Issue
• Offer to help out with triage

Please send feedback to the #contour channel in the Kubernetes Slack

[chris93111]

Hi any update of this ?

[sunjayBhatia]

Hey @chris93111 This has not been prioritized at this time, but nevertheless I think this issue is a good candidate for community contribution if you are in need of this feature! We can of course provide some guidance on where to start to get this implemented

[vishal-chdhry]

Hi @sunjayBhatia! I would love to implement this, can you please guide me on where to start?

[sunjayBhatia]

@vishal-chdhry

• add a new boolean field to this struct that signifies we will not modify the Server header, should default to false and have a name that means we maintain existing behavior (Server header is modified):

  contour/apis/projectcontour/v1alpha1/contourconfig.go

  Line 314 in ccb79f8

  type EnvoyListenerConfig struct {

• add same config to the config file fields:

  contour/pkg/config/parameters.go

  Line 434 in ccb79f8

  type Parameters struct {

• ensure "conversion" logic here is up to date with the new field:

  contour/cmd/contour/servecontext.go

  Line 271 in ccb79f8

  func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha1.ContourConfigurationSpec {

• pipe that configuration through, to ensure this field is set: https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-field-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-server-header-transformation
  • Look to see how other similar fields do this:

    contour/cmd/contour/serve.go

    Lines 348 to 351 in ccb79f8

    	DefaultHTTPVersions: parseDefaultHTTPVersions(contourConfiguration.Envoy.DefaultHTTPVersions),
    	AllowChunkedLength: !*contourConfiguration.Envoy.Listener.DisableAllowChunkedLength,
    	MergeSlashes: !*contourConfiguration.Envoy.Listener.DisableMergeSlashes,
    	XffNumTrustedHops: *contourConfiguration.Envoy.Network.XffNumTrustedHops,

  • by default (if new field is false), should be set to OVERWRITE
  • if requested, set to PASS_THROUGH
  • we may want to discuss more if we should use APPEND_IF_ABSENT but PASS_THROUGH seems like a good place to start
• Add a featuretest to ensure the implementation is complete

[vishal-chdhry]

@sunjayBhatia, Sorry for the delay on this issue, i was busy in college exams.

why cant we directly use an enum, i am having trouble with figuring out where it should be an enum and where should i convert it to the boolean type.

I have to add it as an enum here

contour/internal/envoy/v3/listener.go

Line 150 in 057bac9

type httpConnectionManagerBuilder struct {

and convert it to an enum as well probably by doing something like this

contour/internal/envoy/v3/listener.go

Line 239 in 057bac9

func (b *httpConnectionManagerBuilder) MergeSlashes(enabled bool) *httpConnectionManagerBuilder {

I have opened a PR on this as well