#CSP and other security fixes

1 messages · Page 1 of 1 (latest)

thorn mural
#

No objection, just ping @muted urchin and @vital pivot FYI

vital pivot
#

Hey, that's awesome. Please let me know if I can assist. I'm in appsec but have been to lazy to review it yet. I have some sast scan results I've performed, let me know if you want to see them

thorn mural
#

No I don't think so

vital pivot
#

Did you get any payouts?

manic dome
#

Noice

manic dome
#

What specifically looks incorrect?

manic dome
#

Yeah, I have that in my backlog to change. We should detect if oauth is enabled separately from generating the authorize url.

#

Not handling state is something I am aware of 😛. Didn't get around to persisting it server-side.

#

Is it supposed to be handled client-side?

muted urchin
#

I'm pretty sure that state is supposed to be checked serverside, right?

manic dome
#

From what I understand the state value needs to be stored/linked to the session that starts the requests and validated during the callback.

#

What do you mean by "imagine we would have pure javascript frontend without a server"?

#

We're using auth code instead of implicit so access/refresh tokens are never directly exposed in the web and they're not needed client-side for anything anyways.

#

If application desire, state can be stored in a backend (ex:- server). This can be considered more secure (compared to SPA) given that no one can intercept/obtain the value other than from request itself. Once redirect occur, backend can validate the response parameters. Moreover, it can be used to correlate client session.

#

We are a single page app, but the oauth client doesn't run in that context. We're using auth code anyways instead of implicit.

#

The auth code is submitted to the server and that is where the validation needs to happen.

#

That defeats the point of the state param though. At that point we don't need to do anything anyways.

muted urchin
#

We already have redis, serverside sessions shouldn't be a problem

manic dome
#

Basically, what we have right now is:

  • Client generates an authorize url
  • Client receives an auth code
  • Client posts auth code to the server
  • At this point we need to know if the auth code came from the client in step one or somebody else
#

The server needs to have a session for the original request and store the state linked to that session and then when it receives the auth code it needs to validate the same session originated the request.

#

Yes, just pointing out that if the server doesn't store and validate the state param it doesn't prevent against CSRF.

#

For the web implementation, we can also store state in an HttpOnly cookie.

#

Correct.

#

A second redis instance?

#

A second adapter?

#

Idk the details. We should be able to use the same redis server and use a different namespace for storing the details. Specifically for the web though, it might be easier to just set a http only cookie with the state. What do you think?

#

In local storage not in a cookie. A cookie is only accessible by the server anyways.

#

In a cookie not local storage*

#

Even if you stored the state in redis, you still need to link that state to a http request via a session (cookie)

#

I am not sure if we can use that same strategy for the mobile app. Not 100% sure if we set a cookie if we will get it back in the authorize callback.

#

It uses some auth_* library that opens the link and then waits for the callback. Not 100% how it works.

#

If you have the server changes running locally you can test it pretty easily though.

#

And see if it works or throws an invalid state error. If it works, we can assume the cookie is getting set/read correctly I think. If not, we'll have to fix mobile before releasing the changes.

#

@thorn mural did the oauth changes on mobile.

thorn mural
manic dome
#

You didn't do the oauth changes on mobile?

thorn mural
#

oh I thought that was a question

#

ah yes I implemented oAuth on mobile

#

the webview part is handled by the package we used to handle oAuth and redirection

#

The package is FlutterWebAuth

muted urchin
#

Can you link the code?

#

Some context - the api request to get oauth configuration also returns the boolean to tell the UI if password login is enabled or not. Unfortunately, in the case the OAuth configuration is invalid or the auth server is down, this results in an error and the web is never informed if password login is enabled or not, resulting in the login form being hidden, even when password login is might be enabled. This is a UI only bug, which is why password login on the mobile app still works, and requests directly to the api work as well.

#

in that comment, @manic dome mentions wanting to revisit this, but I don't know if he ever actually did

manic dome
#

Not yet, but we have feature flags now so we can use that instead.

muted urchin
#

Good thinking

manic dome
#

This would probably be good to do in septate PRs too. One for migrating to feature flags, one to remove the auth config endpoint and just redirect to the API and let it forward to the authorize endpoint, and one to implement state.

#

It only does that if the config endpoint failed.

#

And this is only client-side. Server side it is always enforced regardless.

#

If you disable it you can never login with a password regardless what the UI is showing

#

If you disable password and misconfigured oauth and are essentially locked out and have no active sessions open, you can use an admin CLI command to re-enable password login

#

What is the security issue?

muted urchin
#

It's not a security issue

#

This is clientside only

#

At worst it will confuse a user a little bit

#

Anyways, the way forward is to move this flag (whether to display the password form) out of the oauth config endpoint and into a feature flag

#

Just on the client UI

#

The API will still reject the login no matter what

manic dome
#

The immich web app

#

You understand client vs server terminology in this context right?

#

Rest apis?

#

The web app and mobile app submit a http request to login with a username and password, that request independently checks is password login is enabled or disabled.

#

By default you need to route external traffic to port 2283 on the immich proxy.

#

How it is exposed is irrelevant though.

#

Not really

#

Client UI = frontend UI opposed to the server running at home

#

The website loaded on the remote computer is the "client"

#

The rendering of the login form happens on the remote machine just to collect the login credentials.

#

Define "will enable password"

#

That is incorrect

#

We don't know if it is misconfigured or not per se

#

We just know we don't know if oauth is enabled or if password login is disabled

#

We don't know what to do so we fallback to showing the login screen

#

You can have login enabled for oauth and password login at the same time. If something is wrong with oauth you still should be able to login with a password if it is enabled.

#

What is your question?

#

That is false. Password login is not enabled when that endpoint fails.

#

We just show the login form because it might be enabled.

#

Showing the form is different from letting a user successfully login with a username and password

#

Are you running the system locally?

#

Can you comment out that line and always show the form?

#

Then disable password login and try to login?

muted urchin
#

Everything in web/ only runs in the browser, and doesn't really have any say over security decisions. It just collects information and then calls the API (server/), which makes decisions

#

Indeed, but that doesn't actually enable anything serverside

#

The fallback is just because the request that says whether or not it should show the password form is failing

manic dome
#

I'm kind of tired of repeating myself, so I'm not going to explain why it is like that for the 4th time.

manic dome
#

I don't follow

muted urchin
#

The login endpoint will get the config, yes...?

#

And then do the rest of the login logic

#

Hold up, what are you doing exactly? /auth/login has nothing to do with oauth, that's the normal login endpoint

manic dome
#

The login page has some code that runs server side which loads the config (auth/login/page.server.ts) it passes the initial config to the login form.

muted urchin
#

Have you tried reading the server code?

#

wait, are you doing like curl http://immich:2283/auth/login?

#

That's not hitting the immich-server api

#

but the server of the svelte web project

#

That's an important distinction you need to be aware of

#

Everything under /api is immich-server

#

Yes, it gets the config when loading the page

#

It works, right? Or is something going wrong?

#

As a bit of general advice: When getting familiar with a new codebase, you will run into many things that seem like "weird behaviour" or bugs, as they don't match how you think things should be working. Most of the time however, that's because "how you think things should be working" is not correct, as you're not fully familiar with the codebase yet. That's helpful to keep in mind.