Add CSP information to Security Panel.

Add CSP information to Security Panel.

BUG=588970

[Jonathan Garbee, 2016-02-26 16:58:13]

jonathan.garbee@chromium.org changed reviewers:
+ lgarron@chromium.org, paulirish@chromium.org

[Jonathan Garbee, 2016-02-26 17:00:27]

PTAL

Provides CSP in a pretty fashion under the Security panel. Along with warnings
for possibly unsafe modes.

Directives also are given a security state based on the lowest-state within
their group. So it is very easy to skim over and see which groups may have
issues.

Example output currently in place: http://i.imgur.com/1D4UhtW.png

[lgarron, 2016-02-26 23:21:42]

On 2016/02/26 at 17:00:27, jonathan.garbee wrote:
> PTAL
> 
> Provides CSP in a pretty fashion under the Security panel. Along with warnings
for possibly unsafe modes.
> 
> Directives also are given a security state based on the lowest-state within
their group. So it is very easy to skim over and see which groups may have
issues.
> 
> Example output currently in place: http://i.imgur.com/1D4UhtW.png

Overall, I like the idea!

Some concerns right off the bat:
- Color should never be the only way a piece of information is communicated
(accessibility).
- In the Security panel, things should only be marked as "secure" or "insecure"
if they affect the lock icon. Everything else is informational. What about using
non-colored checkmarks and exclamation icons?
- Unlike most information in the origin view, there could be a large variation
in CSP for the same origin, right? If there's a significant chance this will be
the case, we should improve https://crbug.com/503170 to handle different details
for the same origin.
- These lists can be long. How about collapsing at least all the ones we're okay
with (similar to the SAN field)?
- Would it be useful enough to show CSP for the main page only? My working idea
has been to introduce a "Security Policy" sidebar item below "Overview" instead
of putting it with the origin view.

Would you mind talking to maxwalker@ to talk about the design for this? I think
the functionality in this CL is sufficiently useful, but I'm sure Max would have
good thoughts on how to fit it into DevTools most naturally.

Note: I haven't taken a close look at the Javascript, but it looks reasonable at
a quick glance. (I didn't know we used arrow functions in DevTools yet, but
apparently we do! :-)

[neil.matatall, 2016-02-27 00:12:55]

Would you consider sorting the source expressions within the directives? Perhaps
bubbling up things like unsafe-eval/inline and sorting the host sources
alphabetically? Often I just want to confirm whether or not a given host is
contained in a directive and a directive with many host values can make that
difficult.

[Jonathan Garbee, 2016-02-27 00:38:07]

On 2016/02/26 at 23:21:42, lgarron wrote:
> On 2016/02/26 at 17:00:27, jonathan.garbee wrote:
> > PTAL
> > 
> > Provides CSP in a pretty fashion under the Security panel. Along with
warnings for possibly unsafe modes.
> > 
> > Directives also are given a security state based on the lowest-state within
their group. So it is very easy to skim over and see which groups may have
issues.
> > 
> > Example output currently in place: http://i.imgur.com/1D4UhtW.png
> 
> Overall, I like the idea!
> 
> Some concerns right off the bat:
> - Color should never be the only way a piece of information is communicated
(accessibility).
> - In the Security panel, things should only be marked as "secure" or
"insecure" if they affect the lock icon. Everything else is informational. What
about using non-colored checkmarks and exclamation icons?
> - Unlike most information in the origin view, there could be a large variation
in CSP for the same origin, right? If there's a significant chance this will be
the case, we should improve https://crbug.com/503170 to handle different details
for the same origin.
> - These lists can be long. How about collapsing at least all the ones we're
okay with (similar to the SAN field)?
> - Would it be useful enough to show CSP for the main page only? My working
idea has been to introduce a "Security Policy" sidebar item below "Overview"
instead of putting it with the origin view.
> 
> Would you mind talking to maxwalker@ to talk about the design for this? I
think the functionality in this CL is sufficiently useful, but I'm sure Max
would have good thoughts on how to fit it into DevTools most naturally.
> 
> Note: I haven't taken a close look at the Javascript, but it looks reasonable
at a quick glance. (I didn't know we used arrow functions in DevTools yet, but
apparently we do! :-)

Yup, I'll get in touch with max about UX ideas. This was a quick hack to get the
idea going. From what you are describing though, I should move to a
table-structure for the content and have a column that shows Good/Possibly
Unsafe/ Unsafe as well as a third column for "Recommendation" in case we have
any. Then decide on how/if to apply color hints one that round is done (or if
Max has a good idea up front for color hints.)

Collapsing I didn't do on the first pass since doing that with a definition list
is not-straight-forward and the logic would get complicated. However, if we go
to a table then collapsing is trivial.

On showing it for the main page only... Could this be a problem with iframed
content? The current implementation (untested) should show CSP for any frame
which provides it. Could be useful in the case of iframed content to see what
the rules are perhaps.

On the variation within the same origin... I'm not sure how likely the case is
here. I think this functionality is a good first-pass to get it out, then if
that case comes up we can refactor later to address it. Sound good?

Thanks for the feedback!

[Jonathan Garbee, 2016-02-27 00:39:46]

