huge authentication flaw

[itocinc]

1 - Success, but continue processing MySQL tables as well
2 - Success, don't bother checking MySQL tables

Hi,

I installed this and noticed two huge flaws.

1. Your password in the MySQL table must be identical to the ldap user password. If not, it will return as an error because the username and password don't match.
2. I found that since it doesn't check the MySQL table, you don't even have to enter a password. I literally entered a username and hit enter and was allowed in the admin section. It simply ignores all passwords.

This should authenticate through Ldap username and password without having to be identical to the Mysql table (it defeats the purpose and will be a nightmare when you change your ldap password) It should also not ignore the need to enter the correct ldap password as in #2.

Hope this helps.

[anzenehansen]

If you tried this in recent Blesta versions (3.2+) it very well could be broken. I haven't worked on it since 3.0 or 3.1, and it was more so a proof of concept of how to make it happen and also I wanted to know how to create my own event listeners in their framework.

However, to address some points you made:

1. Your password in the MySQL table must be identical to the ldap user password. If not, it will return as an error because the username and password don't match.

This shouldn't happen, as I was able to successfully log in with a different password via LDAP than I had in MySQL. But, again, this could also be due to the version difference as well.

1. I found that since it doesn't check the MySQL table, you don't even have to enter a password. I literally entered a username and hit enter and was allowed in the admin section. It simply ignores all passwords.

I'm not 100% sure I did the admin check. Could've swore I did, but perhaps not?

I might rewrite this though, especially when 3.3 comes out (which I'm hoping is soon). I do appreciate you bringing this to my attention, however. Right now I'm going to leave this open and I will revisit this with 3.3.

[itocinc]

First let me say the proof of concept was excellent. I wish I was a programmer, but I only know how to fiddle with others code more than I know how to write it. I attached my users.php file in-case maybe I did something wrong. I can at least help provide information and also eliminate user error.

I have to admit, I couldn't get it fully working to know exactly what the issues are, so I just wrote to you what was happening after I installed. When you work on it, I think a few additions would make his great:

1. Ldap check: just a few minutes ago, I entered the wrong ldap information on purpose , and it was accepted successfully. There should be a check that only on a successful connection to the ldap will allow the ldap info get stored in the database.
2. Since the ldap info is stored in the MySQL table, it would be nice to be able to authenticate blesta users using multiple ldap servers. I actually would want customers and admins on different ldaps (but that's just my preference to keep clients in their own system). I thought it was working like this but could not verify it as I didn't get it to work successfully.

I do know I entered the ldap information in correctly because I used the same info I use to connect a few other apps. When I couldn't log in (kept getting mismatch error), I changed return 1 to return 2 below. Then I was able to log in even without a password.

/* Add this function */ public function pre_auth($username, array $vars, $type="any"){ $this->Events->register("Users.pre_auth", array("EventsUsersCallback", "pre_auth")); $res = $this->Events->trigger(new EventObject("Users.pre_auth", $vars));

            return 1;
    }

o.k. Take care.

On Wed, Aug 20, 2014 at 8:39 AM, anzenehansen notifications@github.com wrote:

If you tried this in recent Blesta versions (3.2+) it very well could be broken. I haven't worked on it since 3.0 or 3.1, and it was more so a proof of concept of how to make it happen and also I wanted to know how to create my own event listeners in their framework.

However, to address some points you made:

1. Your password in the MySQL table must be identical to the ldap user password. If not, it will return as an error because the username and password don't match.

   This shouldn't happen, as I was able to successfully log in with a different password via LDAP than I had in MySQL. But, again, this could also be due to the version difference as well.

2. I found that since it doesn't check the MySQL table, you don't even have to enter a password. I literally entered a username and hit enter and was allowed in the admin section. It simply ignores all passwords.

   I'm not 100% sure I did the admin check. Could've swore I did, but perhaps not?

I might rewrite this though, especially when 3.3 comes out (which I'm hoping is soon). I do appreciate you bringing this to my attention, however. Right now I'm going to leave this open and I will revisit this with 3.3.

— Reply to this email directly or view it on GitHub https://github.com/anzenehansen/Blesta-Goodies/issues/1#issuecomment-52771530 .

[anzenehansen]

The original idea when writing it was to do an LDAP check actually. The issue with it is that the DN will most likely be different for admins vs clients, so figuring out how to handle both in one isn't too easy. Granted, the user could just have a test account for such purposes, but that's a restriction that's kind of silly to make a requirement.

As for authenticating with different servers, that would also fall into the DN string, I believe. I can't remember. However, implementing something like that wouldn't be difficult. Its definitely possible to separate the host/port/etc... from the LDAP query string.

I might get bored this weekend and revamp this sucker, who knows, lol. If so I will probably move it to its own repo to keep things separate.

[itocinc]

I can help with the testing. Sounds promising.

On Thu, Aug 21, 2014 at 11:26 AM, anzenehansen notifications@github.com wrote:

The original idea when writing it was to do an LDAP check actually. The issue with it is that the DN will most likely be different for admins vs clients, so figuring out how to handle both in one isn't too easy. Granted, the user could just have a test account for such purposes, but that's a restriction that's kind of silly to make a requirement.

As for authenticating with different servers, that would also fall into the DN string, I believe. I can't remember. However, implementing something like that wouldn't be difficult. Its definitely possible to separate the host/port/etc... from the LDAP query string.

I might get bored this weekend and revamp this sucker, who knows, lol. If so I will probably move it to its own repo to keep things separate.

— Reply to this email directly or view it on GitHub https://github.com/anzenehansen/Blesta-Goodies/issues/1#issuecomment-52936167 .

[itocinc]

Hi,

Hope all is well. I have a question. I read on the Blesta forum that you do custom development.

Could I hire you to complete this for my needs? I was also wondering could you keep the ldap for the client and admins as two separate modules to help with the security check. If you can keep the client ldap on the front end and the admin ldap on the backend, then they could have checks and bind independently of each other.

I know most of the work is already done. So I'm hoping I could help get what I'm looking for by hiring you just to complete the work. Of course, it would also be non-exclusive as you already have developed the plugin for the most part. The reason this has now become important is that my registrar is beginning to finally work on a plugin for Blesta and I can now use it for my needs.

Let me know your thoughts, I'm interested for reasonable compensation.

Thanks,

Jamal

On Thu, Aug 21, 2014 at 11:26 AM, anzenehansen notifications@github.com wrote:

The original idea when writing it was to do an LDAP check actually. The issue with it is that the DN will most likely be different for admins vs clients, so figuring out how to handle both in one isn't too easy. Granted, the user could just have a test account for such purposes, but that's a restriction that's kind of silly to make a requirement.

As for authenticating with different servers, that would also fall into the DN string, I believe. I can't remember. However, implementing something like that wouldn't be difficult. Its definitely possible to separate the host/port/etc... from the LDAP query string.

I might get bored this weekend and revamp this sucker, who knows, lol. If so I will probably move it to its own repo to keep things separate.

— Reply to this email directly or view it on GitHub https://github.com/anzenehansen/Blesta-Goodies/issues/1#issuecomment-52936167 .

[anzenehansen]

The problem with trying to segregate this as you want is that Blesta doesn't allow it. Really they don't even allow custom authentication plugins so this would still require editing core files (unless more recent updates have changed this, haven't kept up with the change logs since 3.2.1 came out).

So, while it's possible to separate the DN string for both, its not possible to make them transparent from each other, because from what I remember the system checks to see if the username is in the admin/staff or client table and goes from there.

I'd be glad to work on this though, that's not a problem. If you want to go into some details email me at ehansen@anzensolutions.com