Cvmcosta / ltijs

Turn your application into a fully integratable LTI 1.3 tool provider.
https://cvmcosta.github.io/ltijs/
Apache License 2.0
299 stars 67 forks source link

Cookieless mode #76

Closed ChiefGnome closed 3 years ago

ChiefGnome commented 3 years ago

While testing, I found out that a lot of people block third-party cookies, and also Chrome plans to get rid of them.

I suggest a switch to run the server in a mode where it doesn't depend on cookies or sessionStorage (as this is also blocked). Instead, it should just use query parameters or similar safe ways to keep the state. As this already works in 'devMode', there should be a simple solution.

Currently, the only way to also support users that block third party cookies is to run the server in devMode with an ugly warning.

Cvmcosta commented 3 years ago

Hello! There is already a mode in place that handles authentication using only the ltik token. Just use ltiaas: true in the options and it should work. The only cookie still required is a state cookie set at the start of the login flow and deleted shortly after, this cookie is required by the LTI 1.3 spec to prevent XSRF vulnerabilities, however, i haven't experienced issued while setting it.

You can find more info here: Ltiaas Mode

ChiefGnome commented 3 years ago

Thank you for the quick answer. Unfortunately, this doesn't solve the problem. I enabled "Block third-party cookies" in Chrome with a hosted canvas instance, and I get the infamous {"status":401,"error":"Unauthorized","details":{"description":"Error validating ltik or IdToken","message":"ISS_CLAIM_DOES_NOT_MATCH"}} error. In devMode it still works.

Cvmcosta commented 3 years ago

I see, i can't really remove this cookie since i need it to be compliant with the IMS Specification and deal with vulnerabilities. What i can do is add the devMode warning to the set of strings controlled by the silent deployment option, which would suppress the warning.

ChiefGnome commented 3 years ago

Ok, I see.

Would it be possible to create a handler like onCookieMissing where I can give instructions, instead of the ISS_CLAIM_DOES_NOT_MATCH error? Or even better, where I can have a link to open the page as a separate tab, if cookies are blocked?

ChiefGnome commented 3 years ago

Out of curiosity, I quickly checked the IMS LTI specs and security specs for the word "cookie" and I could not find one instance, where it is mentioned. So I guess there should be another way to validate the request, that doesn't rely on cookies.

I am just worried, that all major browsers will block them soon.

Cvmcosta commented 3 years ago

The IMS Security Framework mentions the state parameter from the OpenID Connect specification as a mitigation for CSRF attacks. The state must be an opaque string used to ensure that request and response originated in the same browser.

The state parameter is a string so you can encode any other information in it. You send a random value when starting an authentication request and validate the received value when processing the response. You store something on the client application side (in cookies, session, or localstorage) that allows you to perform the validation. If you receive a response with a state that doesn't match, you can infer that you may be the target of an attack because this is either a response for an unsolicited request or someone trying to forge the response.

In the LTI context, the only way i can be sure the parameter will be returned is by setting a cookie, or at least i cannot think of another way at the moment. Even the IMS official reference PHP implementation uses a cookie to store the state parameter:

// Generate State.
// Set cookie (short lived)
$state = str_replace('.', '_', uniqid('state-', true));
$this->cookie->set_cookie("lti1p3_$state", $state, 60);

However, i am open to suggestions since this is an issue I've been struggling with for quite some time.

Cvmcosta commented 3 years ago

Would it be possible to create a handler like onCookieMissing where I can give instructions, instead of the ISS_CLAIM_DOES_NOT_MATCH error? Or even better, where I can have a link to open the page as a separate tab, if cookies are blocked?

This is a great idea, i'll try to look into it during this weekend, thanks!

ChiefGnome commented 3 years ago

The IMS Security Framework mentions the state parameter from the OpenID Connect specification as a mitigation for CSRF attacks. The state must be an opaque string used to ensure that request and response originated in the same browser. ... However, i am open to suggestions since this is an issue I've been struggling with for quite some time.

Ok, I understand. This may be hard, as also localStorage/sessionStorage is blocked, if you block third party cookies. I am not security expert, so I have no idea if there is another way to keep the state around. Thank you very much for looking into the work around with the "CookieMissing" handler!

Cvmcosta commented 3 years ago

Hello, sorry it took a bit longer that i expected, i didn't have too much time this week.

I decided that it was unnecessary to add a specific handler for the missing cookie. What i did was remove invalidtoken route and make sure that whenever there's an error the onInvalidToken handler is called right way. You can then retrieve the res.locals.err variable and use it to identify the issue and deal with it accordingly (display a more descriptive error page):

lti.onInvalidToken(async (req, res, next) => { 
   if (res.locals.err.details.message === 'ISS_CLAIM_DOES_NOT_MATCH ') { // Testing error code
     // Deal with missing cookie
   }
})

Please let me know if this solution works for you.

ChiefGnome commented 3 years ago

This is a great idea! Thank you! I still think it would be helpful, to have two messages. One for "doesn't match" and one for "cookie completely missing" as those have different causes. With this current system this wouldn't require any additional handlers.

Cvmcosta commented 3 years ago

Hello! Just released a new version 5.6.3 changing the error message to MISSING_VALIDATION_COOKIE when the cookie is not present.

Please let me know if it solves your problem.

ChiefGnome commented 3 years ago

Thank you very much for your work!

It still doesn't solve the initial problem, as it still doesn't work without cookies, but at least I can now present a nice and specific error message.

junglebarry commented 3 years ago

I think this is a known issue with the LTI spec at present, and the spec will need to change to accommodate third-party-cookie-blocks.

That is to say: third-party-cookie-blockers are a shared problem in the LTI community, and not something this library can (or should) fix alone.

(Great library, by the way!)

Cvmcosta commented 3 years ago

Hello @junglebarry ! You are right. I am an IMS Member and during the LTI roundtable meetings there are a lot of discussions regarding possible fixes to this issue, so far nothing proved to be a good substitute to the state parameter but options are being considered (device fingerprinting for example).

We are definitely aware of the issue and as soon as there's a solid solution or workaround i will make sure to bring to the library. I will be closing this issue for now.

Thanks a lot!