-
Notifications
You must be signed in to change notification settings - Fork 676
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
descriptor: sha{256,512}: disallow /[A-F]/ characters #667
Conversation
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
@@ -137,11 +140,17 @@ If a useful algorithm is not included in the above table, it SHOULD be submitted | |||
[SHA-256][rfc4634-s4.1] is a collision-resistant hash function, chosen for ubiquity, reasonable size and secure characteristics. | |||
Implementations MUST implement SHA-256 digest verification for use in descriptors. | |||
|
|||
When the _algorithm identifier_ is `sha256`, the _encoded_ portion MUST match `/[a-f0-9]{64}/`. | |||
Note that `[A-F]` MUST NOT be used here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather stay DRY by defining a hex encoding with these restrictions, and then saying that sha256
and sha512
MUST be hex-encoded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think hex-encoding is specific to algorithm.
Depending on the algorithm, hex-encoded digest can be somealgorithm+uuid:f81d4fae-7dec-11d0-a765-00a0c91e6bf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think hex-encoding is specific to algorithm.
This is true, but both of the current algorithms use the same hex encoding. Defining that in one place doesn't block other algorithms from defining or using alternative hex encodings, but it does make it easier for other algorithms that want the current hex encoding.
Should probably make the regex [:upper:] then; i imagine base64 will make
an appearance eventually.
…On Wed, May 10, 2017 at 9:37 PM Akihiro Suda ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In descriptor.md
<#667 (comment)>
:
> @@ -137,11 +140,17 @@ If a useful algorithm is not included in the above table, it SHOULD be submitted
[SHA-256][rfc4634-s4.1] is a collision-resistant hash function, chosen for ubiquity, reasonable size and secure characteristics.
Implementations MUST implement SHA-256 digest verification for use in descriptors.
+When the _algorithm identifier_ is `sha256`, the _encoded_ portion MUST match `/[a-f0-9]{64}/`.
+Note that `[A-F]` MUST NOT be used here.
I think hex-encoding is specific to algorithm.
Depending on the algorithm, hex-encoded digest can be
somealgorithm+uuid:f81d4fae-7dec-11d0-a765-00a0c91e6bf6
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#667 (comment)>,
or mute the thread
</~https://github.com/notifications/unsubscribe-auth/AABJ6yL43ceEiR5nfhpDRLHU1jnXm-lfks5r4mZwgaJpZM4NWKVg>
.
|
base64 should use different algorithm identifier, so I think that is the out scope of this PR, which only focuses on |
@AkihiroSuda This looks good. Could you also make a PR to |
Disallowing upper characters will be useful for avoiding implementation bugs
Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp