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

Keep hashed things tokens in the database #424

Open
ghost opened this issue Oct 23, 2018 · 15 comments
Open

Keep hashed things tokens in the database #424

ghost opened this issue Oct 23, 2018 · 15 comments

Comments

@ghost
Copy link

ghost commented Oct 23, 2018

FEATURE REQUEST

Things tokens are stored in a plaintext format in the things db.
Anyone with the read access to the database can see them.
I think they should be stored in a hashed format.

From security perspective, I think that this is a must have feature

@ghost
Copy link
Author

ghost commented Oct 23, 2018

In order for this to happen, there are following things that need to be changed:

  1. the API should return the token upon thing creation
  2. the token should be hashed and stored in the db
  3. auth should be updated so it compares hashes
  4. there should be a way to reset the token in case it's lost of compromised

@drasko drasko added this to the 0.8.0 milestone Nov 25, 2018
@anovakovic01
Copy link
Contributor

anovakovic01 commented Jan 14, 2019

Hashing is one way encryption function that cannot be easily reversed. This is in contradiction with current API which can return plain thing key anytime. IMO we have two options:

  1. Change API and return plain key only when creating thing.
  2. Keep current API and use some other kind of encryption function that can be reversed only on client side.

I would like to hear other opinions regarding this @mainflux/contributors.

@drasko
Copy link
Contributor

drasko commented Jan 14, 2019

Find appropriate solution with Vault (or similar) - it's a storage for keys and secrets.

@anovakovic01
Copy link
Contributor

@drasko I like this idea of using Vault too.

@nmarcetic
Copy link
Collaborator

Needs more research and brainstorming, moving for 0.9

@nmarcetic nmarcetic modified the milestones: 0.8.0, 0.9.0 Jan 28, 2019
@nmarcetic nmarcetic modified the milestones: 0.9.0, 0.9.5 Jul 10, 2019
@drasko
Copy link
Contributor

drasko commented Sep 25, 2019

Apart from this - I think we should separate thing token from thingID, and really have an independant pair. This would allow then for users to provide us their Passwords (or tokens) for things.

I am saying this in light of the MQTT protocol, which indeed has an independent Username and Password - while in our case token contains thingID inside as well, as it uniquelly represents Thing.

@anovakovic01
Copy link
Contributor

  1. thing token is a separate piece of info (it has nothing to do with thing ID). Our token doesn't have thing ID info inside of it,
  2. AFAIK user can already set thing token on his own.

@drasko
Copy link
Contributor

drasko commented Sep 25, 2019

OK, then I guess it is just for us to decide do we encrypt, or we let user give us his key.

I am more for the approach where we ecrypt, especially for the reasons that this way encryption key does not have to travel.

So I would say that this issue would then consider:

  • How do we change an API to use one pre-defined (our) key for encryption in the DB
  • How do we store this key (as this is a deployments issue, we can stop at reccomendations, we do not necesary have to do an implementation with Vault. Also - this key can be maybe stored in binary).

@nmarcetic nmarcetic modified the milestones: 0.10.0, 1.0.0 Oct 21, 2019
@absmach absmach deleted a comment from Nitheshnithu121 Jan 19, 2020
@absmach absmach deleted a comment from Nitheshnithu121 Jan 19, 2020
@lpegoraro
Copy link

I did something similar to this, do we want to encrypt or just hash? Would we need to retrieve that or just validate the hash?

@drasko
Copy link
Contributor

drasko commented Jul 12, 2022

At this moment, I think we should encrypt, as I would expect that someone forgot/lost token written in the device flash and wants to retrieve it.

If we just hash, then all we can do is reset, but this might be complicated for devices in the field - they would have to be re-flashed.

But I might be wrong, and hash with reset might be the correct procedure. The rationale for this might be that if device "forgot" its token - there is something already wrong with the device's flash or firmware...

@dborovcanin @nmarcetic what would be your opinions here?

@lpegoraro
Copy link

When you guys figure that out, let me know, I will have some time probably next week and I could work on this.

@drasko
Copy link
Contributor

drasko commented Jul 13, 2022

@lpegoraro great, it would be highly appreciated. We'll let you know very soon.

@dborovcanin dborovcanin modified the milestones: 1.0.0, S5 May 15, 2024
@dborovcanin dborovcanin removed this from the S5 milestone Jun 12, 2024
@dborovcanin dborovcanin moved this to Backlog in SuperMQ Jun 12, 2024
@nyagamunene nyagamunene moved this from ⛏ Backlog to 🚧 In Progress in SuperMQ Jul 24, 2024
@arvindh123
Copy link
Contributor

Hashing the thing key is not good for present Magistrala.
Because things will be shared to other users via groups, channels , domains or individual.
So if thing key is hashed during the thing creation , then other user could able to view the thing key.

Instead we can have like thing key encryption , So other users how have access to thing can able to view the thing key.

For Initial implementation, we can have like common encryption secret for all the thing key in all domain.

Challenges:

If the encryption key changes , then we need to decrypt all the thing key with old encryption key and encrypt again with new key

@nyagamunene
Copy link
Contributor

Sound good. I can get on with the implementation.

@dborovcanin
Copy link
Collaborator

Let's postpone this one. This is an important question and the decision will affect key components of the system. @drasko We will also require your help.

@dborovcanin dborovcanin moved this from 🚧 In Progress to 🛑 Blocked in SuperMQ Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🛑 Blocked
Development

No branches or pull requests

7 participants