motioneye-project / motioneye

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

Motioneye issues warning "kernel: bad security option: ntlm" when mounting samba share #3013

Open chri-kai-in opened 5 months ago

chri-kai-in commented 5 months ago

Versions: motionEye Version 0.43.0 Motion Version 4.5.1 Betriebssystemversion | Linux 6.1.21-v8+

When mounting a Samba share, motioneye issues a warning referring to security option "ntlm" is considered bad security option.

Jun 18 17:02:18 Garage kernel: CIFS: Attempting to mount \\xxx.xxx.xxx.xxx\nvrdata
Jun 18 17:02:18 Garage kernel: bad security option: ntlm
Jun 18 17:02:18 Garage kernel: CIFS: VFS: bad security option: ntlm
Jun 18 17:02:18 Garage meyectl[1935]: Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)

To avoid this i propose to remove ntlm from the list of security options "sec_types" in file "smbctl.py" in def _mount :

def _mount(server, share, smb_ver, username, password):

    mount_point = make_mount_point(server, share, username)

    logging.debug('making sure mount point "%s" exists' % mount_point)

    if not os.path.exists(mount_point):
        os.makedirs(mount_point)

    if username:
        opts = f'username={username},password={password}'
        # sec_types = [None, 'ntlm', 'ntlmv2', 'ntlmv2i', 'ntlmsspi', 'none']
        sec_types = [None, 'ntlmv2', 'ntlmv2i', 'ntlmsspi', 'none']

    else:
        opts = 'guest'
        # sec_types = [None, 'none', 'ntlm', 'ntlmv2', 'ntlmv2i', 'ntlmsspi']
        sec_types = [None, 'none', 'ntlmv2', 'ntlmv2i', 'ntlmsspi']

    opts += ',vers=%s' % smb_ver

    for sec in sec_types:
        if sec:
            actual_opts = opts + ',sec=' + sec

        else:
            actual_opts = opts

        try:
            logging.debug(
                f'mounting "//{server}/{share}" at "{mount_point}" (sec={sec})'
            )
            subprocess.run(
                ['mount.cifs', f'//{server}/{share}', mount_point, '-o', actual_opts],
                check=True,
            )
            break
[...]

With this applied the error message in syslog is removed.

MichaIng commented 5 months ago

Hmm, whether ntlm is a "bad" option, depends on the kernel version. Indeed it has been removed somewhat recently.

I am wondering whether we need to test through and in case add such security options at all. We do not define it in our distribution (other project) and never received a bug report/support request where mounting CIFS failed because no sec option was defined.

But in your case, obviously not defining sec at all seems to have failed, otherwise sec=ntlm would not have been attempted. If you find time, could you enable debug logging and paste the log again when trying to mount the share?

What probably makes sense is switching the order in which those options are tested. None (not defining it at all, using defaults) makes sense as first attempt. none is the only reasonable option when not defining a username and password, since these are all about how to transfer the password to the server. When user/pass have been defined, it would however probably make sense to test the newest/most secure options first, and the weaker ones later, basically inverting the 4 middle options in the list, and adding ntlmssp (without i) as well (not sure why it is missing). Since ntlmssp is the default since Linux 3.8, most servers will support it, hence ntlm would only be tested if really all newer/better options fail.

chri-kai-in commented 5 months ago

Hello MichaIng,

i agree it might only be a smaller issue and only valid for newer release. Access to the samba folder worked even with the error fine. I am accessing my samba share with username and password given, so this is the path to be checked in the code.

Excerpt from system log file with log_level debug and original smbctl.py file:

[...]
Jun 19 22:36:44 Garage meyectl[467]:    DEBUG: motion detection is disabled for camera with id <built-in function id>
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: checking SMB mounts...
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: listing smb mounts...
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: found smb mount "//xxx.xxx.xxx.yyy/nvrdata/Aussen" at "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login"
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: found smb mount "//xxx.xxx.xxx.yyy/nvrdata/Aussen" at "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login"
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: making sure mount point "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login" exists
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: mounting "//xxx.xxx.xxx.yyy/nvrdata/Aussen" at "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login" (sec=None)
Jun 19 22:36:54 Garage kernel: CIFS: Attempting to mount \\xxx.xxx.xxx.yyy\nvrdata
Jun 19 22:36:54 Garage meyectl[1615]: mount error(16): Device or resource busy
Jun 19 22:36:54 Garage meyectl[1615]: Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: mounting "//xxx.xxx.xxx.yyy/nvrdata/Aussen" at "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login" (sec=ntlm)
Jun 19 22:36:54 Garage meyectl[1617]: mount error(22): Invalid argument
Jun 19 22:36:54 Garage meyectl[1617]: Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)
Jun 19 22:36:54 Garage kernel: bad security option: ntlm
Jun 19 22:36:54 Garage kernel: CIFS: VFS: bad security option: ntlm
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: mounting "//xxx.xxx.xxx.yyy/nvrdata/Aussen" at "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login" (sec=ntlmv2)
Jun 19 22:36:54 Garage kernel: CIFS: Attempting to mount \\xxx.xxx.xxx.yyy\nvrdata
Jun 19 22:36:54 Garage meyectl[1619]: mount error(16): Device or resource busy
Jun 19 22:36:54 Garage meyectl[1619]: Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg)
Jun 19 22:36:54 Garage meyectl[467]:    DEBUG: mounting "//xxx.xxx.xxx.yyy/nvrdata/Aussen" at "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login" (sec=ntlmv2i)
Jun 19 22:36:54 Garage kernel: CIFS: Attempting to mount \\xxx.xxx.xxx.yyy\nvrdata
Jun 19 22:36:55 Garage meyectl[467]:    DEBUG: directory at "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_folder_login" is writable
Jun 19 22:36:55 Garage meyectl[467]:    DEBUG: unmounting "//xxx.xxx.xxx.yyy/nvrdata/aussen" from "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_aussen"
Jun 19 22:36:55 Garage meyectl[1625]: umount: /media/motioneye_xxx_xxx_xxx_yyy_nvrdata_aussen: no mount point specified.
Jun 19 22:36:55 Garage meyectl[467]:    ERROR: failed to unmount smb share "//xxx.xxx.xxx.yyy/nvrdata/aussen" from "/media/motioneye_xxx_xxx_xxx_yyy_nvrdata_aussen"
Jun 19 22:36:55 Garage meyectl[467]:    DEBUG: stopping motion
[...]

If you need more lines before and after or without anonimyzed entries i can provide complete log file via mail.

I agree on changing the sequence of the security options to start with the most recent/most secure option first to then re-try with less secure options with every following connect for both. This sounds reasonable and robust to me, as it will ensure more secure communication in case of samba server supports it and still keeps connectivitiy with a certain level of security for the less recent server installations.

MichaIng commented 5 months ago

mount error(16): Device or resource busy

Hmm, with valid sec= options or None, the issue seems to be a different one. Probably the share is mounted already when motionEye tries to do so, but the 4th attempt seems to succeed, probably the case when mount options match to what has been mounted already. Also weird is that it finally tries but fails to unmount the share. This could also be the case when a mount exists already, or of course when it was not successfully mounted after the attempts.

Could you check whether mount on the host system shows shows that share mounted, probably somewhere else, and probably even after you unmount it in motionEye?

For now to me it looks like we could remove that options loop let the system negotiate it or choose defaults. And instead we might need to check whether the mount exists already, in case unmount it, or inform the user when the same share has been mounted elsewhere.