Fix HSTS Missing From HTTPS Server(Nessus Scanner) #96502

249043822 · 2020-11-12T08:17:30Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

Nessus scanner treats this as a Medium Risk

Which issue(s) this PR fixes:

Fixes #96040

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Adds the ability to pass --strict-transport-security-directives to the kube-apiserver to set the HSTS header appropriately.  Be sure you understand the consequences to browsers before setting this field.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2020-11-12T08:17:37Z

@249043822: This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2020-11-12T08:17:38Z

Hi @249043822. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

fedebongio · 2020-11-12T21:12:48Z

/remove-sig api-machinery
/sig auth

staging/src/k8s.io/apiserver/pkg/server/filters/hsts.go

leilajal · 2020-11-17T21:09:39Z

/remove-sig api-machinery

249043822 · 2020-11-27T08:11:21Z

/assign @mikedanese @deads2k

249043822 · 2020-11-27T08:13:28Z

/assign @mikedanese @deads2k

249043822 · 2020-11-27T09:03:33Z

/assgin @liggitt @caesarxuchao

liggitt · 2020-11-27T13:56:24Z

/hold

Thanks for the PR, but we cannot unilaterally indicate all ports on the host the API server was accessed by must be serving https (other processes can be serving http on different ports). This would affect things like development servers on localhost and servers which surface other services via other ports. The same applies to including subdomains… the kube-apiserver cannot make decisions for all subdomains of its host

deads2k · 2021-01-08T13:52:34Z

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go