On 2016/02/27 at 00:12:55, neil.matatall wrote:
> Would you consider sorting the source expressions within the directives?
Perhaps bubbling up things like unsafe-eval/inline and sorting the host sources
alphabetically? Often I just want to confirm whether or not a given host is
contained in a directive and a directive with many host values can make that
difficult.

Yea, I can find a way to sort these. We can pull up anything with a single-quote
to be first then show the domains ordered alphabetically by domain. Good
thinking.

[lgarron, 2016-02-27 00:42:32]

On 2016/02/27 at 00:38:07, jonathan.garbee wrote:
> On 2016/02/26 at 23:21:42, lgarron wrote:
> > On 2016/02/26 at 17:00:27, jonathan.garbee wrote:
> > > PTAL
> > > 
> > > Provides CSP in a pretty fashion under the Security panel. Along with
warnings for possibly unsafe modes.
> > > 
> > > Directives also are given a security state based on the lowest-state
within their group. So it is very easy to skim over and see which groups may
have issues.
> > > 
> > > Example output currently in place: http://i.imgur.com/1D4UhtW.png
> > 
> > Overall, I like the idea!
> > 
> > Some concerns right off the bat:
> > - Color should never be the only way a piece of information is communicated
(accessibility).
> > - In the Security panel, things should only be marked as "secure" or
"insecure" if they affect the lock icon. Everything else is informational. What
about using non-colored checkmarks and exclamation icons?
> > - Unlike most information in the origin view, there could be a large
variation in CSP for the same origin, right? If there's a significant chance
this will be the case, we should improve https://crbug.com/503170 to handle
different details for the same origin.
> > - These lists can be long. How about collapsing at least all the ones we're
okay with (similar to the SAN field)?
> > - Would it be useful enough to show CSP for the main page only? My working
idea has been to introduce a "Security Policy" sidebar item below "Overview"
instead of putting it with the origin view.
> > 
> > Would you mind talking to maxwalker@ to talk about the design for this? I
think the functionality in this CL is sufficiently useful, but I'm sure Max
would have good thoughts on how to fit it into DevTools most naturally.
> > 
> > Note: I haven't taken a close look at the Javascript, but it looks
reasonable at a quick glance. (I didn't know we used arrow functions in DevTools
yet, but apparently we do! :-)
> 
> Yup, I'll get in touch with max about UX ideas. This was a quick hack to get
the idea going. From what you are describing though, I should move to a
table-structure for the content and have a column that shows Good/Possibly
Unsafe/ Unsafe as well as a third column for "Recommendation" in case we have
any. Then decide on how/if to apply color hints one that round is done (or if
Max has a good idea up front for color hints.)
> 
> Collapsing I didn't do on the first pass since doing that with a definition
list is not-straight-forward and the logic would get complicated. However, if we
go to a table then collapsing is trivial.
> 
> On showing it for the main page only... Could this be a problem with iframed
content? The current implementation (untested) should show CSP for any frame
which provides it. Could be useful in the case of iframed content to see what
the rules are perhaps.
> 
> On the variation within the same origin... I'm not sure how likely the case is
here. I think this functionality is a good first-pass to get it out, then if
that case comes up we can refactor later to address it. Sound good?
> 
> Thanks for the feedback!

If iframes are a use case we care about, then I think we should find a way to
make it very clear which CSP you are looking at. I can easily imagine multiple
frames from the same origin with different CSP, if we're already considering
niche cases for CSP below the main document.

Some questions, because I don't  know enough about CSP:

Exactly where can sites send CSP that the browser will honor?
- The original page (meta tag)
- The original page (header)
- Any iframe (meta tag)
- Any iframe (header)

Can we surface where each CSP directive came from (e.g. header vs. meta tag)? Is
that information either available or easy to plumb?
Something like that sounds useful for debugging to me.

[Jonathan Garbee, 2016-02-27 00:46:23]

Ah right, meta tags as well. Forgot about that directive.

We can easily tell, since we get the headers via the request itself and
meta tag stuff will need to be asked for. So it is just an extra property
added in the configuration loops to tell which came from where.

Let me look at the specification again tomorrow for whether iframe's are
honored. If I recall correctly they are, but I'd rather see exactly to what
extent.

On Fri, Feb 26, 2016 at 7:42 PM, <lgarron@chromium.org> wrote:

