Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect iss value on tokens, not the same as issuer value in OpenID configuration #1609

Open
michielbdejong opened this issue Aug 12, 2021 · 10 comments

Comments

@michielbdejong
Copy link
Member

michielbdejong commented Aug 12, 2021

When trying to log in to CSS using the run-against-css.sh script of the WAC tests, you get:

2021-08-11T12:45:33.820Z [DPoPWebIdExtractor] warn: Error verifying WebID via DPoP-bound access token: The access token issuer doesn't match its associated WebID's trusted OIDC issuers.
Actual: https://solidcommunity.net/
Expected: https://solidcommunity.net

So maybe we can change NSS to remove that trailing slash from the issuer?

@RubenVerborgh
Copy link
Contributor

https://openid.net/specs/openid-connect-core-1_0.html#IssuerIdentifier reads:

The issuer returned by discovery MUST exactly match the value of iss in the ID Token. 

However, https://solidcommunity.net/.well-known/openid-configuration returns

"issuer":"https://solidcommunity.net"

so it is an NSS bug that it mints tokens with a different issuer.

@RubenVerborgh RubenVerborgh changed the title Trailing slash on issuer Incorrect iss value on tokens, not the same as issuer value in OpenID configuration Aug 12, 2021
@bourgeoa
Copy link
Member

@jaxoncreed are you ok with the fact that it is a NSS bug.

@jaxoncreed
Copy link
Contributor

Yeah, if there is no trailing slash in the openid-configuration, there shouldn't be one in the token.

bourgeoa referenced this issue in nodeSolidServer/oidc-auth-manager Aug 22, 2021
@michielbdejong
Copy link
Member Author

michielbdejong commented Aug 30, 2021

The value on https://solidcommunity.net/.well-known/openid-provider is loaded from /mnt/volume_lon1_01/.db/oidc/op/provider.json, I edited it in that file, and that fixed the direct issue for now.
But we should also fix the code that (re-)generates this file when it is not there.

EDIT: this was a bad idea because it caused #1613 so undid this now.

@bourgeoa
Copy link
Member

bourgeoa commented Aug 31, 2021

@michielbdejong So in fact you did not remove trailing slash to issuer but added a trailing slash.

"issuer" : "https://solidcommunity.net/"

@michielbdejong
Copy link
Member Author

michielbdejong commented Aug 31, 2021

@bourgeoa yeah, and it broke something else so i'm trying to put it back now - can you pop into https://gitter.im/solid/solidcommunity.net ?

EDIT: fixed now, the server is back to how it was before last night

@michielbdejong
Copy link
Member Author

I'm now trying out a less invasive work-around, namely hard-coding the issuer in ./node_modules/@solid/oidc-op/src/handlers/OpenIDConfigurationRequest.js

@michielbdejong
Copy link
Member Author

I tried out various things but it also ties in with the issuer linked to from the webid profile, and apparently CSS isn't successfully extracting the issuer from the DPop token that the WAC tests send, so in the end I gave up trying to fix this situation and just commented out https://github.com/solid/access-token-verifier/blob/3f99fb7/src/algorithm/verifySolidAccessTokenIssuer.ts#L17 in my system-under-test. This issue still needs fixing though.

@RubenVerborgh
Copy link
Contributor

apparently CSS isn't successfully extracting the issuer from the DPop token that the WAC tests send

Well, the token is there and CSS spells it out, so that can't really be the issue.

The issue is a bug in the test suite where https://solidtestsuite.solidcommunity.net/profile/card#me uses https://solidcommunity.net/ as issuer rather than the advertised https://solidcommunity.net. Because as you can see in https://github.com/solid/access-token-verifier/blob/3f99fb7/src/algorithm/verifySolidAccessTokenIssuer.ts#L9, it compares the advertised issuer against those listed in the WebID document, which should be exact matches according to https://solid.github.io/solid-oidc/#resource-access-validation.

This means that he test suite is actually testing whether NSS behaves incorrectly, and confirms that it's not a good idea to base a test suite on it.

@RubenVerborgh
Copy link
Contributor

In related news, why is https://github.com/solid/web-access-control-tests/blob/main/run-against-css.sh publicly listing actual username/password combinations? Separately from the dozens of bad reasons I can think of, it basically means that anyone can break the test suite at any time. As tempted as I felt to fix the slash in https://solidtestsuite.solidcommunity.net/profile/card#me, the fact that I can does not seem good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants