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

Base64 signature #429

Merged
merged 7 commits into from
Dec 30, 2019
Merged

Base64 signature #429

merged 7 commits into from
Dec 30, 2019

Conversation

nenaddedic
Copy link
Contributor

This is a fix for #427.

Changes:

  • When verifying an attestation, treat the signature as base64-encoded.
  • When creating an attestation, create it with a base64-encoded signature.

This change breaks backwards compatibility. Attestations verifiable with old code are no longer verifiable with the new code.

Users of GAP will have to recreate the attestations.

Effects on ISP will be limited: attestations previously created by ISP will become invalid, but ISP will create new valid ones for images passing the ISP. The only tricky case is that of a deployment previously passing the ISP (under the old code), but which is now not passing. In this case, new Pods will trigger ISP re-evaluation (because invalid attestations, under the new code), which will fail.

@nenaddedic nenaddedic requested a review from aysylu December 20, 2019 19:37
Copy link
Contributor

@aysylu aysylu left a comment

Choose a reason for hiding this comment

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

A few minor comments about severity of logging and a nit on fn naming.

pkg/kritis/review/validating_transport.go Outdated Show resolved Hide resolved
pkg/kritis/review/validating_transport.go Show resolved Hide resolved
pkg/kritis/review/validating_transport_test.go Outdated Show resolved Hide resolved
@aysylu
Copy link
Contributor

aysylu commented Dec 27, 2019

@nenaddedic: before merging, could you please confirm integration tests pass locally for you? The kokoro results look like a flake, but I'd like to confirm first.

@nenaddedic
Copy link
Contributor Author

@nenaddedic: before merging, could you please confirm integration tests pass locally for you? The kokoro results look like a flake, but I'd like to confirm first.

@aysylu yes, they pass locally

@aysylu aysylu merged commit 6bb7d13 into grafeas:master Dec 30, 2019
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.

2 participants