+	if len(hstsHeaders) != 0 {
+		for _, hstsHeader := range hstsHeaders {
+			if len(strings.TrimSpace(hstsHeader)) == 0 {
+				return fmt.Errorf("empty value in strict-transport-security-headers")


build a full slice of errors []error and then use pkg/util/errors.NewAggregate at the end. If memory serves, it correctly results nil on an empty slice, so you get the "easy" behavior that works and all the errors at the same time.

deads2k · 2021-01-08T13:54:06Z

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go

+			if hstsHeader != "includeSubDomains" && hstsHeader != "preload" {
+				maxAgeValue := strings.Split(hstsHeader, "=")
+				if len(maxAgeValue) != 2 || maxAgeValue[0] != "max-age" {
+					return fmt.Errorf("--strict-transport-security-headers invalid, must be of format: max-age=expireTime,includeSubDomains,preload")


please link to the spec saying these are the only valid values. The format appears more flexible to me https://tools.ietf.org/html/rfc6797#section-6.1 with some kind of a registry somewhere for extension.

deads2k · 2021-01-08T13:55:34Z

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go

@@ -161,6 +185,9 @@ func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) {
 		"List of allowed origins for CORS, comma separated.  An allowed origin can be a regular "+
 		"expression to support subdomain matching. If this list is empty CORS will not be enabled.")

+	fs.StringSliceVar(&s.HSTSHeaders, "strict-transport-security-headers", s.HSTSHeaders, ""+


please provide an example in the usage. I may have misunderstood the code. Are these the hsts directives that go into a single header? The code suggests the header cannot be repeated?

Yea, all hsts directives go into a single header

staging/src/k8s.io/apiserver/pkg/server/filters/hsts.go

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go

deads2k · 2021-01-11T22:00:41Z

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go

@@ -18,7 +18,9 @@ package options

 import (
 	"fmt"
+	"k8s.io/apimachinery/pkg/util/errors"


nit order. this belongs in the next block of imports.

deads2k · 2021-01-11T22:03:15Z

Yea, all hsts directives go into a single header

Sorry about this, I hadn't read in detail before. Can you replace all the headers with directives to match the RFC. The variables and the flags.

/approve

After we get the names fixed, I think this is ready to go.

dims · 2021-01-11T22:29:36Z

what does Nessus scanner treats this as a Medium Risk in kubernetes 1.18.10. mean? @249043822

dims · 2021-01-11T22:35:44Z

@deads2k looks like we are just trying to make a specific scanner happy. are we going to get a deluge of more such PRs?

brandond · 2021-01-11T23:42:12Z

@dims It means someone pointed at security scanning tool at a host running the apiserver and it came back complaining about a HTTPS 'website' not returning HSTS headers. To me these really only make sense to send for a site serving browser traffic, but it doesn't seem like it hurts anything to let someone configure them if it gets their infosec team off their back.

249043822 · 2021-01-12T09:03:54Z

what does Nessus scanner treats this as a Medium Risk in kubernetes 1.18.10. mean? @249043822

@dims The Nessus Scanner report as followers:

MEDIUM HSTS Missing From HTTPS Server
Description
The remote HTTPS server is not enforcing HTTP Strict Transport Security (HSTS). HSTS is an optional response header that can be configured on the server to instruct the browser to only communicate via HTTPS. The lack of HSTS allows downgrade attacks, SSL-stripping man-in-the-middle attacks, and weakens cookie-hijacking protections.

Solution
Configure the remote web server to use HSTS.

See Also
https://tools.ietf.org/html/rfc6797

Output
The remote HTTPS server does not send the HTTP
"Strict-Transport-Security" header.

Port	Hosts
6443 / tcp / www

249043822 · 2021-01-12T09:05:02Z

Yea, all hsts directives go into a single header

Sorry about this, I hadn't read in detail before. Can you replace all the headers with directives to match the RFC. The variables and the flags.

/approve

After we get the names fixed, I think this is ready to go.

@deads2k I have fixed these.

249043822 · 2021-01-12T10:36:03Z

/retest

caesarxuchao · 2021-01-12T21:06:32Z

/sig-remove api-machinery

caesarxuchao · 2021-01-12T21:07:09Z

/remove-sig api-machinery

deads2k · 2021-01-13T13:30:43Z

The feedback from the website PR is that the doc is updated automatically.

Please don't do this.
This file is autogenerated, not for direct editing.
The related flag will show up when the feature is in the next release.

The release-note has been specified.

/lgtm
/approve

k8s-ci-robot · 2021-01-13T13:31:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 249043822, deads2k

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~staging/src/k8s.io/apiserver/OWNERS~~ [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Nov 12, 2020

k8s-ci-robot requested review from smarterclayton and sttts November 12, 2020 08:18

k8s-ci-robot added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. labels Nov 12, 2020

JornShen reviewed Nov 13, 2020

View reviewed changes

staging/src/k8s.io/apiserver/pkg/server/filters/hsts.go Outdated Show resolved Hide resolved

249043822 force-pushed the br-hsts branch from ce0dd65 to 42019a5 Compare November 13, 2020 04:24

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Nov 13, 2020

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Nov 17, 2020

k8s-ci-robot assigned deads2k and mikedanese Nov 27, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 27, 2020

deads2k reviewed Jan 8, 2021

View reviewed changes

staging/src/k8s.io/apiserver/pkg/server/filters/hsts.go Outdated Show resolved Hide resolved

deads2k reviewed Jan 8, 2021

View reviewed changes

staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go Show resolved Hide resolved

249043822 force-pushed the br-hsts branch from c9bc66f to 32ae143 Compare January 9, 2021 04:55

deads2k reviewed Jan 11, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 11, 2021

Fix HSTS Missing From HTTPS Server(Nessus Scanner)

4801f4d

249043822 force-pushed the br-hsts branch from 32ae143 to 4801f4d Compare January 12, 2021 08:43

k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 12, 2021

249043822 mentioned this pull request Jan 13, 2021

add --strict-transport-security-directives support for apiserver kubernetes/website#26069

Closed

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jan 13, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 13, 2021

k8s-ci-robot merged commit 3351827 into kubernetes:master Jan 13, 2021

k8s-ci-robot added this to the v1.21 milestone Jan 13, 2021

github-actions bot mentioned this pull request Jan 19, 2021

Week Ending January 17, 2021 dev-obs/actus#320

Open

249043822 mentioned this pull request Mar 16, 2021

add integration test for apiserver hsts #100285

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix HSTS Missing From HTTPS Server(Nessus Scanner) #96502

Fix HSTS Missing From HTTPS Server(Nessus Scanner) #96502

249043822 commented Nov 12, 2020 •

edited by deads2k

k8s-ci-robot commented Nov 12, 2020

k8s-ci-robot commented Nov 12, 2020

fedebongio commented Nov 12, 2020

leilajal commented Nov 17, 2020

249043822 commented Nov 27, 2020

249043822 commented Nov 27, 2020

249043822 commented Nov 27, 2020

liggitt commented Nov 27, 2020 •

edited

deads2k Jan 8, 2021

deads2k Jan 8, 2021

deads2k Jan 8, 2021

249043822 Jan 9, 2021

deads2k Jan 11, 2021

deads2k commented Jan 11, 2021

dims commented Jan 11, 2021

dims commented Jan 11, 2021

brandond commented Jan 11, 2021 •

edited

249043822 commented Jan 12, 2021

249043822 commented Jan 12, 2021

249043822 commented Jan 12, 2021

caesarxuchao commented Jan 12, 2021

caesarxuchao commented Jan 12, 2021

deads2k commented Jan 13, 2021

k8s-ci-robot commented Jan 13, 2021

Fix HSTS Missing From HTTPS Server(Nessus Scanner) #96502

Fix HSTS Missing From HTTPS Server(Nessus Scanner) #96502

Conversation

249043822 commented Nov 12, 2020 • edited by deads2k

k8s-ci-robot commented Nov 12, 2020

k8s-ci-robot commented Nov 12, 2020

fedebongio commented Nov 12, 2020

leilajal commented Nov 17, 2020

249043822 commented Nov 27, 2020

249043822 commented Nov 27, 2020

249043822 commented Nov 27, 2020

liggitt commented Nov 27, 2020 • edited

deads2k Jan 8, 2021

Choose a reason for hiding this comment

deads2k Jan 8, 2021

Choose a reason for hiding this comment

deads2k Jan 8, 2021

Choose a reason for hiding this comment

249043822 Jan 9, 2021

Choose a reason for hiding this comment

deads2k Jan 11, 2021

Choose a reason for hiding this comment

deads2k commented Jan 11, 2021

dims commented Jan 11, 2021

dims commented Jan 11, 2021

brandond commented Jan 11, 2021 • edited

249043822 commented Jan 12, 2021

249043822 commented Jan 12, 2021

249043822 commented Jan 12, 2021

caesarxuchao commented Jan 12, 2021

caesarxuchao commented Jan 12, 2021

deads2k commented Jan 13, 2021

k8s-ci-robot commented Jan 13, 2021

249043822 commented Nov 12, 2020 •

edited by deads2k

liggitt commented Nov 27, 2020 •

edited

brandond commented Jan 11, 2021 •

edited