Implements SD-JWT #103

andresuribe87 · 2023-10-27T17:15:18Z

Overview

Implements a module that supports SD-JWT according to https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-05.htm
This PR will be a first step towards fully closing #56

Description

SD-JWT is becoming the de-factor standard. This PR implements the basic functionality in order to support SD-JWT from the perspective of the holder, the issuer, and the verifier. Simple documentation has been added in this iteration. It's published as a separate module, and it's dependencies are kept lean on purpose, since this module is meant to be used independently from DIDs and VCs.

Most of the design takes inspiration from the Nimbus library, where a JWT needs to be created first, and then signed.

Feedback would be much appreciated.

How Has This Been Tested?

Unit tests everywhere.

codecov · 2023-10-27T17:19:45Z

Codecov Report

Merging #103 (4eb9bcb) into main (a0e4225) will increase coverage by 0.95%.
Report is 7 commits behind head on main.
The diff coverage is 67.36%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
+ Coverage   78.96%   79.91%   +0.95%     
==========================================
  Files          33       41       +8     
  Lines        1930     2788     +858     
  Branches      265      477     +212     
==========================================
+ Hits         1524     2228     +704     
- Misses        274      378     +104     
- Partials      132      182      +50

Components	Coverage Δ
credentials	`84.60% <ø> (+3.92%)`	⬆️
crypto	`38.07% <ø> (ø)`
dids	`90.98% <ø> (+1.38%)`	⬆️
common	`80.55% <ø> (+0.10%)`	⬆️

nitro-neal · 2023-10-27T17:29:22Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+    }
+
+    // Check that the SD-JWT is valid using nbf, iat, and exp claims, if provided in the SD-JWT, and not selectively disclosed.
+    val claimsVerifier = DefaultJWTClaimsVerifier<SecurityContext>(null, null)


Are these nulls in the constructor expected?

Yes. Those provide fields that are required. passing null validated the nbf, iat, and exp claims only when they're present.

nitro-neal · 2023-10-27T17:31:17Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+
+      // Ensure that a signing algorithm was used that was deemed secure for the application. Refer to [RFC8725], Sections 3.1
+      // and 3.2 for details.
+      require(verificationOptions.supportedAlgorithms.contains(keyBindingJwt.header.algorithm)) {


did similar supportedAlgorithms above, maybe pull out into a fuction

I didn't quite follow the suggestion, mind elaborating?

nitro-neal · 2023-10-27T17:31:38Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+          "supported algorithms (${verificationOptions.supportedAlgorithms})"
+      }
+      // The none algorithm MUST NOT be accepted.
+      require(keyBindingJwt.header.algorithm != JWSAlgorithm.NONE) {


verificationOptions.supportedAlgorithms are meant to be user specified, BUT the spec explicitly says that NONE cannot be part of it. Let me know if that makes sense. If not, mind making a concrete suggestion to improve?

nitro-neal · 2023-10-27T17:35:14Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+
+      // Ensure that the disclosure is object or array disclosure
+      when (disclosureElems.size) {
+        3 -> {


fancy fancy

nitro-neal · 2023-10-27T17:35:55Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+          )
+        }
+
+        else -> throw IllegalArgumentException("Disclosure \"$encodedDisclosure\" must have exactly 3 elements")


Is this correct? Seems like there is a case to handle 2 elements

fixed, thx!

nitro-neal · 2023-10-27T17:39:42Z

sdjwt/src/main/kotlin/web5/security/SdJwtBlinder.kt

+
+              repeat(totalDigests(arrayDisclosures.size) - arrayDisclosures.size) {
+                val randBytes = ByteArray(32)
+                SecureRandom().nextBytes(randBytes)


We so need external audits on this stuff :hahaha:

Curious what parts were red flags?

nitro-neal · 2023-10-27T17:41:57Z

sdjwt/src/main/kotlin/web5/security/SdJwtBlinder.kt

+
+
+/** Interface for generating salt values as described in https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-05.html#disclosures_for_object_properties */
+public interface ISaltGenerator {


Is there not a library for this? Is this a special case sd-jwt salt?

There isn't a library that does exactly what's needed. This module provides an implementation below, which is used by default. It can be overriden, which was super useful for tests.

nitro-neal · 2023-10-27T17:45:58Z

sdjwt/src/main/kotlin/web5/security/SdJwtBlinder.kt

+
+}
+
+private fun getNextPowerOfTwo(n: Int): Int {


wow some leetcode stuff

More like CHAD stuff hahaha

nitro-neal · 2023-10-27T17:51:18Z

sdjwt/src/test/kotlin/web5/security/SdJwtTest.kt

+        "zip_code": "12345"
+      }
+    }""".trimIndent()
+    val claimsToBlind = mapOf(


Why can't this be dynamic? Is it to give the option to the user?
If they provide null as claimsToBlind can it dynamically do default blinding to all?

The issuer of an SdJwt must, at some point, select which claims they'll allow the holder to selectively disclose. For those, they must also say how they'll be disclosable, since there are various mechanisms.

What do you think should be the way in which they are all blinded by default?

nitro-neal

Wow mind blown. Great stuff! I need to reread the sd-jwt spec and look over one more time.

Will this only be used for credentials? Does it make sense to put it in a sub package of credentials or something?

There will be more changes in the credentials package to make it user friendly to get SD JWT creds along with the normal cred they made?

VerifiableCredential.create(sdJwt=ture) // something like this?

andresuribe87 · 2023-10-31T16:04:07Z

Will this only be used for credentials?

No, it can be used for other types credentials, that aren't w3c vcs.

There will be more changes in the credentials package to make it user friendly to get SD JWT creds along with the normal cred they made?

Yes, but that comes later. Once we have support for VC2.0, then we'll need to allow securing it using sd-jwt. That will likely be the piece that does belong inside the credentials module.

VerifiableCredential.create(sdJwt=ture) // something like this?

Something like that sounds reasonable.

sdjwt/src/README.md

nitro-neal · 2023-12-20T20:40:07Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+   * Verifies this SD-JWT according to [verificationOptions].
+   */
+  @Throws(Exception::class)
+  public fun verify(


Maybe add some more detailed explanations on these comments, similar to our other packages, and add example code if it is short and sweet

Added more documentation, and an example.

nitro-neal · 2023-12-20T20:44:45Z

sdjwt/src/main/kotlin/web5/security/SdJwtUnblinder.kt

+  public fun unblind(
+    serializedSdJwt: String
+  ): JWTClaimsSet {
+    // 2. Separate the Presentation into the SD-JWT, the Disclosures (if any), and the Holder Binding JWT (if provided).


Suggested change

// 2. Separate the Presentation into the SD-JWT, the Disclosures (if any), and the Holder Binding JWT (if provided).

// Separate the Presentation into the SD-JWT, the Disclosures (if any), and the Holder Binding JWT (if provided).

nitro-neal · 2023-12-20T20:44:55Z

sdjwt/src/main/kotlin/web5/security/SdJwtUnblinder.kt

+    val disclosuresByDigest = sdJwt.disclosures.associateBy { it.digest(hashAlg) }
+
+    // Process the Disclosures and _sd keys in the SD-JWT as follows:
+    //


Suggested change

//

nitro-neal · 2023-12-20T20:46:03Z

sdjwt/src/test/kotlin/web5/security/SdJwtBlinderTest.kt

+  }
+
+  @Test
+  fun testExample1() {


fix the test names in this file

Fixed, and added where they came from

nitro-neal · 2023-12-20T20:47:06Z

sdjwt/src/test/kotlin/web5/security/SdJwtBlinderTest.kt

+    val keyManager = InMemoryKeyManager()
+    DidKey.create(keyManager)


are these lines needed?

They aren't! Removed.

nitro-neal · 2023-12-20T20:52:33Z

settings.gradle.kts

@@ -1,3 +1,4 @@
 rootProject.name = "web5"
 include("common", "crypto", "dids", "credentials")


Suggested change

include("common", "crypto", "dids", "credentials")

include("common", "crypto", "dids", "credentials", "sdjwt")

Actually, separated everything into single lines and sorted alphabetically.

nitro-neal · 2023-12-20T20:57:00Z

Why is this another package?

In the future should we abstract like a lot of this away and have something like:

data class StreetCredibility(val localRespect: String, val legit: Boolean)

val vc = VerifiableCredential.create(
  type = "StreetCred",
  issuer = "did:example:issuer",
  subject = "did:example:subject",
  data = StreetCredibility(localRespect = "high", legit = true)
)

val sdVcs = vc.toSdVcs()

Am I thinking about this the wrong way?

nitro-neal

Happy to get this in since it is isolated in another package, but I want to see how we fit this into the bigger picture of our VerifiableCredential packages.

Would love to the see the api interface easier to understand and/or integrated in our existing VC package if possible

decentralgabe · 2023-12-21T19:43:39Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+    return when (jwk) {
+      is ECKey -> jwk.toPublicKey()
+      is OctetKeyPair -> jwk.toPublicKey()
+      is OctetSequenceKey -> jwk.toSecretKey()


why is this secret but the others are public?

Ooof, good catch. Removed this line. Should only be using asymmetric keys.

decentralgabe · 2023-12-21T19:44:31Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+          )
+        }
+
+        2 -> {


nit: put 2 before 3

decentralgabe

lgtm - would appreciate a review from @mistermoe or @phoebe-lew before merging

phoebe-lew · 2024-01-18T04:32:03Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+ * Note: While [issuerJwt] and [keyBindingJwt] are of type [SignedJWT], they may or may not be signed.
+ */
+public class SdJwt(
+  public val issuerJwt: SignedJWT,


So in order to use SD-JWT, if for example if I have VC in JWT format created by the web5/credentials package, do I first need to convert it into the nimbusds SignedJWT object?

Yes you do.

Assuming you have an object of type String that was returned from the VerifiableCredential.sign function, you can do something like this:

val vcJwtString = "" val issuerJwt = SignedJWT.parse(vcJwtString)

The motivation behind this design for a the SdJwt object was to:

Reduce cognitive overhead by only allowing specific types instead of generic primitives (i.e. SignedJWT vs String).

Allow for others to use the sd-jwt library by itself by decoupling it from other existing packages in web5. Later on, changes can be made to web5/credentials and web5/dids to make it easy to create an sd-jwt that contains a VC.

phoebe-lew · 2024-01-18T04:36:01Z

sdjwt/src/main/kotlin/web5/security/SdJwt.kt

+/**
+ * Represents a disclosure for an Object Property as defined in https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-05.html#name-disclosures-for-object-prop.
+ */
+public class ObjectDisclosure(


Maybe move out disclosure classes into separate file?

phoebe-lew · 2024-01-18T04:37:26Z

sdjwt/src/main/kotlin/web5/security/SdJwtBlinder.kt

+    return Pair(blinded, allDisclosures)
+  }
+
+  fun toBlindedClaimsAndDisclosures(


@andresuribe87 can you add comments or doc for this function? For code readability rather than external documentation

sdjwt/src/main/kotlin/web5/security/SdJwtBlinder.kt

phoebe-lew · 2024-01-18T04:40:57Z

sdjwt/src/test/kotlin/web5/security/SdJwtTest.kt

+  }
+
+  @Test
+  fun `whole flow from issuer to holder to verifier`() {


super helpful to understand how the whole thing works!

phoebe-lew · 2024-01-18T04:41:42Z

sdjwt/src/test/kotlin/web5/security/SdJwtTest.kt

+      .build()
+    val sdJwt = builder.build()
+
+    sdJwt.signAsIssuer(issuerSigner)


Can I not sign this with my did?

No. As mentioned in #103 (comment), the motivation was to make this a lower level lib. What you're suggesting is an excellent idea, and I would encourage that to be done in a separate PR that changes the public APIs that exist in the web5/credentials module.

andresuribe87 added 13 commits October 25, 2023 03:13

Pre detekt commit, with simple functionality

f4d1834

Tests pass.

2bcc336

Removing cruft

213bb31

Merge branch 'main' into sd_jwt

6309e1d

Prune gradle file.

5fdad7c

Ok we're good at this point

46b9e52

more goodies

fea91e9

Some docs.

5d18b3a

Anchors

4227951

anchors

be0ebaf

one todo down

6a0849e

Only allow the set of supported algos.

60caf3d

names

d0399ae

andresuribe87 requested review from mistermoe, nitro-neal, tomdaffurn and phoebe-lew as code owners October 27, 2023 17:15

nitro-neal reviewed Oct 27, 2023

View reviewed changes

Fix naming

bd95849

andresuribe87 added 2 commits December 12, 2023 11:56

Merge branch 'main' into sd_jwt

162c013

Post merge fixes.

c7ef78d

nitro-neal reviewed Dec 20, 2023

View reviewed changes

sdjwt/src/README.md Show resolved Hide resolved

nitro-neal reviewed Dec 20, 2023

View reviewed changes

nitro-neal approved these changes Dec 20, 2023

View reviewed changes

decentralgabe reviewed Dec 21, 2023

View reviewed changes

andresuribe87 added 6 commits December 21, 2023 14:51

Merge branch 'main' into sd_jwt

404f97b

Format

db9bc6b

More docs

2e66d86

reviews

34c58c7

Names and more comments.

eeb4ad2

Use custom exceptions.

4b4aa1d

decentralgabe approved these changes Jan 8, 2024

View reviewed changes

phoebe-lew reviewed Jan 18, 2024

View reviewed changes

andresuribe87 added 3 commits January 18, 2024 11:15

comment

d92bfe4

comment

9f21a14

move disclosure

4eb9bcb

jiyoontbd mentioned this pull request Feb 14, 2024

Support SD-JWT as format of VC 2.0 creds #56

Open

jiyoontbd linked an issue Feb 16, 2024 that may be closed by this pull request

Support SD-JWT as format of VC 2.0 creds #56

Open

decentralgabe requested review from diehuxx, kirahsapong, jiyoontbd and frankhinek as code owners August 13, 2024 04:25



		/** Interface for generating salt values as described in https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-05.html#disclosures_for_object_properties */
		public interface ISaltGenerator {

	// 2. Separate the Presentation into the SD-JWT, the Disclosures (if any), and the Holder Binding JWT (if provided).
	// Separate the Presentation into the SD-JWT, the Disclosures (if any), and the Holder Binding JWT (if provided).

		val keyManager = InMemoryKeyManager()
		DidKey.create(keyManager)

		@@ -1,3 +1,4 @@
		rootProject.name = "web5"
		include("common", "crypto", "dids", "credentials")

+                        )
+                      }
+-> {

Implements SD-JWT #103

Are you sure you want to change the base?

Implements SD-JWT #103

Conversation

andresuribe87 commented Oct 27, 2023 • edited Loading

Overview

Description

How Has This Been Tested?

codecov bot commented Oct 27, 2023 • edited Loading

Codecov Report

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

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

nitro-neal left a comment

Choose a reason for hiding this comment

andresuribe87 commented Oct 31, 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

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

nitro-neal commented Dec 20, 2023

nitro-neal 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 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

andresuribe87 commented Oct 27, 2023 •

edited

Loading

codecov bot commented Oct 27, 2023 •

edited

Loading