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

Compatibility with Content Security Policy without 'unsafe-eval'? #21

Open
josephguillaume opened this issue Jan 31, 2021 · 2 comments
Open

Comments

@josephguillaume
Copy link

Describe the bug
nodeSolidServer/solid-auth-client#162 is an issue here too.
I'm not sure what the relationship is with solid-auth-client or the degree to which CSP compatibility is a goal here compared to solid-client-authn, but new Function calls still exist in the solid-auth-fetcher bundle.
(I'm mostly adding this in case others run into an error)

To Reproduce
Specify a Content-Security-Policy header without 'unsafe-eval'
Login ultimately fails with "Could not obtain the key to sign the token with."

@josephguillaume
Copy link
Author

josephguillaume commented Feb 19, 2021

It seems two calls to new Function can be avoided in webpack.browser.js by using:

node: {
      global: false,
      setImmediate: false,
      clearImmediate: false,
    },

The last call is in the ajv package:
https://github.com/ajv-validator/ajv/blob/9376841f1dc963f59b02bdf94800b9fb7822cb24/lib/compile/index.ts#L159
It appears the only way to avoid this is to compile schemas prior to runtime: https://github.com/ajv-validator/ajv/blob/9200e928c8927bf6c626d5f9488b6e9ca7e6171d/docs/security.md#content-security-policy

It looks like that would work for loginInputOptionsSchema:
https://github.com/solid/solid-auth-fetcher/blob/9181f67377db1c24c2e511b64bdca43be3ccb0a2/src/AuthFetcher.ts#L59-L62

The use of validateSchema in safeGet in principle takes an arbitrary schema:
https://github.com/solid/solid-auth-fetcher/blob/9181f67377db1c24c2e511b64bdca43be3ccb0a2/src/localStorage/StorageUtility.ts#L158

However, the calls to safeGet only use jwkSchema and issuerConfigSchema, so it should be possible to compile these ahead of time too.
https://github.com/solid/solid-auth-fetcher/blob/9181f67377db1c24c2e511b64bdca43be3ccb0a2/src/dpop/DpopClientKeyManager.ts#L61-L64

https://github.com/solid/solid-auth-fetcher/blob/9181f67377db1c24c2e511b64bdca43be3ccb0a2/src/login/oidc/IssuerConfigFetcher.ts#L159-L164

In the end, though, given that safeGet is used in storage of secrets in localStorage, it's probably worth seeing whether this code still exists after a solution is selected for inrupt#423.

Edit: with the closure version of storage in solid-client-authn-js, it appears that safeGet and validateSchema are not used at all, so the question is how storage of the refresh token will be handled

@josephguillaume
Copy link
Author

Update:
solid-client-authn-js now uses handleIncomingredirect with {restorePreviousSession: true}
It makes sense not to update to that mechanism given that other parts of this package still use localStorage, in which the proposed changes to allow the use of CSP could reduce the attack surface for this package, even if it won't necessarily be as secure as solid-client-authn-js.
The README would then be updated to mention these security issues if the package is used in the browser rather than server-side/in a CLI tool.

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

No branches or pull requests

1 participant