[
https://issues.apache.org/jira/browse/YARN-8198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16456334#comment-16456334
]
Szilard Nemeth edited comment on YARN-8198 at 4/27/18 1:03 PM:
---------------------------------------------------------------
1. HttpServer2 in general about regex usage:
You use {{HTTP_HEADER_REGEX}} in 3 places:
- addHeaders
- setHeaders (2 occurences)
A.) In {{addHeaders()}}, you could simply use {{String.startsWith()}} because
AFAIK you don't validate with the regex, you just check whether the String
starts with a prefix.
B.) In {{setHeaders()}}, you call {{conf.getValByRegex()}}, I think if you just
leverage the startsWith capability of your regex, you could just call
{{conf.getPropsWithPrefix()}} because it's more lightweight than a regex
matching.
If you still decide to keep regex-matching instead of simple String operations,
I think you should fix these:
A.) {{setHeaders()}}: you call {{conf.getValByRegex()}}, this method returns
every config matching for a regex. Then you forEach on the {{headerConfigMap}}
and do the same regex-matching that the {{getValByRegex}} did, which is not
needed.
Then, you could just simply call {{xFrameParams.putAll(headerConfigMap);}} as
the last step of this method.
B.) {{setHeaders()}}: if you want to ensure that no config is present with name
"hadoop.http.header.X", where X does not match for your regex ([a-zA-Z\-_]+), I
think you should indicate the unmatched values with a log message (warning?).
C.) You should call {{Pattern.compile(HTTP_HEADER_REGEX)}} only once and save
it to a static final field of {{HttpServer2}}.
You have 2 occurrences of this pattern call, both in a loop, which is costly
and unnecessarry to perform every time.
D.) In your {{HTTP_HEADER_REGEX}}, I think the first regex group is not
relevant as you are only interested in what follows "hadoop.http.header." I
guess.
- {{HttpServer2.addHeaders()}}:
You have: {{String value = config.getInitParameter(key);]]
You don't use the value local variable, I guess you wanted to use it on the
next line.
– {{HttpServer2.getDefaultHeaders()}}: You used split twice on both headers,
you could store the split results in arrays and access the elements from 0th
and 1st index.
2. HttpServer2 (minor thing): As you only use {{getDefaultHeaders}} from
{{setHeaders}}, I would put the {{getDefaultHeaders}} method under the
{{setHeaders}} method.
3. HttpServer2: introduced constant Strings could be package private.
was (Author: snemeth):
- HttpServer2 (minor thing): As you only use {{getDefaultHeaders}} from
{{setHeaders}}, I would put the {{getDefaultHeaders}} method under the
{{setHeaders}} method.
- HttpServer2: introduced constant Strings could be package private.
- *HttpServer2 in general about regex usage: You use {{HTTP_HEADER_REGEX}} in 3
places: *
- addHeaders
- setHeaders (2 occurences)
In {{addHeaders()}}, you could simply use {{String.startsWith()}} because AFAIK
you don't validate with the regex, you just check whether the String starts
with a prefix.
In {{setHeaders()}}, you call {{conf.getValByRegex()}}, I think if you just
leverage the startsWith capability of your regex, you could just call
{{conf.getPropsWithPrefix()}} because it's more lightweight than a regex
matching.
If you still decide to keep regex-matching instead of simple String
operations, I think you should fix these:
- {{setHeaders()}}: you call {{conf.getValByRegex()}}, this method returns
every config matching for a regex.
Then you forEach on the {{headerConfigMap}} and do the same regex-matching
that the {{getValByRegex}} did, which is not needed.
You could just simply call {{xFrameParams.putAll(headerConfigMap);}} as the
last step of this method.
- {{setHeaders()}}: if you want to ensure that no config is present with name
"hadoop.http.header.X", where X does not match for your regex ([a-zA-Z\-_]+), I
think you should indicate the unmatched values with a log message (warning?).
- You should call {{Pattern.compile(HTTP_HEADER_REGEX)}} only once and save it
to a static final field of {{HttpServer2}}.
You have 2 occurences of this pattern call, both in a loop, which is costly
and unnecessarry to perform every time.
- In your {{HTTP_HEADER_REGEX}}, I think the first regex group is not relevant
as you are only interested in what follows "hadoop.http.header." I guess.
- {{HttpServer2.addHeaders()}}:
You have: {{String value = config.getInitParameter(key);]]
You don't use the value local variable, I guess you wanted to use it on the
next line.
- {{HttpServer2.getDefaultHeaders()}}: You used split twice on both headers,
you could store the split results in arrays and access the elements from 0th
and 1st index.
> Add Security-Related HTTP Response Header in Yarn WEBUIs.
> ---------------------------------------------------------
>
> Key: YARN-8198
> URL: https://issues.apache.org/jira/browse/YARN-8198
> Project: Hadoop YARN
> Issue Type: Improvement
> Components: yarn
> Reporter: Kanwaljeet Sachdev
> Assignee: Kanwaljeet Sachdev
> Priority: Major
> Labels: security
> Attachments: YARN-8198.001.patch, YARN-8198.002.patch,
> YARN-8198.003.patch
>
>
> As of today, YARN web-ui lacks certain security related http response
> headers. We are planning to add few default ones and also add support for
> headers to be able to get added via xml config. Planning to make the below
> two as default.
> * X-XSS-Protection: 1; mode=block
> * X-Content-Type-Options: nosniff
>
> Support for headers via config properties in core-site.xml will be along the
> below lines
> {code:java}
> <property>
> <name>hadoop.http.header.Strict_Transport_Security</name>
> <value>valHSTSFromXML</value>
> </property>{code}
>
> A regex matcher will lift these properties and add into the response header
> when Jetty prepares the response.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
