motioneye-project / motioneye

A web frontend for the motion daemon.
GNU General Public License v3.0
3.94k stars 650 forks source link

Revert "store surveillance user password as SHA1" #2944

Closed zagrim closed 6 months ago

zagrim commented 6 months ago

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.

MichaIng commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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.

zagrim commented 6 months ago

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 commented 6 months ago

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.