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 security audit report and blogpost #5775

Merged
merged 8 commits into from
Dec 5, 2023

Conversation

AdamKorcz
Copy link
Contributor

Adds the Knative security audit report and blogpost.

cc @davidhadas @evankanderson

Signed-off-by: Adam Korczynski <adam@adalogics.com>
@knative-prow knative-prow bot requested review from skonto and snneji November 27, 2023 20:51
Copy link

netlify bot commented Nov 27, 2023

Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit b93cf11
🔍 Latest deploy log https://app.netlify.com/sites/knative/deploys/656e5ca09f9afa00087cae14
😎 Deploy Preview https://deploy-preview-5775--knative.netlify.app/blog/events/security-audit-2023
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@knative-prow knative-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 27, 2023
@ReToCode
Copy link
Member

PTAL:

/assign @evankanderson
/assign @davidhadas

Copy link
Member

@dprotaso dprotaso left a comment

Choose a reason for hiding this comment

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

There are typos in the PDF.

eg. under thread modelling you have 'Knative Serving' but then the paragraphs under neath it talk about Knative Eventing

Then under the Knative Serving section there are phrases like Knative Eventing will autoscale - but it should be 'Knative Serving'

@AdamKorcz
Copy link
Contributor Author

There are typos in the PDF.

eg. under thread modelling you have 'Knative Serving' but then the paragraphs > under neath it talk about Knative Eventing

Then under the Knative Serving section there are phrases like Knative Eventing will > autoscale - but it should be 'Knative Serving'

Thanks for catching that. Should now be fixed with the latest report.

Signed-off-by: Adam Korczynski <adam@adalogics.com>
@davidhadas
Copy link
Contributor

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2023

One CVE was assigned during the audit for a vulnerability that could allow an attacker with already escalated privileges to cause further damage in the cluster. The attacker needs to first establish a position in a Knative pod, and from there, they could exploit the vulnerability and cause denial of service of the Knative autoscaling, thereby denying any autoscaling of Knative. The issue was assigned CVE-2023-48713 of Moderate severity and has been fixed in v1.10.5, v1.12.0 and v1.11.3.

The auditors found that Knative does not include provenance with releases; Provenance is a critical component of complying with [SLSA](https://slsa.dev/) and ensuring tamper resistance of release artifacts. Recently, the SLSA community released v1.9.0 of the [slsa-github-generator](https://github.com/slsa-framework/slsa-github-generator) which produces SLSA L3 compliant provenance. slsa-github-generator ensures tamper-resistance of artifacts by producing verifiable provenance, thereby mitigating a series of known supply-chain risks, many of which have been exploited in the wild in recent years. Ada Logics recommend that Knative switches its build to the slsa-github-generator to comply with SLSA L3.
Copy link
Member

Choose a reason for hiding this comment

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

Feel free to include this, but Knative is using Prow to build and test releases (our usage requires more resources than are available on GitHub Actions, including real Kubernetes clusters). As such, the GitHub Actions SLSA generator is probably not a good fit.

We're also using ko to build these releases, which doesn't currently have signing support (see ko-build/ko#357 and ko-build/ko#603).

Copy link
Member

Choose a reason for hiding this comment

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

Actually, we have a provenance reporter here:

https://github.com/knative/hack/blob/7030d5bf584de17a82ec8742c1078ca50633d45e/release.sh#L415

Which is built into the image here: https://github.com/knative/infra/blob/b90750d6063cfcca6322d72d947f1670a83a9349/images/prow-tests/Dockerfile#L220

From the source code here: https://github.com/knative/toolbox/tree/main/provenance-generator

I'm currently trying to figure out the correct cosign download attestation invocation, but I'm pretty sure we have provenance implemented, even though it's not through GHA.

Copy link
Member

Choose a reason for hiding this comment

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

Huh, it turns out that Knative Serving does not have attestations, but most of the other releases do. For example:

cosign download attestation $(curl -L https://github.com/knative/eventing/releases/download/knative-v1.12.1/eventing-core.yaml | grep image: | head -1 | cut -d: -f2-)

(Grab the first image from the Eventing release and run cosign download attestation on it...)

Copy link
Member

Choose a reason for hiding this comment

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

(And append | jq -r '.payload' | base64 --decode | jq to read the actual in-toto attestation, rather than the base64-encoded envelope)

Copy link
Member

Choose a reason for hiding this comment

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

knative/infra#287 for the attestations for serving.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The auditors found that Knative does not include provenance with releases; Provenance is a critical component of complying with [SLSA](https://slsa.dev/) and ensuring tamper resistance of release artifacts. Recently, the SLSA community released v1.9.0 of the [slsa-github-generator](https://github.com/slsa-framework/slsa-github-generator) which produces SLSA L3 compliant provenance. slsa-github-generator ensures tamper-resistance of artifacts by producing verifiable provenance, thereby mitigating a series of known supply-chain risks, many of which have been exploited in the wild in recent years. Ada Logics recommend that Knative switches its build to the slsa-github-generator to comply with SLSA L3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to change that paragraph to mention Knatives good work on generating SLSA-compliant provenance than removing it. I have changed the paragraph in b93cf11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have also changed the executive summary and SLSA sections of the report to reflect this.

@evankanderson
Copy link
Member

(Other than the comments on our builds not being done via GitHub Actions, this LGTM.)

@evankanderson
Copy link
Member

(If you're willing to accept the two strikeouts on provenance information, I'm willing to publish this.)

Co-authored-by: Evan Anderson <evan.k.anderson@gmail.com>
@knative-prow knative-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 4, 2023
@AdamKorcz
Copy link
Contributor Author

@evankanderson PTAL

Copy link
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2023
Copy link

knative-prow bot commented Dec 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AdamKorcz, evankanderson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 5, 2023
@knative-prow knative-prow bot merged commit a8ee4e9 into knative:main Dec 5, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants