search

openid / sharedsignals

OpenID Shared Signals Working Group Repository
45 stars 11 forks source link

Are subjects required in SSF events? #60

Closed FragLegs closed 1 year ago

FragLegs commented 1 year ago

The spec says that the subject claim is REQUIRED for all SSF events but also makes the subject OPTIONAL for StatusChanged events and doesn't mention subject at all for the Verification event.

We need to resolve this inconsistency.

tulshi commented 1 year ago

Where do we specify in the SSF spec that a subject is required?

FragLegs commented 1 year ago

Hmm, those links have drifted. Sorry.

scvenema commented 1 year ago

Building on Shayne's concerns, §11.1.8.1 of the current framework draft seems to imply there is some unfinished business in harmonizing this SSF framework spec with the Subject Identifiers for SETs - draft 17 spec? I'm referring to the phrase, "The Shared Signals Framework is asking for further restrictions"...

We seem to have three different subject-like fields in play here:

  1. "sub" as defined in RFC 7519 §4.1.2
  2. "sub_id" as defined in §4.1 of the Subject Identifiers for SETs spec (draft 17)
  3. "subject" as defined in our SSF Frameworks draft spec, which is part of an event JWT claim, and requires the exclusion of "sub" (and "sub_id") usages for SETs Plus a new term defined in the framework spec: "Subject Principal", which seems to overlap with the term "subject" used in the secevent specs.

It all seems a bit muddy to an SSF noob like me. I understand that "sub_id" is needed because the "sub" claim's value has type StringOrURI, while "sub_id" claim's value is a JSON object instead. I expect that there is some history behind all of this that I'm not aware of. But, at a minimum, I think we could add some additional non-normative explanatory text somewhere in the framework spec that clarifies these distinctions. I'd be happy to draft some text, but I think it might be useful to first discuss this on an upcoming WG call.

I'm happy to open this as a separate issue if desired.

independentid commented 1 year ago

Just an FYI item. The sub claim is problematic in the context of SET tokens for 2 major reasons:1 . Sub in a JWT was always defined in connection with an issuer.  However, in OpenId if a client issues an event its “iss” value is different. This is why the subject identifier draft came about and why there is a format for this. 2. There is a security worry (not sure how legit it is) that a set might be confused as authorization. This is why rfc8417 recommends against using the “sub” claim. I would recommend implementers NOT use “sub” per 8417. PhilOn Jun 24, 2023, at 3:44 PM, Steve Venema @.***> wrote: Building on Shayne's concerns, §11.1.8.1 of the current framework draft seems to imply there is some unfinished business in harmonizing this SSF framework spec with the Subject Identifiers for SETs - draft 17 spec? I'm referring to the phrase, "The Shared Signals Framework is asking for further restrictions"... We seem to have three different subject-like fields in play here:

"sub" as defined in RFC 7519 §4.1.2 "sub_id" as defined in §4.1 of the Subject Identifiers for SETs spec (draft 17) "subject" as defined in our SSF Frameworks draft spec, which is part of an event JWT claim, and requires the exclusion of "sub" (and "sub_id") usages for SETs Plus a new term defined in the framework spec: "Subject Principal", which seems to overlap with the term "subject" used in the secevent specs.

It all seems a bit muddy to an SSF noob like me. I understand that "sub_id" is needed because the "sub" claim's value has type StringOrURI, while "sub_id" claim's value is a JSON object instead. I expect that there is some history behind all of this that I'm not aware of. But, at a minimum, I think we could add some additional non-normative explanatory text somewhere in the framework spec that clarifies these distinctions. I'd be happy to draft some text, but I think it might be useful to first discuss this on an upcoming WG call. I'm happy to open this as a separate issue if desired.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

FragLegs commented 1 year ago

Thank you @scvenema and @independentid . I am going to move these comments over to issue #52 where this concern is being discussed. The current issue was about an inconsistency in requirements that has been addressed in PR #70. I want to make sure your comments aren't lost when that PR merges and this issue closes.