Skip to content

Conversation

@alex-hofsteede
Copy link
Contributor

Refactored the CSP endpoint and helpers to allow for support of HPKP violation reports. Added schema validation and filtering logic to make it easier to add more interface schemas in future.

Requires some more tests.

@alex-hofsteede alex-hofsteede requested a review from a team October 24, 2017 18:31
@ghost
Copy link

ghost commented Oct 24, 2017

1 Warning
⚠️ You should update CHANGES due to the size of this PR

Generated by 🚫 danger

Copy link
Contributor

@mattrobenolt mattrobenolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a big change, so just letting you know I'm looking at it and will get back to you. :)

@alex-hofsteede alex-hofsteede changed the title feat: HPKP support feat: HPKP and Expect-CT support Nov 3, 2017
@graingert
Copy link
Contributor

@alex-hofsteede any chance of doing Expect-Staple at the same time?

@alex-hofsteede
Copy link
Contributor Author

Yeah, Expct-Staple is next on the list.

@alex-hofsteede alex-hofsteede force-pushed the alex/hpkp branch 3 times, most recently from fe3d870 to 466eb47 Compare December 7, 2017 02:28
@alex-hofsteede alex-hofsteede changed the title feat: HPKP and Expect-CT support feat: Expect-Staple and Expect-CT support Dec 7, 2017
@alex-hofsteede alex-hofsteede force-pushed the alex/hpkp branch 2 times, most recently from 5f18331 to 8f0e310 Compare December 14, 2017 03:36
@alex-hofsteede alex-hofsteede force-pushed the alex/hpkp branch 2 times, most recently from fe9aaa5 to 0616ab7 Compare December 29, 2017 21:56
Copy link
Contributor

@mattrobenolt mattrobenolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this looks fine. The only missing piece is the UI to display these.

Would really like to address the urls though before approving. But code seems good otherwise..

@alex-hofsteede
Copy link
Contributor Author

@mattrobenolt so this can go out?

In preparation for accepting other types of browser security reports,
start using JSON schema to validate the structure of these documents
to avoid duplicating a bunch of manual validation for the different
report types.
Refactored the CSP endpoint and helpers to allow for support of HPKP
violation reports. Added schema validation and filtering logic to make
it easier to add more interfaces in future. Each can be checked against
its own schema.
Adds an endpoint and schema for Expect-CT reports to be ingested.
Schemas, Interface class, and simple test.
Determine which type of security report is being sent from the report body
Copy link
Contributor

@mattrobenolt mattrobenolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 looks great, let's make sure we coordinate a change for the new endpoint with ops.

cc @JTCunning

We'll need a location block for /api/<project>/security/ that's identical to the current /api/<project>/csp-report/

( this can happen after this gets shipped since it's not really used yet )

@alex-hofsteede alex-hofsteede merged commit 3b75c9b into master Jan 9, 2018
@alex-hofsteede alex-hofsteede deleted the alex/hpkp branch January 9, 2018 21:10
@ghost
Copy link

ghost commented Jan 9, 2018

I'm a bit late to the party (sorry) but it looks like the expect-staple header isn't something that's implemented by UAs and may never ship? See https://twitter.com/estark37/status/949336829349707776 for information (and a note on the preload version of expect-staple)

@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants