descriptor: Define the 'sha256' algo identifier

[wking]

Before this commit, there wasn't something obvious to point to if you wanted to explain the sha256 identifier.

The “SHOULD be submitted” wording follows runtime-spec's example.

[stevvooe]

The spec has left the algorithm undefined, purposefully. Defining disallows implementations from choosing other hashes.

Is there anywhere else where we make this distinction?

[wking]

On Tue, Feb 28, 2017 at 04:19:23PM -0800, Stephen Day wrote: The spec has left the algorithm undefined, purposefully. Defining disallows implementations from choosing other hashes.

The intent here is not to limit algorithm choices (can you point to the wording that gives you that impression?). The idea is that when I use the ‘sha256’ algorithm identifier, folks are clear that I mean the SHA-256 algorithm as specified in [1]. I don't want folks hashing with SHA-224 or some such instead and then getting confused when the hashes don't match. Before this commit, there is nothing besides examples to make the connection between the ‘sha256’ identifier and the SHA-256 algorithm as specified in [1]. This commit makes the ‘sha256’ algorithm identifier reliable. A second benefit of an algorithm-identifier registry is that digest authors are more likely to reuse existing identifiers. Without a registry (or a bunch of examples acting as a quasi-registry), Alice might create digest with ‘sha-256’ as the algorithm identifier for SHA-256. With this commit she's still allowed to do that, but she's more likely to discover the ‘sha256’ identifier and use that instead of inventing her own identifier. That increases the odds that digests will use reliable algorithm identifiers. And again, there is only a SHOULD-level request for algorithm identifier registration. If you want to use ‘my-secret-algorithm’ and not register it, go right ahead. Just don't be surprised when Bob's digester chokes with “unrecognized algorithm: my-secret-algorithm”. [1]: https://tools.ietf.org/html/rfc4634#section-4.1

[jonboulle]

LGTM
I also don't see how it disallows/limits anything.

[stevvooe]

I also don't see how it disallows/limits anything.

Saying one thing exists creates the presumption that unmentioned things don't exist.

The format is already defined and documented here. While I wasn't quite accurate above, we already call out a whole section on sha256. This PR just adds more unnecessary indirection. If this actually added value or context, I would be in favor. As it stands, it just plops another table in a confusing spot.

I think the same effect can be had by simply adding a sentence after the following:

While the _algorithm_ component of the digest does allow one to utilize a wide variety of algorithms, compliant implementations SHOULD use [SHA-256](#sha-256).
Digests using this algorithm are identified by a prefix of `sha256`.

If you think a table with one item is great, then fine but this seems better.

[wking]

On Wed, Mar 01, 2017 at 11:23:48AM -0800, Stephen Day wrote: > I also don't see how it disallows/limits anything. Saying one thing exists creates the presumption that unmentioned things don't exist.

I feel like the “If a useful algorithm is not included in the above table…” provides a sufficient hint that the table is not exhaustive. If you feel like it needs additional/alternative wording to make that point clear, what sort of wording would you like to see?

The format is already defined and documented here.

We already define the ‘algorithm ":" hex’ format [1], but that does not define any values for the algorithm component. Defining at least the ‘sha256’ value is what I'm trying to add here.

While I wasn't quite accurate above, we already call out a whole section on `sha256`.

Not quite. We have a section on SHA-256 the algorithm [2], but there is no mention in that section on ‘sha256’ the identifier.

This PR just adds more unnecessary indirection. If this actually added value or context, I would be in favor. As it stands, it just plops another table in a confusing spot.

Maybe. If ‘sha256’ is the only algorithm identifier we're ever going to document, I agree that the table is overkill and your proposed wording seems fine. However, go-digest already defines two additional algorithm identifiers [3]. I feel like at least sha512 should be registered as well, although that seemed out of scope for this PR. Using a table here sets the stage for subsequent registrations like that. But again, if there is no intention of registering other algorithm identifiers, let me know and I'll update this PR. [1]: /~https://github.com/opencontainers/image-spec/blame/v1.0.0-rc5/descriptor.md#L72 [2]: /~https://github.com/opencontainers/image-spec/blob/v1.0.0-rc5/descriptor.md#sha-256 [3]: /~https://github.com/opencontainers/go-digest/blob/v1.0.0-rc0/algorithm.go#L32-L33

[stevvooe]

LGTM

@wking Fair enough. I think we can maintain this as a restricted set, but we need to be careful about adding new algorithms. While there are hooks for making it happen, adding support for a new algorithm should not be casual. What sha256 is broken, it will be a once in a decade event.