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

Hashed claim values #47

Merged
merged 6 commits into from Aug 28, 2019
Merged

Hashed claim values #47

merged 6 commits into from Aug 28, 2019

Conversation

David-Chadwick
Copy link
Contributor

@David-Chadwick David-Chadwick commented Aug 22, 2019

fixes #42


Preview | Diff

hashed claim values
index.html Outdated
@@ -1642,6 +1642,18 @@ <h3>Selective Disclosure</h3>
all of the credential's claims to be revealed.

</li>
<li>
<b> Hashed Values</b> - With this method, the <a>issuer</a> issue a single
Copy link
Member

Choose a reason for hiding this comment

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

<b> Hashed Values</b> - With this method, the <a>issuer</a> issue a single
-> <b>Hashed Values</b> &mdash; With this method, the <a>issuer</a> issues a single

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes fine

Copy link
Member

Choose a reason for hiding this comment

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

<b> Hashed
-> <b>Hashed
was overlooked

index.html Outdated
@@ -1642,6 +1642,18 @@ <h3>Selective Disclosure</h3>
all of the credential's claims to be revealed.

</li>
<li>
<b> Hashed Values</b> - With this method, the <a>issuer</a> issue a single
<a>verifiable credential</a> containing all the subject's <a>claims</a>.
Copy link
Member

Choose a reason for hiding this comment

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

all the subject's <a>claims</a>.
-> all the <a>issuer</a>'s <a>claims</a> about the <a>subject</a>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes fine

<code>credentialSubject</code> contains the identifier of the hashing
algorithm used to create all the hashed values. The holder includes the actual
values of the <a>claims</a> that are to be revealed to the <a>verifier</a> in
the <a>verifiable presentation</a>.
Copy link
Member

Choose a reason for hiding this comment

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

More rewording is needed. Perhaps replacing your "actual value" with "original
literal value" would work... as --

However, each <a>claim</a> value in the <a>verifiable credential</a> is created
by hashing the original literal value with a different nonce. The value for the
<a>claim</a> in the <a>verifiable credential</a> then comprises an object
containing 1) the nonce and 2) the hashed value of the nonce concatenated with
the original literal <a>claim</a> value. In addition, the
<code>credentialSubject</code> contains the identifier of the hashing algorithm
used to create all the hashed values. The holder includes the original literal
values of the <a>claims</a> they intend to reveal to the <a>verifier</a> in their
<a>verifiable presentation</a>.

I am a bit confused by the <code>credentialSubject</code> contains the identifier of the hashing algorithm. Does the <code>credentialSubject</code> here not contain the identifier of the subject of the credential? Or does it include both identifiers (making it an array, rather than a single value)? Or something else?

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 think that 'actual value' is better than 'original literal value'

The credentialSubject comprises a whole set of attributes, including the id. This is a proposal to add another property to this set i.e. the hashing algorithm. I will clarify this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Having a hashing algorithm as a property of a credentialSubject makes very little sense. It would mean, for example, that a VC about David Chadwick would state that David has a hashing algorithm of SHA-256.

It sounds like this information should be captured in the proof section somewhere instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The hashing algorithm is actually a property of the claim value e.g. the claim {name=David} is transposed into {name={hashID=SHA-256, nonce=123....9, value=a8clyga}}. But since the same hashing algorithm is used for all claim values, there is no point in repeating the hashing algorithm ID multiple times. So we move it up a level to the encapsulating object, which is the credentialSubject, and put it there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Has this been resolved? I lean towards what @dlongley said above. The hashing algorithm, and all cryptographic attributes should be decoupled from the contents of the credentialSubject.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we had a data model in which a credentialSubject contained an ID and a set of claims, and each claim contained an ID and a set of properties, then we could have explicitly placed the hashID in the claim object. But unfortunately the data model has not explicitly identified the claim object with a claim ID and so the fact that this exists is obscured (and has led to many previous misunderstandings and issues.) Thus you think (wrongly) that the hashID is a property of the credentialSubject, when in fact, it is a property of the (invisible) claim object.

Copy link
Contributor

Choose a reason for hiding this comment

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

@David-Chadwick, btw, (we don't need to mention this), but there has been other exploratory work in this area that involves reducing nonce storage via HMAC keys and that transforms the data to RDF quads prior to hashing -- and other work that uses URLs to store the hash values.

I think given that there are a number of ways to do the above scheme that we should perhaps just leave a note saying that it is up to the proof scheme to figure out how it's done -- like we do with ZKPs. We won't need to debate this further if we take that approach. It's worth just noting the nonce+hash method here as a means of redaction. We can also point to the CCG for further work in the area.

@TallTed
Copy link
Member

TallTed commented Aug 22, 2019

fixes #42

(@David-Chadwick -- note that the line above, including the space before the #, associates this PR with that Issue, and if you include it in your initial post (and possibly in one of your comments), merging this PR will then auto-close that issue.)

@TallTed TallTed mentioned this pull request Aug 22, 2019
@David-Chadwick
Copy link
Contributor Author

fixes #42

@TallTed
Copy link
Member

TallTed commented Aug 22, 2019

@David-Chadwick - I was wrong about putting it in a comment. If you edit your initial post/comment on this PR, and add fixes #42, you should then (after saving your edit) see that fixes is highlighted, and if you mouse over it, you'll see a tooltip from GitHub -- like what you can see here.

@deiu
Copy link
Contributor

deiu commented Aug 27, 2019

The issue autofix text can also be added during merge, don't worry too much about it.

Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

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

Left some spelling, etc. nits.

index.html Outdated
@@ -1642,6 +1642,17 @@ <h3>Selective Disclosure</h3>
all of the credential's claims to be revealed.

</li>
<li>
<b> Hashed Values</b> &mdash; With this method, the <a>issuer</a> issues a single
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<b> Hashed Values</b> &mdash; With this method, the <a>issuer</a> issues a single
<b>Hashed Values</b> - With this method, the <a>issuer</a> issues a single

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dlongley My original text had it your way, but @TallTed said I should change it to the current way. So can I suggest that the two of you agree between yourselves which way you want it to be. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After reviewing the existing text I see that the two previous bullets already use -, so I have changed mine back to this (how it was originally).

index.html Outdated
about the subject.
However, each <a>claim</a> value is created by hashing the actual value with
a different nonce so that the <a>verifier</a> cannot determine the actual value.
There are several different ways of modelling this, and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
There are several different ways of modelling this, and
There are several different ways of modeling this, and

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

index.html Outdated
hashed values. The holder includes the actual
a different nonce so that the <a>verifier</a> cannot determine the actual value.
There are several different ways of modelling this, and
no standard way is currently defined. The holder includes the actual
Copy link
Member

Choose a reason for hiding this comment

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

holder
->
<a>holder</a>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@deiu deiu merged commit bf183a5 into gh-pages Aug 28, 2019
@deiu deiu deleted the Selective-Disclosure branch August 28, 2019 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selective Disclosure
4 participants