oauth-wg / oauth-selective-disclosure-jwt

https://datatracker.ietf.org/doc/draft-ietf-oauth-selective-disclosure-jwt/
Other
56 stars 30 forks source link

Presented form of an SD-JWT using holder binding doesn't prove integrity of disclosures back to holder #277

Closed tplooker closed 1 year ago

tplooker commented 1 year ago

From what I can understand an SD-JWT that is presented by a holder using holder binding doesn't prove integrity of the disclosures to the holder, only instead back to the issuer. This means a party in the middle could remove values that were intended to be presented from a presented form SD-JWT without a verifier being able to detect it, similarly a party that knows the claim values and salts could inject other disclosure values beyond what the holder intended to present (e.g an issuer).

In general I believe the presented form of an SD-JWT should provide holder integrity over the disclosures when holder binding is in use.

tplooker commented 1 year ago

As a proposed solution to this that doesn't really effect the existing structure, I would suggest adding in a claim to the holder binding JWT that includes an array of the digests being revealed in the overall presentation and add language that requires processing of and validation of this claim by a verifier.

tplooker commented 1 year ago

For example the HB JWT would look like

{
  "nonce": "XZOUco1u_gEPknxS78sWWg",
  "aud": "https://example.com/verifier",
  "iat": 1677838084,
  "_sd": [
    "5nXy0Z3QiEba1V1lJzeKhAOGQXFlKLIWCLlhf_O-cmo",
    "9gZhHAhV7LZnOFZq_q7Fh8rzdqrrNM-hRWsVOlW3nuw",
    "S-JPBSkvqliFv1__thuXt3IzX5B_ZXm4W2qs4BoNFrA",
    "bviw7pWAkbzI078ZNVa_eMZvk0tdPa5w2o9R3Zycjo4",
    "o-LBCDrFF6tC9ew1vAlUmw6Y30CHZF5jOUFhpx5mogI",
    "pzkHIM9sv7oZH6YKDsRqNgFGLpEKIj3c5G6UKaTsAjQ",
    "rnAzCT6DTy4TsX9QCDv2wwAE4Ze20uRigtVNQkA52X0"
   ]
}

Where the digests in the _sd array match the disclosures presented.

tplooker commented 1 year ago

Another possible solution is to just include a hash of everything that precedes the HB JWT, this would be simpler to verify and also more terse in overall payload length.

paulbastian commented 1 year ago

That's great Tobias. I think it's easier to include Hash (SD-JWT~disclosure 1~disclosure 2~) because this is what the holder has to build anyway? The verifier would strip of the HB-JWT, hash the rest and check if it matches the new claim in the HB-JWT

bc-pi commented 1 year ago

It took me some work to convince @danielfett that the holder signing the disclosures wasn't needed. So I must admit that I wasn't exactly exited to see this issue and reopen the discussion. He's "the security guy" though so I'll defer to his judgement here. But I remain unconvinced that it is needed. The integrity of the disclosures as issued by the issuer is the important thing. The integrity of the number of disclosures sent by the holder over what really should be a secure channel anyway just doesn't seem like a concern to me. Maybe I'm not paranoid enough...

If we do add something for this, I prefer the approach of one hash in the HB-JWT that is calculated over the whole presentation up to but not including the HB-JWT. That kind of thing does introduce one more opportunity for interop problems, however, and ones that can be difficult to debug in practice.

tplooker commented 1 year ago

I agree that integrity of the disclosures back to the issuer is the most important, however I do think holder integrity is also required otherwise it leaves presentations free to a form of unintuitive manipulation that I can see having downstream consequences such as a verifier stripping information from a presentation and saying "i wasnt presented that claim", or a man in the middle dropping information from a presentation. Really what a lack of holder integrity does here in general is introduce ambiguity, e.g did the verifier strip claims from a presentation, did the holder never reveal them or did someone in the middle drop them. I understand that we want to be careful about managing implementation complexity to avoid interoperability issues, but I do think the single hash over everything that precedes the HB-JWT is relatively easy to perform.

danielfett commented 1 year ago

We just discussed this among the editors. We see the following problems with signing over the disclosures or their digests:

Meanwhile, we think there are good reasons not do this:

tplooker commented 1 year ago

Appreciate the context but I'm not sure I agree with the conclusions here

When holder binding is not used, this method of proving integrity of the disclosure set is not possible.

This is true but I dont think this a reason not to do this, following this logic one would not define a HB-JWT in the first place because SD-JWTs without it dont have replay attack detection capability.

For the JSON Serialization, we would need to define a different way to do it. There is no obvious way how to sign over the JSON-serialized SD-JWT in that case.

Understand there is complexity here and a solution would need to exist for consistency reasons one such that I believe would work is the solution I proposed above which is slightly more complex then validating a single hash, but very much the kind of expected behaviour already going on with SD-JWT when it comes to validating disclosures.

The specification promises integrity of the data, but does not guarantee to the verifier that a holder wanted to release this exact dataset even in presence of an adversary. We should call this out more expicitly and will create a PR for this.

Im not sure what the presences of an adversary here has to do with it, this is about having no ambiguity around what was sent and that once that presentation is prepared it cant be modified in anyway without it being cryptographically invalid.

In the presence of such a powerful adversary, it seems plausible that the protection can be circumvented or stronger attacks can be mounted; e.g., by manipulating the request to the holder as well.

I think this is kind of a seperate issue, if the holder was tricked into revealing more information then intended that doesn't negate the need for wanting integrity around what was presented, in fact ironically it perhaps underscores the importance of having such an integrity check in the first place.

For these reasons, integrity and non-repudiation of the message, if needed, must be provided by the transport protocol (in the most simple form by TLS to protect integrity, but things like JARM can provide a second layer for integrity and provide non-repudiation).

This is certainly possible but part of the point I was trying to make is it splits where the integrity in a presented SD-JWT comes from, e.g most of it is from the signatures, the rest from the transport and that can make it awkward to evaluate from a security perspective.

Hashing can be hard to debug and adds another complexity for developers.

I mean considering how much hashing is used as a primitive in this spec I really do find it a bit hard to believe this check is where implementations will struggle. I agree it adds another check though which is additional implementation work, but I think the cost is worth it.

As a compromise perhaps it could be added as an optional check in the sort of same way the at_hash is optional in OpenID Connect (for different reasons entirely of course).

bc-pi commented 1 year ago

305 was merged. It adds some security considerations text about what the Key Binding JWT does and does not cover. And some suggestions about what can be done in situations that need more.