Add support for BLS keys and BBS+ signatures #288

decentralgabe · 2023-02-17T22:41:04Z

Fixes #27

cryptosuite/bbsplussignatureproofsuite.go

codecov-commenter · 2023-02-18T00:22:13Z

Codecov Report

Merging #288 (625fe9c) into main (c9cdf21) will decrease coverage by 1.37%.
The diff coverage is 48.84%.

@@            Coverage Diff             @@
##             main     #288      +/-   ##
==========================================
- Coverage   58.68%   57.31%   -1.37%     
==========================================
  Files          42       46       +4     
  Lines        4717     5467     +750     
==========================================
+ Hits         2768     3133     +365     
- Misses       1462     1759     +297     
- Partials      487      575      +88

Impacted Files	Coverage Δ
crypto/jwt.go	`46.15% <0.00%> (ø)`
cryptosuite/jsonwebkey2020.go	`47.48% <0.00%> (+1.33%)`	⬆️
did/util.go	`52.69% <0.00%> (ø)`
crypto/bbs.go	`24.14% <24.14%> (ø)`
cryptosuite/bls12381g2key2020.go	`49.32% <49.32%> (ø)`
cryptosuite/cryptosuite.go	`58.62% <50.00%> (-3.88%)`	⬇️
cryptosuite/jwssignaturesuite.go	`52.24% <50.00%> (-1.90%)`	⬇️
cryptosuite/bbsplussignaturesuite.go	`52.83% <52.83%> (ø)`
cryptosuite/bbsplussignatureproofsuite.go	`55.17% <55.17%> (ø)`
did/key.go	`72.87% <100.00%> (ø)`
... and 2 more

Co-authored-by: Andres Uribe <auribe@tbd.email>

* origin/bbs-plus: Update cryptosuite/bbsplussignaturesuite.go # Conflicts: # cryptosuite/bbsplussignaturesuite.go

andresuribe87

PTAL - sorry for long review cycle.

andresuribe87 · 2023-03-20T17:30:32Z

crypto/bbs.go

