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

Revert "store surveillance user password as SHA1" #2944

Merged

Conversation

zagrim
Copy link
Collaborator

@zagrim zagrim commented Mar 17, 2024

Motion requires having plaintext password for stream_authentication in camera-*.conf files, and keeping the password hashed internally in ME isn't then possible.

If someone has a better idea that allows storing passwords hashed, I'm ready to scrap this PR, but it doesn't seem possible due to lack of support from Motion.

This fixes #2813 . See the discussion there for more context.

Motion requires having plaintext password for stream_authentication in
camera-*.conf files, and keeping the password hashed internally in ME isn't
then possible.
This reverts commit 14156d2.
@zagrim zagrim requested review from MichaIng and jmichault March 17, 2024 09:36
@MichaIng
Copy link
Member

MichaIng commented Mar 18, 2024

Approved as per my post in the linked issue: If I am not mistaken, having the surveillance user password hashed, even if we did implement support for and store in MD5 digest format, supported by motion, motionEye itself requires it in plain text to authentication against motion, to process and show streams through its PR. I remember I indeed had issues using the surveillance user at all, also for motionEye UI, after assigning a password. That would explain it.

So for now: Let's restore the previous working state. And we can think about enhancing security in another PR, when we find time. At least assuring 600 or 640 mode on all config files makes sense, so they are not world-readable, if this is not already the case.

@zagrim
Copy link
Collaborator Author

zagrim commented Mar 18, 2024

motionEye itself requires it in plain text to authentication against motion

Well, ME could store the password hashed in main config (motion.conf) but it wouldn't make much sense when the same password needs to be in the camera configs in clear-text in order for the user to be able to use that password (and not the hash of the password) when they authenticate (against Motion) when opening the streaming URL. Motion has no support for hashed credentials in config file.

@MichaIng
Copy link
Member

MichaIng commented Mar 18, 2024

The other way round, isn't it? It can be stored in hashed way in the camera/motion configs, but motionEye needs to access it in plain text somewhere, so it can authenticate at the motion instances to access/process the steams, to show them in the motionEye UI. Or which config/s is/are the ones motion compares passed passwords against?

However, the result is the same. Let's get this merged and fixed the way it originally was. A better solution or other security measures can be applied in a follow up PR, if someone has a good idea and time to implement.

@MichaIng MichaIng changed the base branch from main to dev March 18, 2024 20:31
@MichaIng MichaIng merged commit 0b7a6b8 into motioneye-project:dev Mar 18, 2024
9 checks passed
@zagrim
Copy link
Collaborator Author

zagrim commented Mar 19, 2024

The other way round, isn't it? It can be stored in hashed way in the camera/motion configs, but motionEye needs to access it in plain text somewhere, so it can authenticate at the motion instances to access/process the steams, to show them in the motionEye UI. Or which config/s is/are the ones motion compares passed passwords against?

Well, I didn't even think of how ME integrates with Motion, but we didn't notice any issues getting current camera frame to UI etc, did we? After some more looking at the code I'm not really understanding how hashing of normal_password didn't break also that, since those same credentials that are put in camera config (which I think is the only place Motion can get the credentials) to stream_authentication seem to at least be used by the Mjpgclient to get current picture... But since I'm not at all clear on how that part works, I'm assuming that it might also depend on the camera type, or something?

Anyway, as long as Motion can't understand hashed password in camera-*.conf, that password needs to be stored in clear-text. For the rest we can do whatever we want, I suppose.

@MichaIng
Copy link
Member

I'll have a closer look at this when I find time. MJPEG cameras bypass motion, but else they are processed by motion. But probably ME can somehow bypass authentication as it starts the motion processes by itself.

This was referenced May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Python3 (Dev): User Authentication not working
2 participants