socialwifi / RouterOS-api

Python API to RouterBoard devices produced by MikroTik.
MIT License
255 stars 98 forks source link

Error Login API V 6.43 #29

Closed oortega closed 6 years ago

oortega commented 6 years ago

Hello

Mikrotik released an update V6.43 then the login by api no longer works. The details are shown here https://wiki.mikrotik.com/wiki/Manual:API#Initial_login

Can you please give me an idea on how to solve this?

Thank you very much for your project.

jgoclawski commented 6 years ago

Hi,

thanks for letting us know. Version 6.43 is still a release candidate so we haven't used it yet and haven't investigated how to support it. The method that handles logging in is: https://github.com/socialwifi/RouterOS-api/blob/8078b9e3e6f1f62abedb5f8c039cc6c048971b5e/routeros_api/api.py#L60-L70 and that's the place that has to updated. It seems that the password should be now sent as plaintext. If anyone has some spare time, Pull Requests are welcome! Please remember that the code should handle both login methods (<6.43 and >=6.43) :)

oortega commented 6 years ago

Hello @jgoclawski

I'm researching in the mikrotik forum, the idea of login is something like this: https://forum.mikrotik.com/viewtopic.php?f=21&t=133420&start=250#p665728

_You don't have to check version.

  • Send /login with username and password in plain text
  • if response is with challenge then fall back to old login method
  • otherwise check status if login is successful_

regards

stevehaskew commented 6 years ago

I've submitted PR #32 for this. It handles the latest v6.43 RC fine. Incidentally, both old and new login methods work on the current candidate release. You can force it to use the old method to prevent sending the password in plaintext unnecessarily, otherwise it uses the fallback scenario above.

oortega commented 6 years ago

@stevehaskew great job. It's true 6.43RC32 works with the old login. Looking at the PR I will try to do it in another way to avoid the var force_old_login because I have clients working with both versions.

stevehaskew commented 6 years ago

@oortega You don't need the force_old_login - it works for both, backward compatible etc, but because the initial attempt is with plaintext password it is possible you want to choose not to send your password in plaintext if you know all your devices are <v6.43.

oortega commented 6 years ago

@stevehaskew I say sometime like this:

https://github.com/oortega/RouterOS-api/blob/c2f7ee1781d546aa67beb69a27ed0a3df3322e21/routeros_api/api.py#L60-L76

def login(self, login, password):
    try:
        #Default Old_login < 6.42
        response = self.get_binary_resource('/').call('login')
        token = binascii.unhexlify(response.done_message['ret'])
        hasher = hashlib.md5()
        hasher.update(b'\x00')
        hasher.update(password.encode())
        hasher.update(token)
        hashed = b'00' + hasher.hexdigest().encode('ascii')
        #assert False
        self.get_binary_resource('/').call(
            'login', {'name': login.encode(), 'response': hashed})

    except exceptions.RouterOsApiCommunicationError:
        #New Login >6.43
        response = self.get_binary_resource('/').call('login', {'name': login, 'password': password})
davidc commented 6 years ago

If I'm reading the PR correctly, it attempts to first login plaintext, and only if that fails attempts the hashed version.

This default behaviour means that it may be be sending plaintext passwords on unsecured connections, even if using the old API where that wasn't necessary.

Even if the ordering is changed to try hashed first, it seems that it might still attempt sending plaintext e.g. if incorrect credentials specified.

I feel it would be a more secure default behaviour to never send the plaintext password unless:

a) the caller has specifically requested it; or b) if SSL is in use it might be acceptable to try it automatically

It seems the best solution (rather than auto-detection) would be for RouterOS to send something prior to the login attempt that will indicate what login method it is expecting (possibly by means of an initial version query command, or by the welcome banner if one exists) - I've not gone down into the nitty-gritty of the API due to the appalling documentation, so I don't know which is possible. Perhaps someone who knows it in more detail can submit a request to Mikrotik support, as a fix may still be possible while it is still only a RC?

But even if this is added (which solves the auto-detection problem), I don't believe this API client should ever send the password plaintext over non-SSL without the caller's explicit request to do so.

I believe Mikrotik removed the password hashing to make it clearer to API users that it really isn't secure and that they should be using SSL. I believe that clarity should be passed down to users of this API.

stevehaskew commented 6 years ago

I agree with the suggestion that we should only use the new insecure login type if the user requests it. Since my PR for SSL support has not been merged, we don't officially have SSL support yet and therefore we can't opt to permit insecure login when SSL is enabled, but I suggest we do.

stevehaskew commented 6 years ago

This is now implemented and available in master branch.

jgoclawski commented 6 years ago

Fixed in #32. Released as 0.15.0. Thanks a lot to everyone involved! @stevehaskew @oortega @kramarz @davidc