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

Add signature dst #95

Merged
merged 16 commits into from
May 3, 2022
Merged

Conversation

BasileiosKal
Copy link
Contributor

Closes Issues #78 and #85

Changes:

  1. Adds the number of the revealed messages, the generators, the public key and some ciphersuite specific information to a signature domain separation tag (a signed -always revealed- message).
  2. Switches notation from H0 to H_s, as discussed in the WG call

Note:

This PR leaves the cipher suite specific info as optional for the sig_dst. The reason is that when it comes to the generators, the total number of messages etc., there are real risks for not adding them to the sig_dst.

The last one (total number of messages) is not documented in an issue to my knowledge, but I think it is well understood that it is dangerous to not include it. For example, the prover could reveal an additional “hidden” attribute by calculating C1_new = C1 + H_j(U+1) * m~_j(U+1) and m^_j(U+1) = m~_j(U+1) and have the proof be validated (reproduction).

(Including the generators also solves this issue, so we could potentially only include the generators and not the total number of messages. That said, IMO it would be better to be more careful since it does not hurt performance or complexity)

In the case of the ciphersuite info though, IMO its good security practice to include it but not necessary?? I’m more than happy to update the pr and make the ciphersuite info mandatory, but to me it seems that the separation between ciphersuites could also happen elsewhere more naturally (in hash-2-curve for example, similarly to what the bls draft does)

draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
Copy link
Contributor

@mikelodder7 mikelodder7 left a comment

Choose a reason for hiding this comment

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

I’ll need to work this in detail to understand why you can manipulate it to add an additional hidden message and still have it verify. I don’t think this is necessary until I convince myself. This seems superfluous. Being able to just arbitrarily add like this seems like the scheme itself is broken

draft-bbs-signatures.md Outdated Show resolved Hide resolved
@BasileiosKal
Copy link
Contributor Author

I’ll need to work this in detail to understand why you can manipulate it to add an additional hidden message and still have it verify. I don’t think this is necessary until I convince myself. This seems superfluous. Being able to just arbitrarily add like this seems like the scheme itself is broken

I'm more than happy to discuss this further. Personally, i don't think that this breaks the scheme. The original proofs of security implicitly require the verifier to trust not only the generators but their number as well (i.e., the specific generators used for this specific signature), which is why IMO they are either in the signers PK or supplied by a trusted third-party CRS authority etc. Creating an arbitrary number of generators from a seed broke the trust to their number and this proposal re-enforces it.

Of-course this is just my intuition. Really keen to discuss.

@BasileiosKal
Copy link
Contributor Author

A couple more updates,

  1. sig_dst is turned into a scalar using OS2IP and moding after. This leaves open the question of the length of the HASH output, but since this will be parameterized in the ciphersuites, I don’t define it further here

  2. If sig_dst is 0 the sign will abort. We could use an XOF and keep drawing bytes instead of aborting but then verifier and holder should do the same procedure. I think that getting a 0 is so unlikely that just aborting would do fine while not raising the complexity of the draft. That said introducing a “hash_to_scalar” function (as I plan to), could also solve this issue without needing to abort.

@andrewwhitehead
Copy link
Contributor

If sig_dst is 0 the sign will abort. We could use an XOF and keep drawing bytes instead of aborting but then verifier and holder should do the same procedure. I think that getting a 0 is so unlikely that just aborting would do fine while not raising the complexity of the draft. That said introducing a “hash_to_scalar” function (as I plan to), could also solve this issue without needing to abort.

Whether we abort or keep reading from the XOF, it would be nice to use the same method for both this calculation and the presentation challenge. I don't think that either case risks exposing sensitive information through a timing attack (maybe to some third party monitoring the process, but not to the verifier who will learn the hash inputs anyway?), so rather than aborting the program with a minuscule probability or adding special handing for the error case it seems like simply looping to the next hash output would be the best solution.

@mikelodder7
Copy link
Contributor

mikelodder7 commented Apr 12, 2022

I don't think this is necessary. If you use the same info in the Fiat Shamir heuristic then this avoids the attack mentioned.

challenge = HASH(PK || H_s || H_1 || ... || H_L || L || rvl_msgs with indices || C1 || C2 || T1 || T2 || nonce)

@andrewwhitehead
Copy link
Contributor

Adding the generators to the challenge hash is sufficient, but adding them to the DST is slightly more efficient for the prover, as they can cache the DST scalar and avoid hashing the generators with every presentation. The generators are still included in the Fiat-Shamir hash by virtue of the revealed DST message being hashed. If we agree that a DST message is necessary then I think that is the best place for them.

@tplooker
Copy link
Member

but adding them to the DST is slightly more efficient for the prover, as they can cache the DST scalar and avoid hashing the generators with every presentation.

+1 that is my perspective also

@mikelodder7
Copy link
Contributor

I disagree. We're adding more work for both sides. How is that saving anything?!? I think we should arrange a time to discuss it. Github isn't the best place for this.

@tplooker
Copy link
Member

tplooker commented Apr 12, 2022

I disagree. We're adding more work for both sides. How is that saving anything?!? I think we should arrange a time to discuss it. Github isn't the best place for this.

Just adding my perspective here as I wont be able to attend the special call on this topic. The proposal is to include these public parameters (generators, total number of signed messages etc) in the signature DST instead of in the challenge computed for the proof. This is slightly more efficient as @andrewwhitehead said because the signature DST value could be cached across multiple proofs where the challenge value cannot. Also these public parameters are set by the issuer so the safest way to prevent these from being manipulated by the holder or any other party, is to just protect them in the original signature produced by the issuer via the signature DST.

draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
Copy link
Member

@tplooker tplooker left a comment

Choose a reason for hiding this comment

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

A couple more minor tweaks, getting there!

BasileiosKal and others added 4 commits May 1, 2022 14:24
Co-authored-by: Tobias Looker <tplooker@gmail.com>
Co-authored-by: Tobias Looker <tplooker@gmail.com>
draft-bbs-signatures.md Outdated Show resolved Hide resolved
draft-bbs-signatures.md Outdated Show resolved Hide resolved
@tplooker
Copy link
Member

tplooker commented May 3, 2022

@BasileiosKal can you please resolve conflicts?

@tplooker
Copy link
Member

tplooker commented May 3, 2022

Open three weeks, several reviews and discussion on WG calls, multiple approvals now, merging

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.

5 participants