Skip to content

Add docker/utils.ManifestMatchesDigest#75

Merged
runcom merged 1 commit intocontainers:masterfrom
mtrmac:matches-manifest-digest
Jun 2, 2016
Merged

Add docker/utils.ManifestMatchesDigest#75
runcom merged 1 commit intocontainers:masterfrom
mtrmac:matches-manifest-digest

Conversation

@mtrmac
Copy link
Contributor

@mtrmac mtrmac commented May 24, 2016

As opposed to callers just calling utils.ManifestDigest(image.Manifest()), this is a forward-compatible interface, allowing other digest algorithms to be added in the future.

Right now, we only support SHA-256, so the underlying implementation does not change anything.

@mtrmac
Copy link
Contributor Author

mtrmac commented May 24, 2016

(Right now I need this for an types.Image, and with the various MIME types giving the full information to a types.Image seems reasonable. OTOH eventually the core should probably go into docker/utils, and it should also be used by VerifyDockerManifestSignature.

Feel free to defer merging this after I get to cleaning up VerifyDockerManifestSignature.)

runcom
runcom reviewed May 25, 2016
View reviewed changes
docker/docker_image.go Outdated
return false, err
}
// Note that this is not doing ConstantTimeCompare; by the time we get here, the cryptographic signature must already have been verified,
// or we are not using a cryptographic channel and the attacker can modify the digest along with the manifest blob.
Copy link
Member

@runcom runcom May 25, 2016

Choose a reason for hiding this comment

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

can you put this comment in the func comment above? so that godoc is aware of this and users also

Copy link
Contributor Author

@mtrmac mtrmac May 25, 2016

Choose a reason for hiding this comment

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

Good point, will update.

@runcom
Copy link
Member

runcom commented May 25, 2016

Feel free to defer merging this after I get to cleaning up VerifyDockerManifestSignature.)

I'd rather wait for this than merging :)

@mtrmac mtrmac force-pushed the matches-manifest-digest branch from 83bc6d2 to 846cc1c Compare May 25, 2016 14:31
@mtrmac
Copy link
Contributor Author

mtrmac commented May 25, 2016 •
edited
Loading

Feel free to defer merging this after I get to cleaning up VerifyDockerManifestSignature.)

I'd rather wait for this than merging :)

Sure, marking as WIP for now.

@mtrmac mtrmac changed the title Add types.Image.MatchesManifestDigest WIP: Add types.Image.MatchesManifestDigest May 25, 2016
@mtrmac mtrmac force-pushed the matches-manifest-digest branch from 846cc1c to d07f84f Compare May 25, 2016 18:54
@mtrmac mtrmac mentioned this pull request May 28, 2016
Merged
@mtrmac mtrmac force-pushed the matches-manifest-digest branch from d07f84f to 16a6937 Compare May 28, 2016 00:24
@mtrmac
Copy link
Contributor Author

mtrmac commented May 28, 2016 •
edited
Loading

(Currently depends on, and includes, #79. Conceptually it is independent, the dependency is for merging with changes of that PR. I will rebase as needed.)

@mtrmac mtrmac force-pushed the matches-manifest-digest branch 2 times, most recently from cc6aa37 to e917465 Compare May 31, 2016 16:18
@mtrmac mtrmac mentioned this pull request May 31, 2016
Merged
@mtrmac
Add docker.utils.ManifestMatchesDigest
938478e 
As opposed to callers just calling utils.ManifestDigest(), this is
a forward-compatible interface, allowing other digest algorithms to
be added in the future.

Right now, we only support SHA-256, so the underlying implementation
does not change anything.
@mtrmac mtrmac force-pushed the matches-manifest-digest branch from e917465 to 938478e Compare June 1, 2016 14:39
@mtrmac
Copy link
Contributor Author

mtrmac commented Jun 1, 2016

Moved the functionality from types.Image directly to docker/utils; after that, having a helper in types.Image did not seem all that useful.

Now with a test, and ready to be merged.

@mtrmac mtrmac changed the title WIP: Add types.Image.MatchesManifestDigest Add docker/utils.ManifestMatchesDigest Jun 1, 2016
@runcom runcom merged commit ee7c5eb into containers:master Jun 2, 2016
@mtrmac mtrmac deleted the matches-manifest-digest branch June 2, 2016 14:10
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

locked - please file new issue/PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mtrmac @runcom