-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IT-3558: Set Strict-Transport-Security on HTTPS responses, as requested by Pentester #104
Conversation
@Override | ||
public void doGet(HttpServletRequest req, HttpServletResponse resp) | ||
throws IOException { | ||
try { | ||
if (req.isSecure()) { | ||
resp.setHeader(StrictTransportSecurityHeaderName, StrictTransportSecurityHeaderValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setHeader
is used here and subsequently used in doGetIntern
method as well. according to the java docs the setHeader
will replace the existing header value. I assume we want to send multiple headers in some cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to the java docs the setHeader will replace the existing header value
No, I believe you are misreading the doc'. setHeader
will only override an existing header with the same name, i.e., an existing header named, "Strict-Transport-Security". Since we are only adding this header in one place, there will be no collision.
@Override | ||
public void doGet(HttpServletRequest req, HttpServletResponse resp) | ||
throws IOException { | ||
try { | ||
if (req.isSecure()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not sure this check is necessary since it seems like browsers will ignore the header if http is used.. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security#description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check might not be necessary, however (from the page you link):
The Strict-Transport-Security header is ignored by the browser when your site has only been accessed using HTTP. Once your site is accessed over HTTPS with no certificate errors, the browser knows your site is HTTPS capable and will honor the Strict-Transport-Security header. Browsers do this as attackers may intercept HTTP connections to the site and inject or remove the header.
So there's no point in adding the header for HTTP
requests.
Set Strict-Transport-Security on HTTPS responses, as requested by Pentester
The response should have a
Strict-Transport-Security
header, but only if the scheme is https, not http.