Skip to content

User token security #390

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

Open
benloh opened this issue Apr 14, 2025 · 10 comments
Open

User token security #390

benloh opened this issue Apr 14, 2025 · 10 comments
Labels
design Design epic EPIC Large issue that encompasses many sub-issues. template

Comments

@benloh
Copy link
Collaborator

benloh commented Apr 14, 2025

Joshua writes:

I am wondering about the security implications and next-steps on tokens. Short-term, assuming DO hosting and tokens, we should likely disable shared logins, and that way only a teacher can give logins to their server. It's still pretty fast and loose since anyone who knows the code and / or admin trick can do it, but it'd be slightly safer?

Longer-term, I assume we are moving to a hidden system that no-longer uses the url? We don't need to change it now, but just wondering your thoughts as we move towards something that is hopefully more secure? I also wasn't clear on how DO v non-DO will work? One quick-and-dirty option short-term would be to use the password in the sessame file for admin so that it is local, hidden, and customizable? Just thinking aloud.

@benloh benloh added design Design epic EPIC Large issue that encompasses many sub-issues. template labels Apr 14, 2025
@benloh
Copy link
Collaborator Author

benloh commented Apr 15, 2025

There are multiple layers of issues here...

First, I think given the state of the budget and the work that still needs to be done to get templates/preferences working and Turbo360 up and running, a full user authentication system is not likely to happen with this round of funding. There is significant design work still needed not only from a system architecture perspective, but also simply working out the use models.

Second, the current DO approach uses nc-multiplex for the "sesame" file and user token generation. This will NOT be present in Turbo360. So if we want to use Turbo360, we needed to add the User Token management tools to to the Advanced UI panel, hence #389.

So there are two key problems:

  1. How do we create appropriately shareable/nonshareable tokens for teacher/classroom combos? How do we make sure each teacher/classroom/student has enough security?
  2. Do we need to add additional security beyond ?admin=true?

How does this work currently?

User tokens by default are generated via

  • class -- any arbitrary alphanumeric string up to 12 characters
  • project -- any arbitrary alphanumeric string up to 12 characters -- this is NOT tied to the dataset name!!!
  • dataset -- the filename of the database as defined in the NC_CONFIG value, defined in the netcreate-config.js file.

Shareable user tokens simplifies this, removing the dataset:

  • class -- any arbitrary alphanumeric string up to 12 characters
  • project -- any arbitrary alphanumeric string up to 12 characters -- this is NOT tied to the dataset name!!!

How should it be implemented?

assuming DO hosting and tokens, we should likely disable shared logins, and that way only a teacher can give logins to their server. It's still pretty fast and loose since anyone who knows the code and / or admin trick can do it, but it'd be slightly safer?

This is a little complicated, especially when bringing Turbo360 into the mix. I think at this point it's probably more useful to think through how you would like to use the system and then we'll rework it.

Given that the token system was primarily intended to support a way to identify authors rather than providing any true security, we had assumed given your request for shareable tokens that you weren't terribly concerned about student-to-student access.

Digital Ocean

Assuming Digital Ocean is using nc-multiplex...

  • If you had 20 different teachers who each needed to have their own "Harry Potter" curriculum instance, you would create and spin up 20 instances.
  • You would probably want to have 20 instance names, e.g. HarryPotterDanish, HarryPotterLoh
  • If each teacher had multiple classrooms, then the instance names might be HarryPotterDanishPer1, HarryPotterDanishPer2
  • A teacher with admin access (e.g. ?admin=true) can create shareable or non-shareable tokens.
  • If a teacher were worried about students hacking into each other's accounts, they would create non-shareable tokens. But this means that students need to have multiple tokens, one for each graph -- e.g. the token for HarryPotterDanishPer1 would be different from LordOfTheFliesDanishPer1.

With the current implementation, any teacher can get into the admin interface using ?admin=true. e.g. http://159.203.177.172/graph/testing/#/?admin=true

As long as each graph uses a unique name, non-shareable tokens should work after #389 is merged. (The shareable tokens hack had actually made ALL tokens shareable, so we had to add non-shared restrictions back in).

Turbo360

Things are a little different with Turbo360.

  • As with DO, each teacher will have their own graph instance for each classroom, e.g. HarryPotterDanishPer1, HarryPotterDanishPer2.
  • Each teacher instance can administer their shareable/nonshareable tokens using the ?admin=true admin interface, e.g. https://bennov26-sebfel.turbo360-staging.com/#/edit/A-B-Y2K?admin=true