+// helpers
+
+func prepareBBSMessage(msg []byte) [][]byte {
+	rows := strings.Split(string(msg), "\n")


Thanks - weird... the link seems to be broken for me. Something must be up :/

cryptosuite/bbsplussignaturesuite.go

andresuribe87 · 2023-03-20T17:43:48Z

cryptosuite/bbsplussignaturesuite.go

+	b.ProofValue = proofValue
+}
+
+func (b *BBSPlusSignature2020Proof) ToGenericProof() (crypto.Proof, error) {


Missed this - was the documentation added elsewhere?

cryptosuite/bls12381g2key2020.go

andresuribe87 · 2023-03-20T17:51:30Z

cryptosuite/bls12381g2key2020.go

+}
+
+// GenerateBLSKey2020 https://w3c-ccg.github.io/ldp-bbs2020/#bls-12-381-g2-public-key
+func GenerateBLSKey2020() (*BLSKey2020, error) {


If this is creating a G2, should it be named appropriately?

kept the name and added a param, also updated broken link

cryptosuite/bbsplussignaturesuite.go

andresuribe87 · 2023-03-20T18:59:27Z

cryptosuite/bbsplussignatureproofsuite.go

+}
+
+// CreateDeriveProof https://w3c-ccg.github.io/ldp-bbs2020/#create-derive-proof-data-algorithm
+func (b BBSPlusSignatureProofSuite) CreateDeriveProof(inputProofDocument Provable, revealDocument map[string]interface{}) (*DeriveProofResult, error) {


Does the inputProofDocument need to be of type Provable? Should it be any instead?

updated to any

cryptosuite/bbsplussignatureproofsuite.go

andresuribe87 · 2023-03-20T19:22:03Z

cryptosuite/bbsplussignatureproofsuite_test.go

+		// orig
+		p, _ := util.PrettyJSON(testCred)
+		println(string(p))
+
+		// frame
+		p, _ = util.PrettyJSON(revealDoc)
+		println(string(p))
+
+		// // reveal
+		p, _ = util.PrettyJSON(selectiveDisclosure)
+		println(string(p))


Are the println statements intentional? I'm guessing you wanted to test that the reveal does not contain credentialSubject. If so, can you encode that?

just for the demo I gave, removing

cryptosuite/bbsplussignatureproofsuite.go

Co-authored-by: Andres Uribe <auribe@tbd.email>

* origin/main: Bump github.com/multiformats/go-multibase from 0.1.1 to 0.2.0 (#313) Bump github.com/go-playground/validator/v10 from 10.11.2 to 10.12.0 (#311) Bump github.com/goccy/go-json from 0.10.0 to 0.10.2 (#310) Bump golang.org/x/term from 0.5.0 to 0.6.0 (#299) Update JWX lib to use v2 (#308) Added style and best practices (#298) Add models for Credential Issuer Metadata (#304) Upgrade go version to 1.20.2 (#305) add missing param (#297) interface to any (#296) # Conflicts: # cryptosuite/cryptosuite.go # cryptosuite/jsonwebkey2020.go # cryptosuite/jwssignaturesuite.go # cryptosuite/jwssignaturesuite_test.go # go.mod # go.sum # util/helpers.go # wasm/static/main.wasm

decentralgabe · 2023-03-21T16:31:25Z

@andresuribe87 should have addressed all comments

andresuribe87 · 2023-03-21T18:06:32Z

cryptosuite/bbsplussignatureproofsuite.go

+)
+
+type BBSPlusSignatureProofSuite struct {
+	CryptoSuiteProofType


I think there is a misunderstanding there. Embedding does not force implementation of the interface. Instead, you are saying that the BBSPlusSignatureProofSuite has an element inside it that implements that interface. Said another way, BBSPlusSignatureProofSuite.CryptoSuiteProofType will always be nil, since GetBBSPlusSignatureProofSuite() does not instantiate it. See the example below

type suite struct { fooInter } type fooInter interface{} func GetSuite() *suite { return new(suite) } func TestFoo(t *testing.T) { assert.Nil(t, GetSuite().fooInter) }

I think that's different from your intention, which is that BBSPlusSignatureProofSuite should comply with the interface. That is achieved with line 325 alone.

andresuribe87 · 2023-03-21T18:07:35Z

cryptosuite/bbsplussignaturesuite.go

+	BBSPlusSignatureSuiteCanonicalizationAlgorithm string        = "https://w3id.org/security#URDNA2015"
+	// BBSPlusSignatureSuiteDigestAlgorithm uses https://www.rfc-editor.org/rfc/rfc4634
+	BBSPlusSignatureSuiteDigestAlgorithm gocrypto.Hash = gocrypto.BLAKE2b_384
+	// BBSPlusSignatureSuiteProofAlgorithm  uses https://www.rfc-editor.org/rfc/rfc7797


There is no const named BBSPlusSignatureSuiteProofAlgorithm. Is that intentional?

andresuribe87 · 2023-03-21T18:07:48Z

cryptosuite/bbsplussignaturesuite.go

+)
+
+type BBSPlusSignatureSuite struct {
+	CryptoSuiteProofType


As above, I think this should be removed.

andresuribe87 · 2023-03-21T18:08:48Z

cryptosuite/bbsplussignaturesuite.go

+
+var _ CryptoSuiteProofType = (*BBSPlusSignatureSuite)(nil)
+
+func (BBSPlusSignatureSuite) Marshal(data interface{}) ([]byte, error) {


Prefer any over interface{}. I think you had already done that in a PR?

yeah this pr predated the other, will update

cryptosuite/bbsplussignaturesuite.go

* origin/main: make target optional (#316) ION SDK (#307) # Conflicts: # go.mod # go.sum # wasm/static/main.wasm

andresuribe87

Awesome work! One minor reminder

andresuribe87 · 2023-03-23T00:14:01Z

cryptosuite/bbsplussignatureproofsuite.go

+)
+
+type BBSPlusSignatureProofSuite struct {
+	CryptoSuiteProofType


Reminder to do this.

andresuribe87 · 2023-03-23T00:18:07Z

go.mod

-	github.com/klauspost/cpuid/v2 v2.0.9 // indirect
+	github.com/hyperledger/aries-framework-go/spi v0.0.0-20221025204933-b807371b6f1e // indirect
+	github.com/kilic/bls12-381 v0.1.1-0.20210503002446-7b7597926c69 // indirect
+	github.com/kr/text v0.2.0 // indirect


This looks sus haha. Who's using this?

github.com/hyperledger/aries-framework-go@v0.1.9-0.20230217102417-a948231f8452 github.com/klauspost/compress@v1.10.0

looks like they have widely used packages https://github.com/klauspost/compress

I'm guessing some CPUs can't generate these keys properly https://github.com/klauspost/cpuid but this is an indirect dep even for aries so im not certain

decentralgabe and others added 6 commits February 15, 2023 13:25

simple bbs key usage

5d3e9cf

some progress

47d5a63

progress

ff80ef6

Merge branch 'main' into bbs-plus

ba641de

switch to any

425adbd

sign verify working

f28b789

decentralgabe commented Feb 18, 2023

View reviewed changes

cryptosuite/bbsplussignatureproofsuite.go Show resolved Hide resolved

decentralgabe and others added 19 commits February 17, 2023 18:32

create derive proof

50e56c6

implemented but untested

24c1d20

some bugs

835c3e3

fix test issue

f783cc0

not passing test vector yet

42cc8e2

problem with bls verify

7eeb7b3

fix test vector

1402b90

problem with signing and derive verify

e477161

fix signing with suite

d6e597d

it works

39c8c84

tests pass

4e34014

not there yet

384f5e4

progress with nonces

2d8630e

one bug fixed

c196a03

same input different output

340d947

add compaction

c72ce22

not working fully

23f5e49

bbs+ working

77be15e

Merge branch 'main' into bbs-plus

2319afb

decentralgabe marked this pull request as ready for review February 22, 2023 22:08

decentralgabe requested review from nitro-neal and andresuribe87 as code owners February 22, 2023 22:08

decentralgabe and others added 9 commits February 23, 2023 12:25

better err messages

b6ae137

Update cryptosuite/bbsplussignaturesuite.go

eb8fdf3

Co-authored-by: Andres Uribe <auribe@tbd.email>

pr comments

310b499

Merge remote-tracking branch 'origin/bbs-plus' into bbs-plus

e7883da

* origin/bbs-plus: Update cryptosuite/bbsplussignaturesuite.go # Conflicts: # cryptosuite/bbsplussignaturesuite.go

remove provable from cvh

8f98524

pr comments

805f1a2

move generic provable

b84cfdb

pr comments

425b0c6

Merge branch 'main' into bbs-plus

2b73629

andresuribe87 reviewed Mar 20, 2023

View reviewed changes

decentralgabe and others added 7 commits March 21, 2023 09:17

Update cryptosuite/bbsplussignaturesuite.go

a981ef8

Co-authored-by: Andres Uribe <auribe@tbd.email>

pr comments

3e816a5

Update cryptosuite/bbsplussignatureproofsuite.go

0d36a93

Co-authored-by: Andres Uribe <auribe@tbd.email>

Update cryptosuite/bbsplussignatureproofsuite.go

7d968e2

Co-authored-by: Andres Uribe <auribe@tbd.email>

pr comments

247339f

merge

625fe9c

simpler proof handling

5562aa0

andresuribe87 reviewed Mar 21, 2023

View reviewed changes

update re:pr comments

66c7a3f

decentralgabe mentioned this pull request Mar 22, 2023

Refactor Cryptosuites #315

Open

decentralgabe added 2 commits March 22, 2023 14:29

Merge remote-tracking branch 'origin/main' into bbs-plus

f1fee95

* origin/main: make target optional (#316) ION SDK (#307) # Conflicts: # go.mod # go.sum # wasm/static/main.wasm

merge

4f1c00b

andresuribe87 approved these changes Mar 23, 2023

View reviewed changes

pr comment

fc73c86

decentralgabe merged commit 3d48dfb into main Mar 23, 2023

decentralgabe deleted the bbs-plus branch March 23, 2023 00:38

decentralgabe mentioned this pull request Mar 24, 2023

Support for Privacy Preserving Credentials TBD54566975/ssi-service#343

Open

andresuribe87 mentioned this pull request May 10, 2023

[Idea] Selective Disclosure with JWT-SD #197

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for BLS keys and BBS+ signatures #288

Add support for BLS keys and BBS+ signatures #288

decentralgabe commented Feb 17, 2023 •

edited

Loading

codecov-commenter commented Feb 18, 2023 •

edited

Loading

andresuribe87 left a comment

andresuribe87 Mar 20, 2023

andresuribe87 Mar 20, 2023

andresuribe87 Mar 20, 2023

decentralgabe Mar 21, 2023

andresuribe87 Mar 20, 2023

decentralgabe Mar 21, 2023

andresuribe87 Mar 20, 2023

decentralgabe Mar 21, 2023

decentralgabe commented Mar 21, 2023

andresuribe87 Mar 21, 2023

andresuribe87 Mar 21, 2023

andresuribe87 Mar 21, 2023

decentralgabe Mar 22, 2023

andresuribe87 Mar 21, 2023

decentralgabe Mar 22, 2023

andresuribe87 left a comment

andresuribe87 Mar 23, 2023

andresuribe87 Mar 23, 2023

decentralgabe Mar 23, 2023

decentralgabe Mar 23, 2023

decentralgabe Mar 23, 2023 •

edited

Loading


		var _ CryptoSuiteProofType = (*BBSPlusSignatureSuite)(nil)

		func (BBSPlusSignatureSuite) Marshal(data interface{}) ([]byte, error) {

Add support for BLS keys and BBS+ signatures #288

Add support for BLS keys and BBS+ signatures #288

Conversation

decentralgabe commented Feb 17, 2023 • edited Loading

codecov-commenter commented Feb 18, 2023 • edited Loading

Codecov Report

andresuribe87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

decentralgabe commented Mar 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andresuribe87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

decentralgabe Mar 23, 2023 • edited Loading

Choose a reason for hiding this comment

decentralgabe commented Feb 17, 2023 •

edited

Loading

codecov-commenter commented Feb 18, 2023 •

edited

Loading

decentralgabe Mar 23, 2023 •

edited

Loading