> On 2016/02/27 at 00:38:07, jonathan.garbee wrote:
> > On 2016/02/26 at 23:21:42, lgarron wrote:
> > > On 2016/02/26 at 17:00:27, jonathan.garbee wrote:
> > > > PTAL
> > > >
> > > > Provides CSP in a pretty fashion under the Security panel. Along with
> warnings for possibly unsafe modes.
> > > >
> > > > Directives also are given a security state based on the lowest-state
> within their group. So it is very easy to skim over and see which groups
> may
> have issues.
> > > >
> > > > Example output currently in place: http://i.imgur.com/1D4UhtW.png
> > >
> > > Overall, I like the idea!
> > >
> > > Some concerns right off the bat:
> > > - Color should never be the only way a piece of information is
> communicated
> (accessibility).
> > > - In the Security panel, things should only be marked as "secure" or
> "insecure" if they affect the lock icon. Everything else is informational.
> What
> about using non-colored checkmarks and exclamation icons?
> > > - Unlike most information in the origin view, there could be a large
> variation in CSP for the same origin, right? If there's a significant
> chance
> this will be the case, we should improve https://crbug.com/503170 to
> handle
> different details for the same origin.
> > > - These lists can be long. How about collapsing at least all the ones
> we're
> okay with (similar to the SAN field)?
> > > - Would it be useful enough to show CSP for the main page only? My
> working
> idea has been to introduce a "Security Policy" sidebar item below
> "Overview"
> instead of putting it with the origin view.
> > >
> > > Would you mind talking to maxwalker@ to talk about the design for
> this? I
> think the functionality in this CL is sufficiently useful, but I'm sure Max
> would have good thoughts on how to fit it into DevTools most naturally.
> > >
> > > Note: I haven't taken a close look at the Javascript, but it looks
> reasonable at a quick glance. (I didn't know we used arrow functions in
> DevTools
> yet, but apparently we do! :-)
> >
> > Yup, I'll get in touch with max about UX ideas. This was a quick hack to
> get
> the idea going. From what you are describing though, I should move to a
> table-structure for the content and have a column that shows Good/Possibly
> Unsafe/ Unsafe as well as a third column for "Recommendation" in case we
> have
> any. Then decide on how/if to apply color hints one that round is done (or
> if
> Max has a good idea up front for color hints.)
> >
> > Collapsing I didn't do on the first pass since doing that with a
> definition
> list is not-straight-forward and the logic would get complicated. However,
> if we
> go to a table then collapsing is trivial.
> >
> > On showing it for the main page only... Could this be a problem with
> iframed
> content? The current implementation (untested) should show CSP for any
> frame
> which provides it. Could be useful in the case of iframed content to see
> what
> the rules are perhaps.
> >
> > On the variation within the same origin... I'm not sure how likely the
> case is
> here. I think this functionality is a good first-pass to get it out, then
> if
> that case comes up we can refactor later to address it. Sound good?
> >
> > Thanks for the feedback!
>
> If iframes are a use case we care about, then I think we should find a way
> to
> make it very clear which CSP you are looking at. I can easily imagine
> multiple
> frames from the same origin with different CSP, if we're already
> considering
> niche cases for CSP below the main document.
>
> Some questions, because I don't know enough about CSP:
>
> Exactly where can sites send CSP that the browser will honor?
> - The original page (meta tag)
> - The original page (header)
> - Any iframe (meta tag)
> - Any iframe (header)
>
> Can we surface where each CSP directive came from (e.g. header vs. meta
> tag)? Is
> that information either available or easy to plumb?
> Something like that sounds useful for debugging to me.
>
> https://codereview.chromium.org/1742743002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Jonathan Garbee, 2016-02-27 00:46:23]

Ah right, meta tags as well. Forgot about that directive.

We can easily tell, since we get the headers via the request itself and
meta tag stuff will need to be asked for. So it is just an extra property
added in the configuration loops to tell which came from where.

Let me look at the specification again tomorrow for whether iframe's are
honored. If I recall correctly they are, but I'd rather see exactly to what
extent.

On Fri, Feb 26, 2016 at 7:42 PM, <lgarron@chromium.org> wrote:

