Skip to content
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

Add cure53 security audit report #1065

Merged
merged 8 commits into from
Sep 10, 2018
Merged

Add cure53 security audit report #1065

merged 8 commits into from
Sep 10, 2018

Conversation

RichiH
Copy link
Member

@RichiH RichiH commented Jun 15, 2018

No description provided.

Signed-off-by: Richard Hartmann <richih@richih.org>
Signed-off-by: Richard Hartmann <richih@richih.org>
@RichiH RichiH changed the title Add cre53 security audit report Add cure53 security audit report Jun 15, 2018

## External audits

CNCF sponsored an external security audit by cure53 which ran from April 2018
Copy link
Member

Choose a reason for hiding this comment

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

I would link both the CNCF and cure53.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought they would be obvious once you are this deep in our pages, but will do.


CNCF sponsored an external security audit by cure53 which ran from April 2018
to June 2018. You can find the final report of the audit
[here](assets/downloads/2018-06-11--cure53_security_audit.pdf).
Copy link
Member

Choose a reason for hiding this comment

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

It's nice to provide somewhat larger links than the word "here".

How about You can find more details in the [final report of the audit](assets/downloads/2018-06-11--cure53_security_audit.pdf).

Signed-off-by: Richard Hartmann <richih@richih.org>
Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

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

I've like to have #1063 in before this.

@@ -106,7 +106,7 @@ If using a client-library-provided HTTP handler, it should not be possible for
malicious requests that reach that handler to cause issues beyond those
resulting from additional load and failed scrapes.

## Authentication/Authorisation/Encryption
## Authentication, Authorisation, and Encryption
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, aren't we meant to be using US spellings? I must have let this one slip though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't care which as long as we are consistent; changed.


## External audits

[CNCF](https://cncf.io) sponsored an external security audit by
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we summarise this here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think yes. Attribution is important in both FLOSS & security and I would like to acknowledge the fact that CNCF spent money on us.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant more, that we should add a few sentences summarising the report.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Signed-off-by: Richard Hartmann <richih@richih.org>
Signed-off-by: Richard Hartmann <richih@richih.org>
Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

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

👍, just fix that one small error.

[cure53](https://cure53.de) which ran from April 2018 to June 2018.

The audit found no major concerns outside of the scope of this document. It
re-iterated on the importance of following what is layed out here.
Copy link
Contributor

Choose a reason for hiding this comment

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

laid out

[CNCF](https://cncf.io) sponsored an external security audit by
[cure53](https://cure53.de) which ran from April 2018 to June 2018.

The audit found no major concerns outside of the scope of this document. It
Copy link
Member

Choose a reason for hiding this comment

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

This is really an unduly generous interpretation of what the report says and omits that the report strongly suggests to change our practices. Also I don't think that this document here would even give people an idea that e.g. the CORS and XSRF vulnerabilities exist and need to be protected against. Those are pretty severe IMO. If an attacker can make me visit a website and then they can either read all my Prometheus data or shut down my Prometheus server, that seems bad, and almost nobody reading this document would anticipate those problems from what is written here.

Signed-off-by: Richard Hartmann <richih@richih.org>
[cure53](https://cure53.de) which ran from April 2018 to June 2018.

The audit found concerns regarding Prometheus' secrity model, but following
what is layed out in this document should guard against most of them.
Copy link
Contributor

Choose a reason for hiding this comment

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

laid


The audit found concerns regarding Prometheus' secrity model, but following
what is layed out in this document should guard against most of them.
In particular, the CORS (PRM-01-001) and CSRF (PRM-01-003) attack vectors might
Copy link
Contributor

Choose a reason for hiding this comment

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

They're in this doc now, and many attack vectors are non-obvious. Putting it like this make it seem as though reading this doc is not sufficient.

[CNCF](https://cncf.io) sponsored an external security audit by
[cure53](https://cure53.de) which ran from April 2018 to June 2018.

The audit found concerns regarding Prometheus' secrity model, but following
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this really captures it, and this document covers everything mentioned.

be non-obvious.

For more details, please read the
[final report of the audit](assets/downloads/2018-06-11--cure53_security_audit.pdf).
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the assets from this URL

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you certain? grep -ri assets content implies otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I see now that there's a URL re-write rule in place for static assets. I do think, though, that the URL needs to begin with /assets rather than assets since this is not the index page.

@RichiH
Copy link
Member Author

RichiH commented Aug 18, 2018

@lucperkins Suggestions as to verbiage around the contended parts?

@lucperkins
Copy link
Contributor

@RichiH @brian-brazil Would it be possible to ship this without the summary paragraph for now? We should get the results out to the public. I'd be happy to write up a more detailed summary once the PDF is out.

Also, Richie, make sure to update the URL of the PDF to /assets/downloads/2018-06-11--cure53_security_audit.pdf.

@brian-brazil
Copy link
Contributor

That sounds like a good idea.

@RichiH
Copy link
Member Author

RichiH commented Sep 7, 2018 via email

Signed-off-by: lucperkins <lucperkins@gmail.com>
Signed-off-by: lucperkins <lucperkins@gmail.com>
@lucperkins lucperkins merged commit 04ebe44 into master Sep 10, 2018
aylei pushed a commit to aylei/docs that referenced this pull request Oct 28, 2019
@caniszczyk caniszczyk deleted the richih/security_audit branch March 24, 2020 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants