Add Client Specific Subjects in Claims

[DerAlbertCom]

Currently every Client get's the same Subject for the User. And this is in case if ASP.NET Identity the UserId. So this is a little Information leaked.

Please add an option (Global and/or per Client) so that it is possible to send a client specific Subject to the client.

From my investigation this can be achieved by DefaultClaimsService which make a (subjectId+clientId).Sha256()

And also

IdentityServer4/src/IdentityServer4/Validation/EndSessionRequestValidator.cs

Line 86 in 74827bc

if (subject.GetSubjectId() != subClaim.Value)

must be changed to recreate the hash, and check for this.

Other Places?

Now seems the time to add this, because 2.0 is not yet released. I willing to implement this.

[TomCJones]

If you are going to that trouble, why not enable pairwise sub. See the oidc spec if the term is unfamiliar to you. I can help if useful. ..Tom's phone On Sep 24, 2017, at 11:29 AM, Albert Weinert <notifications@github.com<mailto:notifications@github.com>> wrote: Currently every Client get's the same Subject for the User. And this is in case if ASP.NET<http://ASP.NET> Identity the UserId. So this is a little Information leaked. Please add an option (Global and/or per Client) so that it is possible to send a client specific Subject to the client. From my investigation this can be achieved by DefaultClaimsService which make a (subjectId+clientId).Sha256() And also https://github.com/IdentityServer/IdentityServer4/blob/74827bc4df39c2234d9490ba5ce032ce806ab475/src/IdentityServer4/Validation/EndSessionRequestValidator.cs#L86 must be changed to recreate the hash, and check for this. Other Places? Now seems the time to add this, because 2.0 is not yet released. I willing to implement this. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#1557>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AKxq1ia8cTpoFk-H7ve-wSJMma01v-V7ks5slp-UgaJpZM4Ph-sx>.

[leastprivilege]

Yes - we are interested in implementing pairwise subject identifier. It's been on our todo list for a long time.

@DerAlbertCom If you are interested in this - we can start a discussion. This won't happen for 2.0 anymore though.

[TomCJones]

I thought a little more about your proposal. Assuming that the goal is for the (subjectId+clientId).Sha256() to be unguessable there must be some secret value known only to the Idp (OP). That could be either the subjectId itself or a salted hash were the salt was highly protected. It may not reasonable for the subectId to be protected in a database that is used for lots of other purposes; perhaps it needs to be held in a separate secured db?

…

________________________________ From: Dominick Baier <notifications@github.com> Sent: Sunday, September 24, 2017 9:38 PM To: IdentityServer/IdentityServer4 Cc: tom jones; Comment Subject: Re: [IdentityServer/IdentityServer4] Add Client Specific Subjects in Claims (#1557) Yes - we are interested in implementing pairwise subject identifier. It's been on our todo list for a long time. @DerAlbertCom<https://github.com/deralbertcom> If you are interested in this - we can start a discussion. This won't happen for 2.0 anymore though. — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#1557 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AKxq1hJfRg8Spk0sO8yN00JflCbDUChuks5sly5UgaJpZM4Ph-sx>.

[DerAlbertCom]

I've read through the specs (http://openid.net/specs/openid-connect-core-1_0.html#SubjectIDTypes).

So, basicly (sector_identifier+account_id+salt) then using SHA or AES. I'm for SHA, don't see really additional security in AES here. Performancewise SHA should be faster, but it think in general the generation and validation of the pairwise subject id is not a performance problem. And on the IdentityServer side we have all needed Information to recreate the hash.

I have have some problems with the specs, because the sector_identifier should be either a sector_identifier_uri, or a redirect_uri (if only one is present). But with these kind identifiers a Client cannot be moved to another domain (renaming company, mergers, product changes etc.).

So, the questions i have. Should we go to the "trouble" with a sector_identitier_url or just simply use a simpler sector_identifier (like clientid) instead of a volatile redirect_uri? The sector_identitifer_uri seems not commonly in use. And if we do this, this may be optional. If the client want's to use redirect_uri or sector_identifier then it will be used, otherwise the clientid.

I think this is also in the specs, because the example 3 for creating the subject is about generated GUIDs as sector_identifier.

Also, as I understand the spec ist that the IdServer should announce the availability of the paire_wise subject. And then the Client MAY opt in for pair wise subject, this should an opt out. But the client registration is not a part of IdServer4, so no problem on that side ;)

[brockallen]

We should add the extra property needed to Client now for 2.0 if we're going to do this.

[DerAlbertCom]

First suggestion

• SectorIdentitierUri
• SectorId (if we don't go with ClientId)
• PairWiseSubjectSalt
• EnablePairWiseSubject

[TomCJones]

I reread the pairwise section.
It is unclear if sector ID is required because of the odd mixture and must and should.
Sector ID is not needed in my application and it will not support sectors in my OP database.
It seems unclear what would happen if sectors were presented by (one of) the client in one case and not in another.
I guess it is a judgement call on sector ID.

[leastprivilege]

We will discuss this...

[brockallen]

I think we'll start with adding PairWiseSubjectSalt whose semantics are that if it's null then we do what we already do today, and if it's non-null then we use it to calculate a pair-wise subject id. We'll add the property for 2.0, and in 2.1 we'll actually implement it. And if you want to do the work on it @DerAlbertCom , then let's start a issue/PR to discuss.

[brockallen]

Ok, I've added the property. The next thing is for 2.1 to add the implementation. For now, I'll mark this as a 21. enhancement. If you want to start on a PR, you can @DerAlbertCom .

[DerAlbertCom]

For clarification.

Following things must be done for this.

1. Adapt DefaultClaimsService to calculate the pair wise subject (with the usage of a IPairWiseSubjectService?)?
2. Change EndSessionRequest Validator, to validator the pair wise subject.
3. Announce the availability of pair wise subject in the metadata

The Pairwise Subject could be calculate with

a. (subject+salt).Sha256() as base64
or
b. pbkdf2(subject, salt, 10000) as base64

a. should be enough.

[brockallen]

Marking as 3.0 since we anticipate breaking changes. I think we all knew this, but just doing it for formality.

[DerAlbertCom]

I'm ok with that. On the current project we are far away from connection external clients, and this is the main need for that feature in my point of view.

[kinosang]

One more question, how can we retrieve the user information when we use pairwise subject? Should we add a new table to store these pairwise subject?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.