> On 2016/02/27 at 00:38:07, jonathan.garbee wrote:
> > On 2016/02/26 at 23:21:42, lgarron wrote:
> > > On 2016/02/26 at 17:00:27, jonathan.garbee wrote:
> > > > PTAL
> > > >
> > > > Provides CSP in a pretty fashion under the Security panel. Along with
> warnings for possibly unsafe modes.
> > > >
> > > > Directives also are given a security state based on the lowest-state
> within their group. So it is very easy to skim over and see which groups
> may
> have issues.
> > > >
> > > > Example output currently in place: http://i.imgur.com/1D4UhtW.png
> > >
> > > Overall, I like the idea!
> > >
> > > Some concerns right off the bat:
> > > - Color should never be the only way a piece of information is
> communicated
> (accessibility).
> > > - In the Security panel, things should only be marked as "secure" or
> "insecure" if they affect the lock icon. Everything else is informational.
> What
> about using non-colored checkmarks and exclamation icons?
> > > - Unlike most information in the origin view, there could be a large
> variation in CSP for the same origin, right? If there's a significant
> chance
> this will be the case, we should improve https://crbug.com/503170 to
> handle
> different details for the same origin.
> > > - These lists can be long. How about collapsing at least all the ones
> we're
> okay with (similar to the SAN field)?
> > > - Would it be useful enough to show CSP for the main page only? My
> working
> idea has been to introduce a "Security Policy" sidebar item below
> "Overview"
> instead of putting it with the origin view.
> > >
> > > Would you mind talking to maxwalker@ to talk about the design for
> this? I
> think the functionality in this CL is sufficiently useful, but I'm sure Max
> would have good thoughts on how to fit it into DevTools most naturally.
> > >
> > > Note: I haven't taken a close look at the Javascript, but it looks
> reasonable at a quick glance. (I didn't know we used arrow functions in
> DevTools
> yet, but apparently we do! :-)
> >
> > Yup, I'll get in touch with max about UX ideas. This was a quick hack to
> get
> the idea going. From what you are describing though, I should move to a
> table-structure for the content and have a column that shows Good/Possibly
> Unsafe/ Unsafe as well as a third column for "Recommendation" in case we
> have
> any. Then decide on how/if to apply color hints one that round is done (or
> if
> Max has a good idea up front for color hints.)
> >
> > Collapsing I didn't do on the first pass since doing that with a
> definition
> list is not-straight-forward and the logic would get complicated. However,
> if we
> go to a table then collapsing is trivial.
> >
> > On showing it for the main page only... Could this be a problem with
> iframed
> content? The current implementation (untested) should show CSP for any
> frame
> which provides it. Could be useful in the case of iframed content to see
> what
> the rules are perhaps.
> >
> > On the variation within the same origin... I'm not sure how likely the
> case is
> here. I think this functionality is a good first-pass to get it out, then
> if
> that case comes up we can refactor later to address it. Sound good?
> >
> > Thanks for the feedback!
>
> If iframes are a use case we care about, then I think we should find a way
> to
> make it very clear which CSP you are looking at. I can easily imagine
> multiple
> frames from the same origin with different CSP, if we're already
> considering
> niche cases for CSP below the main document.
>
> Some questions, because I don't know enough about CSP:
>
> Exactly where can sites send CSP that the browser will honor?
> - The original page (meta tag)
> - The original page (header)
> - Any iframe (meta tag)
> - Any iframe (header)
>
> Can we surface where each CSP directive came from (e.g. header vs. meta
> tag)? Is
> that information either available or easy to plumb?
> Something like that sounds useful for debugging to me.
>
> https://codereview.chromium.org/1742743002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[paulirish, 2016-02-27 02:36:46]

It'd make sense to move the CSP parsing into NetworkRequest. (or it's own file
like CookieParser)

[Jonathan Garbee, 2016-02-27 10:26:46]

Well, own file is the best choice. It isn't strictly network bound due to
meta tags also being a possibility. I'll take a look at that as I'm doing
the initial refactor in a few minutes.
On Feb 26, 2016 9:36 PM, <paulirish@chromium.org> wrote:

> It'd make sense to move the CSP parsing into NetworkRequest. (or it's own
> file
> like CookieParser)
>
> https://codereview.chromium.org/1742743002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Jonathan Garbee, 2016-02-27 10:26:46]

Well, own file is the best choice. It isn't strictly network bound due to
meta tags also being a possibility. I'll take a look at that as I'm doing
the initial refactor in a few minutes.
On Feb 26, 2016 9:36 PM, <paulirish@chromium.org> wrote:

> It'd make sense to move the CSP parsing into NetworkRequest. (or it's own
> file
> like CookieParser)
>
> https://codereview.chromium.org/1742743002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Jonathan Garbee, 2016-02-29 22:01:51]

On 2016/02/27 at 10:26:46, Jonathan Garbee wrote:
> Well, own file is the best choice. It isn't strictly network bound due to
> meta tags also being a possibility. I'll take a look at that as I'm doing
> the initial refactor in a few minutes.
> On Feb 26, 2016 9:36 PM, <paulirish@chromium.org> wrote:
> 
> > It'd make sense to move the CSP parsing into NetworkRequest. (or it's own
> > file
> > like CookieParser)
> >
> > https://codereview.chromium.org/1742743002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.
> 

Intermediary patchset is up.

Contentious thing: Using ES6 classes to get simple static functions for easily
building new objects. Needs serious review if we want to start using these,
otherwise I will need to refactor the parser.

What is done so far:
* Table display over definition list.
* Color removed
* Symbols used to determine warning levels.
* Skeleton is added for where I want to improve the UX with an "enforcement"
area to quickly tell the developer whether they are enforcing or reporting and
where to. More refactoring needed before this is functional, but should be soon.
* Collapsable sections with one button to quickly collapse and show all
sections.
* Sorting of each section's content by the value of the rule.

Still needs doing:
* Take into account meta tag on page.
* UX for showing whether a section is hidden or shown. (And any other UX
suggestions Max comes up with.)
* Plumbing out the enforcement reporting URI and level into the policy details
for displaying inline over its own table.
* Investigating iframe's and how engines are supposed to handle them.

http://i.imgur.com/Dl0jgcg.png

[Jonathan Garbee, 2016-03-01 14:59:28]

This mornings patchset:
* Use the warning icon as an arrow indicator for shown/hidden state of
headings.
* Center align headings and border on top and bottom to make them distinct.
Ugly as sin, won't ship so good for now.
* Updated parser to take a boolean for reportOnly mode to be told to it.
* Parser internally finds the report-uri to pull out for construction
during build.
* With the previous two updates, the enforcement area is now functional.
Exact wording needs to be looked at, I feel it could be confusing. Will
look at spec align to how it names things.


Very major thing that needs review, going with all the ES6 goodness... I
used template strings to try and clean up the table row generation and the
enforcement overview area. Easy to roll this back if it is messy/unwanted.

Also, I just tested the iframe scenario. It reports header-set values via
iframes just fine, so all looks to be working fine there.

http://i.imgur.com/UGJg7Qe.png

The only major thing on the implementation side (other than ES6 usage
review/code style) is figuring out how to query up CSP policies set by meta
tags it appears.

On Mon, Feb 29, 2016 at 5:01 PM, <jonathan.garbee@chromium.org> wrote:

> On 2016/02/27 at 10:26:46, Jonathan Garbee wrote:
> > Well, own file is the best choice. It isn't strictly network bound due to
> > meta tags also being a possibility. I'll take a look at that as I'm doing
> > the initial refactor in a few minutes.
> > On Feb 26, 2016 9:36 PM, <paulirish@chromium.org&gt; wrote:
> >
> > > It'd make sense to move the CSP parsing into NetworkRequest. (or it's
> own
> > > file
> > > like CookieParser)
> > >
> > > https://codereview.chromium.org/1742743002/
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
> >
>
> Intermediary patchset is up.
>
> Contentious thing: Using ES6 classes to get simple static functions for
> easily
> building new objects. Needs serious review if we want to start using these,
> otherwise I will need to refactor the parser.
>
> What is done so far:
> * Table display over definition list.
> * Color removed
> * Symbols used to determine warning levels.
> * Skeleton is added for where I want to improve the UX with an
> "enforcement"
> area to quickly tell the developer whether they are enforcing or reporting
> and
> where to. More refactoring needed before this is functional, but should be
> soon.
> * Collapsable sections with one button to quickly collapse and show all
> sections.
> * Sorting of each section's content by the value of the rule.
>
> Still needs doing:
> * Take into account meta tag on page.
> * UX for showing whether a section is hidden or shown. (And any other UX
> suggestions Max comes up with.)
> * Plumbing out the enforcement reporting URI and level into the policy
> details
> for displaying inline over its own table.
> * Investigating iframe's and how engines are supposed to handle them.
>
> http://i.imgur.com/Dl0jgcg.png
>
> https://codereview.chromium.org/1742743002/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[Jonathan Garbee, 2016-03-01 14:59:28]

This mornings patchset:
* Use the warning icon as an arrow indicator for shown/hidden state of
headings.
* Center align headings and border on top and bottom to make them distinct.
Ugly as sin, won't ship so good for now.
* Updated parser to take a boolean for reportOnly mode to be told to it.
* Parser internally finds the report-uri to pull out for construction
during build.
* With the previous two updates, the enforcement area is now functional.
Exact wording needs to be looked at, I feel it could be confusing. Will
look at spec align to how it names things.


Very major thing that needs review, going with all the ES6 goodness... I
used template strings to try and clean up the table row generation and the
enforcement overview area. Easy to roll this back if it is messy/unwanted.

Also, I just tested the iframe scenario. It reports header-set values via
iframes just fine, so all looks to be working fine there.

http://i.imgur.com/UGJg7Qe.png

The only major thing on the implementation side (other than ES6 usage
review/code style) is figuring out how to query up CSP policies set by meta
tags it appears.

On Mon, Feb 29, 2016 at 5:01 PM, <jonathan.garbee@chromium.org> wrote:

> On 2016/02/27 at 10:26:46, Jonathan Garbee wrote:
> > Well, own file is the best choice. It isn't strictly network bound due to
> > meta tags also being a possibility. I'll take a look at that as I'm doing
> > the initial refactor in a few minutes.
> > On Feb 26, 2016 9:36 PM, <paulirish@chromium.org&gt; wrote:
> >
> > > It'd make sense to move the CSP parsing into NetworkRequest. (or it's
> own
> > > file
> > > like CookieParser)
> > >
> > > https://codereview.chromium.org/1742743002/
> > >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
> >
>
> Intermediary patchset is up.
>
> Contentious thing: Using ES6 classes to get simple static functions for
> easily
> building new objects. Needs serious review if we want to start using these,
> otherwise I will need to refactor the parser.
>
> What is done so far:
> * Table display over definition list.
> * Color removed
> * Symbols used to determine warning levels.
> * Skeleton is added for where I want to improve the UX with an
> "enforcement"
> area to quickly tell the developer whether they are enforcing or reporting
> and
> where to. More refactoring needed before this is functional, but should be
> soon.
> * Collapsable sections with one button to quickly collapse and show all
> sections.
> * Sorting of each section's content by the value of the rule.
>
> Still needs doing:
> * Take into account meta tag on page.
> * UX for showing whether a section is hidden or shown. (And any other UX
> suggestions Max comes up with.)
> * Plumbing out the enforcement reporting URI and level into the policy
> details
> for displaying inline over its own table.
> * Investigating iframe's and how engines are supposed to handle them.
>
> http://i.imgur.com/Dl0jgcg.png
>
> https://codereview.chromium.org/1742743002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[paulirish, 2016-03-01 20:44:49]

The new UI doesn't work as well for me. 
I much preferred the straightforward list, and similar to how
https://report-uri.io/ presents it. We could expose the `set by` content in a
tooltip.

[Jonathan Garbee, 2016-03-15 18:17:56]

On 2016/03/01 at 20:44:49, paulirish wrote:
> The new UI doesn't work as well for me. 
> I much preferred the straightforward list, and similar to how
https://report-uri.io/ presents it. We could expose the `set by` content in a
tooltip.

I'm going to add in a quick idea here for showing deprecation messages as well.
For example `frame-src` is deprecated in favor of `child-src` [1]. We can
surface this information nicely to developers in the sec panel UX.

While I'm adding that in, thinking about getting this functionality out for
feedback in M51, we should ship it behind an experiment. This way the code can
start land and get tested by developers while we wait on UX to be figured out
fully. I'll make a bug specifically for discussing and implementing an improved
interface once the experiment has landed. That way we at least have a good idea
of the starting data that needs to be handled.


[1]
https://developer.mozilla.org/en-US/docs/Web/Security/CSP/CSP_policy_directiv...

[Jonathan Garbee, 2016-03-15 23:32:53]

On 2016/03/15 at 18:17:56, Jonathan Garbee wrote:
> On 2016/03/01 at 20:44:49, paulirish wrote:
> > The new UI doesn't work as well for me. 
> > I much preferred the straightforward list, and similar to how
https://report-uri.io/ presents it. We could expose the `set by` content in a
tooltip.
> 
> I'm going to add in a quick idea here for showing deprecation messages as
well. For example `frame-src` is deprecated in favor of `child-src` [1]. We can
surface this information nicely to developers in the sec panel UX.
> 
> While I'm adding that in, thinking about getting this functionality out for
feedback in M51, we should ship it behind an experiment. This way the code can
start land and get tested by developers while we wait on UX to be figured out
fully. I'll make a bug specifically for discussing and implementing an improved
interface once the experiment has landed. That way we at least have a good idea
of the starting data that needs to be handled.
> 
> 
> [1]
https://developer.mozilla.org/en-US/docs/Web/Security/CSP/CSP_policy_directiv...


Latest patchset brings in the overview security state display and a place for
deprecation warnings to be put. This will encourage developers to know what
rules to use and replace ones that may no longer be supported (or that have a
superior choice.) These remain visible always, only the tables (the bulk of the
display) collapse down. So developers can get a quick overview of the states and
only dig into ones with issues if they wish.

Also, big refactor of constructing the display. Instead of `innerHTML` all the
time I create the top-level elements, then inject the inner contents using
template strings for simplicity and append those. That way the event listeners
that get bound will remain bound properly. Also made auxiliary functions to help
with building the markup to make things easier to read/maintain hopefully.

http://i.imgur.com/B7e63bD.png

[Jonathan Garbee, 2016-03-16 14:24:33]

On 2016/03/15 at 23:32:53, Jonathan Garbee wrote:
> On 2016/03/15 at 18:17:56, Jonathan Garbee wrote:
> > On 2016/03/01 at 20:44:49, paulirish wrote:
> > > The new UI doesn't work as well for me. 
> > > I much preferred the straightforward list, and similar to how
https://report-uri.io/ presents it. We could expose the `set by` content in a
tooltip.
> > 
> > I'm going to add in a quick idea here for showing deprecation messages as
well. For example `frame-src` is deprecated in favor of `child-src` [1]. We can
surface this information nicely to developers in the sec panel UX.
> > 
> > While I'm adding that in, thinking about getting this functionality out for
feedback in M51, we should ship it behind an experiment. This way the code can
start land and get tested by developers while we wait on UX to be figured out
fully. I'll make a bug specifically for discussing and implementing an improved
interface once the experiment has landed. That way we at least have a good idea
of the starting data that needs to be handled.
> > 
> > 
> > [1]
https://developer.mozilla.org/en-US/docs/Web/Security/CSP/CSP_policy_directiv...
> 
> 
> Latest patchset brings in the overview security state display and a place for
deprecation warnings to be put. This will encourage developers to know what
rules to use and replace ones that may no longer be supported (or that have a
superior choice.) These remain visible always, only the tables (the bulk of the
display) collapse down. So developers can get a quick overview of the states and
only dig into ones with issues if they wish.
> 
> Also, big refactor of constructing the display. Instead of `innerHTML` all the
time I create the top-level elements, then inject the inner contents using
template strings for simplicity and append those. That way the event listeners
that get bound will remain bound properly. Also made auxiliary functions to help
with building the markup to make things easier to read/maintain hopefully.
> 
> http://i.imgur.com/B7e63bD.png

This morning's updates:

* Added experiment for enabling the view.
* Updated CSP Parser to be more clean
   * Uses property getters for things that can be calculated on-the-fly to
reduce the number of parameters on initialization.
   * Provides human readable string properties for rule states. (Capitalizing
the first letter and removing the - from possibly-unsafe.)

[Jonathan Garbee, 2016-03-16 15:28:51]

On 2016/03/15 at 23:32:53, Jonathan Garbee wrote:
> On 2016/03/15 at 18:17:56, Jonathan Garbee wrote:
> > On 2016/03/01 at 20:44:49, paulirish wrote:
> > > The new UI doesn't work as well for me. 
> > > I much preferred the straightforward list, and similar to how
https://report-uri.io/ presents it. We could expose the `set by` content in a
tooltip.
> > 
> > I'm going to add in a quick idea here for showing deprecation messages as
well. For example `frame-src` is deprecated in favor of `child-src` [1]. We can
surface this information nicely to developers in the sec panel UX.
> > 
> > While I'm adding that in, thinking about getting this functionality out for
feedback in M51, we should ship it behind an experiment. This way the code can
start land and get tested by developers while we wait on UX to be figured out
fully. I'll make a bug specifically for discussing and implementing an improved
interface once the experiment has landed. That way we at least have a good idea
of the starting data that needs to be handled.
> > 
> > 
> > [1]
https://developer.mozilla.org/en-US/docs/Web/Security/CSP/CSP_policy_directiv...
> 
> 
> Latest patchset brings in the overview security state display and a place for
deprecation warnings to be put. This will encourage developers to know what
rules to use and replace ones that may no longer be supported (or that have a
superior choice.) These remain visible always, only the tables (the bulk of the
display) collapse down. So developers can get a quick overview of the states and
only dig into ones with issues if they wish.
> 
> Also, big refactor of constructing the display. Instead of `innerHTML` all the
time I create the top-level elements, then inject the inner contents using
template strings for simplicity and append those. That way the event listeners
that get bound will remain bound properly. Also made auxiliary functions to help
with building the markup to make things easier to read/maintain hopefully.
> 

Just dug into the rule stacking a bit, here is the flow I've figured out...

1) Report-Only mode can *only* be set by headers. Due to report-uri being
unavailable via a meta tag.
2) As expected, headers take precedent over Meta tags. If a header is specified
only it takes place. Meta tags can't append to a header ruleset.
3) You can use Report-Only header plus enforce header or meta tag to get a mix
of rules.

In light of this I am proposing (and working on handling this now) the following
modifications:

1) Dump the "set by" (Header/Meta) column for rules. It really isn't useful. If
you are using this stuff, you should know where they get set from and headers
always win.
2) Move enforcement level from the policy level to the rule-level. This way if
someone does mix the enforcement levels it is easy to tell what is being
enforced in the view.

[Jonathan Garbee, 2016-03-16 17:29:19]

On 2016/03/16 at 15:28:51, Jonathan Garbee wrote:
> On 2016/03/15 at 23:32:53, Jonathan Garbee wrote:
> > On 2016/03/15 at 18:17:56, Jonathan Garbee wrote:
> > > On 2016/03/01 at 20:44:49, paulirish wrote:
> > > > The new UI doesn't work as well for me. 
> > > > I much preferred the straightforward list, and similar to how
https://report-uri.io/ presents it. We could expose the `set by` content in a
tooltip.
> > > 
> > > I'm going to add in a quick idea here for showing deprecation messages as
well. For example `frame-src` is deprecated in favor of `child-src` [1]. We can
surface this information nicely to developers in the sec panel UX.
> > > 
> > > While I'm adding that in, thinking about getting this functionality out
for feedback in M51, we should ship it behind an experiment. This way the code
can start land and get tested by developers while we wait on UX to be figured
out fully. I'll make a bug specifically for discussing and implementing an
improved interface once the experiment has landed. That way we at least have a
good idea of the starting data that needs to be handled.
> > > 
> > > 
> > > [1]
https://developer.mozilla.org/en-US/docs/Web/Security/CSP/CSP_policy_directiv...
> > 
> > 
> > Latest patchset brings in the overview security state display and a place
for deprecation warnings to be put. This will encourage developers to know what
rules to use and replace ones that may no longer be supported (or that have a
superior choice.) These remain visible always, only the tables (the bulk of the
display) collapse down. So developers can get a quick overview of the states and
only dig into ones with issues if they wish.
> > 
> > Also, big refactor of constructing the display. Instead of `innerHTML` all
the time I create the top-level elements, then inject the inner contents using
template strings for simplicity and append those. That way the event listeners
that get bound will remain bound properly. Also made auxiliary functions to help
with building the markup to make things easier to read/maintain hopefully.
> > 
> 
> Just dug into the rule stacking a bit, here is the flow I've figured out...
> 
> 1) Report-Only mode can *only* be set by headers. Due to report-uri being
unavailable via a meta tag.
> 2) As expected, headers take precedent over Meta tags. If a header is
specified only it takes place. Meta tags can't append to a header ruleset.
> 3) You can use Report-Only header plus enforce header or meta tag to get a mix
of rules.
> 
> In light of this I am proposing (and working on handling this now) the
following modifications:
> 
> 1) Dump the "set by" (Header/Meta) column for rules. It really isn't useful.
If you are using this stuff, you should know where they get set from and headers
always win.
> 2) Move enforcement level from the policy level to the rule-level. This way if
someone does mix the enforcement levels it is easy to tell what is being
enforced in the view.

Latest patchset has a new UX. This uses a two-column table. First column
contains rule + indicator icon if warranted. Second column has a simple run-down
of the rules traits.

http://i.imgur.com/8EXKYOD.png

[maxwalker, 2016-03-22 17:29:16]

This is really cool!