But here's where things do not quite work:

  • The dataset salt used on Turbo360 is currently tied to the base template. e.g. when you deploy the "NetCreate (Blank)" template in Turbo360, the dataset that is defined in netcreate-config.json will be the name of the dataset used for the token. e.g. the current NetCreate (Blank) template is using greek-demo. If you go to the console and type NC_CONFIG you can see the current value.
  • The problem is that with T360, currently that value can't be easily changed. This is because when a project template is created, the initial deployment grabs the current values as defined in the netcreate-config.json file. But that same file is used for all other clones. So each teacher instance would use the same dataset salt. So even if you use a non-shareable token, the tokens that are generated will all use the same greek-demo salt.
  • You can get around this by simply specifying different class or project parameters. In fact you probably want to do something like that anyway, e.g.
    • CLASS: PER1 + PROJ: POTTER => PER1-POTTER-GMW
    • CLASS: PER2 + PROJ: POTTER => PER2-POTTER-NKE
  • But, because the dataset salt is shared, the HarryPotterDanishPer1 instance and the HarryPotterLohPer1 would end up with the same tokens if you used a generic PER1 -- the token will be the same for Danish and Loh's classes because they are using the greek-demo dataset setting in the NetCreate (Blank) template:
    • CLASS: PER1 + PROJ: POTTER => PER1-POTTER-GMW
    • CLASS: PER2 + PROJ: POTTER => PER2-POTTER-NKE
  • This could be a feature or a bug depending on what you need.
  • You can get around this by specifying the name, e.g.
    • CLASS: DANISHPER1 + PROJ: POTTER => DANISHPER1-POTTER-GMW
    • CLASS: DANISHPER2 + PROJ: POTTER => DANISHPER2-POTTER-NKE
    • CLASS: LOHPER1 + PROJ: POTTER => LOHPER1-POTTER-7VX
    • CLASS: LOHPER2 + PROJ: POTTER => LOHPER2-POTTER-QW6

The problem here is that token security is minimal. Since there's no unique dataset salt on Turbo360, all tokens using the same parameters are the same...e.g. if a bad actor research group spun up their own instances, they could easily recreate your user tokens by simply using matching class and project ids.

Ideally we would at least introduce a new dataset value or some other unique value that cannot be guessed via the admin UI. It's not strictly necessary, but it would add a layer of security.

Locking Admin

The second big issue is what kind of security to provide for access to the admin features. If the use of ?admin=true is inadequate, what kind of security do you need?

Since Turbo360 does not use nc-multiplex, we do not have a SESAME password equivalent.

  • Does each teacher need their own password?
  • Does each instance need their own password?
  • How much security is enough to prevent student abuse?

One possible approach:

  1. Initially the Advanced Panel will show only "Export Data"
  2. If you add ?admin=true, at the top of the Advanced Panel you have a "Password" field. If you enter the right password, then the other admin panels are available: Template, Import Data, User Tokens.

I don't think we want to "burn" in a password because that would be difficult to change, and becomes a security issue for any other groups using the system.

But if we need to have a changeable password, we would need to put it somewhere where one can change it. We could potentially define the password in the project template. So then depending on your security needs, you could define a password that is shared across all teachers in the study, or each graph can have its own password.

We probably want some kind of override to reset the project template password in case the project template gets screwed up...the equivalent of the "danishpowers" password to provide some kind of fallback?

@jdanish Lots to think through...any thoughts?

@jdanish
Copy link

jdanish commented Apr 16, 2025

Thanks @benloh. Some important clarifications:

  1. Short-term, we are not worried about kids logging in as each other as the teacher can handle that socially.
  2. However, if they can see the teacher use the admin url they, they may try and duplicate it and cause trouble. That we have seen.
  3. Adding seemingly arbitrary things to the url might scare the teacher where a password field of some sort would not
  4. I think you are right that it is OK to kill the shared-across-all-networks tokens
  5. Ideally, it would be nice to have the tokens work across the same teacher, perhaps by using a code word or last name as part of their hash?
  6. It would be nice if a "bad actor" couldn't guess the login tokens simply by knowing the url
  7. I realize we won't be truly secure until we revamp the whole thing, but am hoping that not having secure data and being a bit more cautious will do some of the work
  8. If all goes well with turbo360, I think we'd use DO purely for local testing so we don't want to break it yet, but it is also OK if it is not quite as convenient as it once was
  9. Agreed, however we setup the teacher password we'll want some way of letting them re-set. In the immediate future, that could be contacting us since I don't anticipate all 20 losing their passwords at once, but even if they do I can update them as administrator assuming it is not too hard?

Does that help limit / narrow the scope temporarily?

@benloh
Copy link
Collaborator Author

benloh commented Apr 16, 2025

Thanks for the clarification.

  1. Adding seemingly arbitrary things to the url might scare the teacher where a password field of some sort would not

To avoid the ?admin=true, we'll just always show a "Password" field in the Advanced Panel. If we want to make even more secret, I suppose we could just have an input box with no label, but that might be too confusing.

  1. Ideally, it would be nice to have the tokens work across the same teacher, perhaps by using a code word or last name as part of their hash?

Do you mean supporting something like having a name in the token like DANISHPER1-POTTER-GMW? Or do you mean the hidden salt would use a teacher-specific string, e.g. if the salt is danish we could use the same class=PER1, proj=POTTER parameters to generate a token like PER1-POTTER-GMW, but if the salt is loh, the class=PER1, proj=POTTER parameters will generate a different token like PER1-POTTER-GKE?

Right now I'm thinking that salt is defined in the template file. e.g. there would be two new values in templates:

salt = "danish"
adminPassword = "danishpowers"

The salt value is used to generate user tokens and "guarantee" that a token from one graph is different from a token from another graph.

The adminPassword value is the password you need to to use when opening the Advanced Panel. You need to enter the password in order to be able to view the template panel, the import panel, and user tokens.

We can also introduce a superuser password like "danishsuperpowersactivate" or something to allow you to edit the Advanced Panel in case for some reason you can't access the template. This is primarily for use with Turbo360 where you won't have direct access to template file but need to change it through importing the template. But this would have to burned into the code (not ideal), or we would have to ask Vertex to allow injecting something like a SESAME file into the file system -- I'm not sure it can work mostly because Turbo360 does not really use the file system once the app is deployed. Alternatively we can also use the ?admin=true trick instead -- it's simpler, and won't be visible to teachers because they'll never see it in everyday use?

If the salt is defined in the template, you can then use the same salt to emulate shareable tokens. e.g. if you used salt = "spring2025" for all of your graphs, and then use the same class ID and proj ID values (e.g. class=PER1, proj=SPRING), then the tokens that are generated for your Harry Potter graph and Lord of the Flies graphs will be the same

Similarly you can set the adminPassword for each teacher separately if you're worried about cross-teacher contamination.

  1. If all goes well with turbo360, I think we'd use DO purely for local testing so we don't want to break it yet, but it is also OK if it is not quite as convenient as it once was

I think we always want to make sure that DO or some other non-Turbo solution works.

Other Misc Comments

  • I don't think we can easily just re-use the login token box as a teacher login, mostly because we would then have to manage the teacher logins somehow.
  • We can hide the "Shareable" button and functionality from the UI. But you can still use ncMakeSharedToken in the console to generate a shared token if you want.

@jdanish
Copy link

jdanish commented Apr 16, 2025

Thanks!

  1. I think making a new login box called admin password inside the advanced tab makes sense and is fine. So long as kids don't know what is being written in there, it feels secure enough.
  2. I wasn't 100% sure what I meant and didn't know the word salt but now that I do ... yes, having the salt and the adminPassword both in the template sounds ideal. That way if the teacher uses the same salt, they can have the same login, but either way it is quite unlikely Kalani and Joshua both simultaneously use the same salt.
  3. I'd rather not bake in the super user password, so it might be good to ask Vertex if there is an alternative for us to have access or be able to read their salt since we'll have access to the logs etc? We will want to download their template, after all, to see what they were using, and if we can do that we can check their password and tell them what it was, no?
  4. I think shared passwords just complicate security, so linking them to the salt feels like idea.
  5. Agreed re: DO.

Did I miss anything?

@benloh
Copy link
Collaborator Author

benloh commented Apr 16, 2025

  1. I'd rather not bake in the super user password, so it might be good to ask Vertex if there is an alternative for us to have access or be able to read their salt since we'll have access to the logs etc? We will want to download their template, after all, to see what they were using, and if we can do that we can check their password and tell them what it was, no?

So if we repurposed the ?admin=true url approach to provide the equivalent of a super user password is that not secure enough? We could even make it a little less obvious, e.g. https://bennov26-sebfel.turbo360-staging.com/#/edit/A-B-Y2K?danishpowers=activate

So teachers would never really ever see ?admin=true. And only researchers would use the url approach. But obviously it's not very private if you're projecting on the screen.

@benloh
Copy link
Collaborator Author

benloh commented Apr 16, 2025

Oh, actually on revisiting this, Turbo360's portal allows you to download the template file directly via the Site > Storage > *.template.toml file.

So theoretically once we define the adminPassword in the template, you can always peek at the template.

And then on the DO side, you could use SSH to grab the template file if you don't know the password.

@jdanish
Copy link

jdanish commented Apr 16, 2025

Re: the admin=true as a variation of danishpowers is fine. The part that made me nervous was that that means anyone who looks at the git repo will know how to get in. That's where the sesame file in nc-multiplex is nice because in theory we can change it on each server even if we mostly don't bother.

Re: checking the template via storage or ssh great.

@benloh
Copy link
Collaborator Author

benloh commented Apr 16, 2025

Cool. Let's give this a try and see if it works. Thanks for all the feedback.

@jdanish
Copy link

jdanish commented Apr 16, 2025 via email

@benloh
Copy link
Collaborator Author

benloh commented Apr 24, 2025

To Do

  • Add a password field to Advanced (hides values)
  • Remove ?admin=true
  • Move "Shareable" checkmark to separate area -- leave the functionality in case we need it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design epic EPIC Large issue that encompasses many sub-issues. template
Projects
None yet
Development

No branches or pull requests

2 participants