Some thoughts and a design proposal:
- I would also prefer a denser representation.
- It would be great if the CSP section would fit in with the Connection and
Certificate sections.
- Maybe we could only mark (possibly) unsafe sources. That way they stand out
more and we don't have to add big amounts of icons to the panel.
- As another way to highlight unsafe sources and to reduce vertical space
consumption, we could follow the pattern we already use for SANs: for each
directive show the first two sources and a "Show all" button. If the directive
contains unsafe sources (marked red/yellow) show (all of) them at the top of the
list.
- We could re-use the icons we show for Console warnings and errors.

Mock: http://cl.ly/2P2Z0e3v3T1z
WDYT?

[Jonathan Garbee, 2016-03-22 20:06:08]

On 2016/03/22 at 17:29:16, maxwalker wrote:
> This is really cool!
> 
> Some thoughts and a design proposal:
> - I would also prefer a denser representation.
> - It would be great if the CSP section would fit in with the Connection and
Certificate sections.
> - Maybe we could only mark (possibly) unsafe sources. That way they stand out
more and we don't have to add big amounts of icons to the panel.
> - As another way to highlight unsafe sources and to reduce vertical space
consumption, we could follow the pattern we already use for SANs: for each
directive show the first two sources and a "Show all" button. If the directive
contains unsafe sources (marked red/yellow) show (all of) them at the top of the
list.
> - We could re-use the icons we show for Console warnings and errors.
> 
> Mock: http://cl.ly/2P2Z0e3v3T1z
> WDYT?

I like this design personally, it aligns more with what the original design was.
The only possible issue I see is Lucas doesn't want to use color if it doesn't
affect the TLS lock icon. Which CSP has no affect on the HTTPS state. So, is
Lucas good with having color icons added?

The only thing I see the mock not addressing is a way to indicate whether a
policy is "Enforced" or "Report Only". This is a pretty quirky case since you
*can* have both enforced and report-only policies at the same time.

Any specific reason to-reuse the console icons over the SVG icons we already
have in the sec panel as I'm doing now? (Other than removing the color as per
Lucas's request.)

Thanks for taking a look Max!

[maxwalker, 2016-03-23 10:50:06]

Sure thing - thank you, Jonathan!

Icons: we could use grey scale icons (similar to the info (i) icon we use for
blocked mixed content) to avoid confusion with the lock-icon color coding. I
think it would still be alarming enough (see updated mock).

Report only: as "Report only" is generally more for temporary/experimentation
purposes while "Enforced" is the common setting, could we just add a text-label
on the right when applicable (see style-src row in mock)?

Updated design: http://cl.ly/0g3V1E1K1A0B

[Jonathan Garbee, 2016-03-24 14:04:24]

On 2016/03/23 at 10:50:06, maxwalker wrote:
> Sure thing - thank you, Jonathan!
> 
> Icons: we could use grey scale icons (similar to the info (i) icon we use for
blocked mixed content) to avoid confusion with the lock-icon color coding. I
think it would still be alarming enough (see updated mock).
> 
> Report only: as "Report only" is generally more for temporary/experimentation
purposes while "Enforced" is the common setting, could we just add a text-label
on the right when applicable (see style-src row in mock)?
> 
> Updated design: http://cl.ly/0g3V1E1K1A0B

LGTM now. The only last *possible* issue (need to do more specific testing to
verify behavior) is the "report only" may need to be moved to the heading
column. Since as I understand it the enforcement level should be *policy* level
over *rule* level. However, exact behavior not tested.

That shouldn't be a big deal to just test and move as-needed. I'll get working
on this UX update tomorrow.

[Jonathan Garbee, 2016-03-27 20:46:30]

On 2016/03/24 at 14:04:24, Jonathan Garbee wrote:
> On 2016/03/23 at 10:50:06, maxwalker wrote:
> > Sure thing - thank you, Jonathan!
> > 
> > Icons: we could use grey scale icons (similar to the info (i) icon we use
for blocked mixed content) to avoid confusion with the lock-icon color coding. I
think it would still be alarming enough (see updated mock).
> > 
> > Report only: as "Report only" is generally more for
temporary/experimentation purposes while "Enforced" is the common setting, could
we just add a text-label on the right when applicable (see style-src row in
mock)?
> > 
> > Updated design: http://cl.ly/0g3V1E1K1A0B
> 
> LGTM now. The only last *possible* issue (need to do more specific testing to
verify behavior) is the "report only" may need to be moved to the heading
column. Since as I understand it the enforcement level should be *policy* level
over *rule* level. However, exact behavior not tested.
> 
> That shouldn't be a big deal to just test and move as-needed. I'll get working
on this UX update tomorrow.

Looking over the Level 2 specification just now, things for the parser are
getting quite a bit more complicated. A party can send *multiple* headers, then
only the rules that exist within *both* must be enforced. Which means the parser
needs to be able to have a rulset, then compare to another and look for the same
rules. Which gets hairy due to `default-src` also coming into play.

Lucas, do you think it is a good idea to try and build something that would call
the CSP Policy being enforced out of the internal parser [1]? That seems like
the best method to me, since we'd then have an exact JSON representation of what
is being enforced that we can then display helpful information about. Or should
I simply build all of this logic in our frontend JS?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...