Serendipity Forums

What about using the local config file. Looking through the code, it should be possible to moving the code for locating and including that prior to starting the session. This would also require setting $serendipity to an array here instead of the compat file as well as defining IS_installed sooner. Bascially if the local config can be read prior to compat.inc.php being included, then there should be no problems with starting the session after reading the local config. The first use of session is in compat.inc.php and no hook events are issued until well after the user configuration is loaded from db, so as long as things can be done before compat is read things should be fine.

On another note I was wondering about the best way to prevent the built in authentication method to not check author accounts created by external auth mechanisms that need to create author accounts yet should not be accessible via the regular user/pass login. Currently it should be possible to just set the username field blank, but wonder if in the long run that is the best way to do this. Other possible options are a flag indicating inernal or external auth or even a field for authtype identification. By not allowing use of the username field, it limits the data (for linking the the external auth to a user account) that can be stored in the author record and might require the external auth mechanism to create an additional table.
Not really a bad thing but wanted to get some opinions on this.

Was also thinking it might be a good idea to increase the password field size to allow for larger sha hashing - I see this more for extern auth so pieces of info can be used as identifier and think allowing only 32 chars is a bit limiting.

Really wish there were some other people interested enough in this to add to the input here :/

Rob

Hi!

Yeah, some more input would help. Sadly the authentication thing is a piece where its harder to wrap someones head around. Which is all the more reason to appreciate you doing it.

I think putting it into the local config file could make it work. I'll try to create a patch for this - however we need to make sure that no vital steps are performed in that file (like setting $serendipity should be done in serendipity_config.inc.php and not the local file, because on existing installations it would else not have that array unset).

Extending the password field size is also a good thing which I will gladly do.

About the identification, I believe a global flag for indiciating the authentication method would be nice. $serendipity['authenticationMethod'] could be used, which defaults to 'internal'?

About a custom field within the database, I believe a siple value in serendipity_config DB table should do, there shouldn'T be a need to add a new field to the serendipity_authors table for that, right?

Best regards,
Garvin

garvinhicking wrote:About a custom field within the database, I believe a siple value in serendipity_config DB table should do, there shouldn'T be a need to add a new field to the serendipity_authors table for that, right?

I'm trying to catch up on what you've discussed so far, but it's by no means an easy issue. Anyway, what if I want to offer multiple ways to log into my blog? Along the lines of: "Login with your $ExternalID, or enter your username and password here", so that you have the choice and both ways would work? Would that be possible?

And one more thing I would really like to have in my blog, but I'm not sure if it has already been covered in this thread, is $ExternalID-enabled comments so that commenters don't have to enter their e-mail anymore but can use their $ExternalID instead. Do you think this should/could go into core also?

- Mattsches

Hi!

I'm trying to catch up on what you've discussed so far, but it's by no means an easy issue. Anyway, what if I want to offer multiple ways to log into my blog? Along the lines of: "Login with your $ExternalID, or enter your username and password here", so that you have the choice and both ways would work? Would that be possible?

Basically it would be possible, but whatever offers the login must make sure that the author is insertedf into the serendipity_authors DB table. Without that DB table, necessary DB joins cannot be made. So it's not really possible to authenticate someone without that "proxy" solution.

And one more thing I would really like to have in my blog, but I'm not sure if it has already been covered in this thread, is $ExternalID-enabled comments so that commenters don't have to enter their e-mail anymore but can use their $ExternalID instead. Do you think this should/could go into core also?

I think this one can already be solved with services like coComment, right?

Best regards,
Garvin

Hi!

Today I tried to wrap my head around shuffling the logic in compat and config.inc.php but failed miserable.

The whole logic is so fixed-ordered that changing a small thing might break shared installations or the installation process easily. Moving the local config file before the session page start is serously wicked, and I don't have the time to test all variations (is_installed, in_installer, shared installation, already installed, upgrade installation).

Maybe the solution with a $_ENV value for configuring the SSLID is better to do. The safer thing is to make http and https distinct, so configuring https+http to work with the same sesion data would make things unsafe again. All the possible pain to offer this functionality might break much more important parts.

So instead of going through that, I might suggest for people who want the solution of shared http+https logins to simply edit the serendipity_config.inc.php file manually?

Best regards,
Garvin

This is a tough one and why I didnt rush back to reply - wanted to think about it more. Really it should be easy to work in a mixed environment and perhaps that previous change to make things more secure was not the best solution. For instance, this is not a banking or other high security app that is required for everything to run under HTTPS. This means that some of the identity and authentication state should be able to be accessed while within HTTP, yet admin should be more secure.

Possibly some type of token could be passed with a secure cookie that would allow the interaction with admin system to take place. For the token it might be something as simple as the hash of the current sessionid and some piece of authentication information. This way it also is only valid for as long as the session lasts. Just thinking out loud here, but I really think the current method is too restrictive yet at the same time beneficial so there needs to be a way to maintain security and not break BC - which happens when upgrading to 1.1.

Rob

Hi!

Even though I agree that Serendipity is, unlike tools like phpMyAdmin, a high security app, I do think that it's cookies can grant you full administartive access to a server and might be used for exploits and phishing. So I do think seperating the advocated safe use of HTTPS should be as safe as possible.

BTW, the reason for the patch inclusions were these tracker items:

http://sourceforge.net/tracker/index.ph ... tid=542822

Of course that doesn'T bring us any closer to the solution

I'm afraid I don't fully understand the suggestion of the use of the token you advised, could you explain that to me more detailed or with examples? It currently sounds as if using this token is the same as using the session id so that http and https will be shared?

I'm still thinking if we can't try to solve this via a php.ini setting, that we check on using ini_get() before we set the cookie IDs. But it seems that one cannot pass custom php.ini values anyhow. How about using that ENV value, and you could edit the .htaccess to set the value there?

I believe a BC break for improved security is worth that hassle, especially because apart from you no one has yet reported issues with shared http/https, which tends to make me believe that few people are using it. Which would be more the reason to leave the current setting by default and not go through all the pain of spending big time on reshuffling the init flow to accomodate an internal configuration value?

I hope I don'T get off as sounding rude or not understanding. I do understand your personal preference of how it should be, but I'm also in a position where I need to weigh the broad user base's usage cases to the specific ones. I strongly believe in making Serendipity as flexible as possible, but I'm also lacking the time to implement big changes like the reshuffling.

Finally, let me tell you how much I appreciate your input on this. If more people like you were involved, we might get a much more sophisticated solution to this problem on the road.

Best regards,
Garvin

garvinhicking wrote:Even though I agree that Serendipity is, unlike tools like phpMyAdmin, a high security app, I do think that it's cookies can grant you full administartive access to a server and might be used for exploits and phishing. So I do think seperating the advocated safe use of HTTPS should be as safe as possible.

I fully agree with that. I just meant that it is unrealistic to require the entire application (non-admin portion) to run within HTTPS while at the same time being able to identify the user and maintain preferences when just browsing the public side. HTTPS is going to be important with these identity technologies to protect the anonymous user who is using credentials to identify themselves with the blog. It only will be needed during the authentication potion - i.e. submitting an infocard or used during the OpenID callback (not required but a good way to protect the users personal info). But now if there is no mixing of HTTP/HTTPS sessions then it is really moot. These authentications could only be used for authors working in admin (defeats the purpose of even implementing them then).

It is in part this and the need for security that I think a better solution than an ini/env setting would be justified. Though they would both work, they are pretty much a short term patch rather than a long term solution. Using either (and running in the mixed environment) would still allow the attack vector of temporarily gaining access to admin possible.

Reading that report the issues appeared to be:
1 - author_information cookie sent as regular cookie when working within HTTPS
2 - because the session is shared, it can get hijacked and someone could gain temp access into admin section.

It should be possible to provide the security mentioned in that report as well as not limiting the application. As far as the BC comment I had made, I merely meant that it is should be possible to implement the security without breaking BC. Its not a quick 4 liner and would involve some work, but in the long term it would pay off.

One option I had thought of involved using shared session but securing specific information. For instance, right off the bat, if within HTTPS, author_information cookie is sent secure (solves issue 1). Next, if a token were set in a secure cookie, then the token would only be available under HTTPS; otherwise it is set as a regular cookie. Now, require admin to validate this token. It validates it by hashing the current user session id, and some piece of authenticating info and comparing the value to what was passed. For example it could be a hash of the session id, and some authenticating piece of user information. This way even if the session were hijacked by sniffing traffic, the attacker could not access admin without this token - which they would not see when sniffing due to the encrypted tunnel when its passed. There are possibly other ways to do the same thing, but just wanted to give you an idea what I meant here.

I am fully aware of the time is short problem Even if this cant be done right this second, it would be good if some sort of plan can be agreed upon. That way wether it takes weeks or months at least it can progress instead of just dropping it all together because it is difficult and take time. I doubt you disagree with that and just wanted to let you know that I am not going to drop this just cause it cant be implemented tomorrow .

Rob

Hi Rob!

It should be possible to provide the security mentioned in that report as well as not limiting the application.

If that is possible somehow, I'd love to see that happen.

My understanding of security is quite limited to basics and what I've learned over the past - I'm by no means a security-focussed enginer. This might be a reason why I have had problems stabbing at authentication possibilities like OpenID.

One option I had thought of involved using shared session but securing specific information. For instance, right off the bat, if within HTTPS, author_information cookie is sent secure (solves issue 1). Next, if a token were set in a secure cookie, then the token would only be available under HTTPS; otherwise it is set as a regular cookie.

Okay, that sounds good and likely and does protect the author_information. Thanks for explaining this to me.

This doesn't even sound too hard - from what I understand it could hook into the current authenticate_author() routines and the serendipity_issueAutologin() function?

Would you maybe like to try patching up the two functions with your idea of the implementation? Then the two ini_set() commands could IMHO be dropped altogether as they are no longer relevant. And before I implement your idea a wrong way, you might be the best person to implement that? And the upside of that would also be that you would get the full credits to all of that...

Many thanks, again, for your input. People like you are who I tried to draw attention from by making serendipity as flexible as possible.

Best regards,
Garvin

Here is my work in progress for eliminating the secure session requirement. It has been testing out fine for me, though I havent tested a multi-blog environment nor every plugin. The security is not locked down hard (would add much more overhead) but imo provides more security than even the current behavior when using https for logins.
http://www.cdatazone.org/files/serendipity.patch.txt

The changes are as follows:
- add an optional parameter to the setcookie function. Currently the cookie function sets the security based on whether it is being set via http or https. I added a flag to allow a cookie to be set with no security when under https.
- session is no generated with unsecure cookie (it can now be shared http and https)
- i set some cookies i found as unsecured as there was no reason to lock them down based on protocol.
- upon login, an author token (just a hash) is set in a cookie (Secure if https or not if http). This hash is also set within the session. The two must match in order to access the admin section, which also means that if login is performed via https then admin must also be accessed via https. (same behavior as when using secure sessions).
- There are only a few spots I could find where the hash cannot be checked; reason being is that the functionality does not pass through the admin page. i.e. deleting comments is possible on the public side, and a user would be hitting this via http but the token was set (assuming https logins) securely so is not passed with the request via http. As I mentioned I havent tested all plugins so dont know other spots that allow functionality like this without passing through admin.

Let me know if you see any problems right off the bat doing any of this so far.

Thanks,

Rob

Hi!

Great! I like small patches. *g*

One question:
Shouldn't the first call of userLoggedIn() be WITHOUT the 'true', as it currently is? If you use 'true' two times, that should be redundant?

One more question:

- There are only a few spots I could find where the hash cannot be checked; reason being is that the functionality does not pass through the admin page. i.e. deleting comments is possible on the public side, and a user would be hitting this via http but the token was set (assuming https logins) securely so is not passed with the request via http. As I mentioned I havent tested all plugins so dont know other spots that allow functionality like this without passing through admin.

Effectively this means if a user has logged in via https, the http portion of the blog will not recognize him as logged in, right?

IMHO that is desired functionality - if people want their login to be recognized on the frontend, they need to access the frontend via HTTPS. That any link performing admin actions will not work when being accessed via HTTP is IMHO the same way as we have now, so I think that's perfectly fine.

Other than that, your patch looks good to me, so I think it could get committed into the 1.2 branch soon.

Best regards,
Garvin

if (!serendipity_userLoggedIn(TRUE)) {

Shouldn't the first call of userLoggedIn() be WITHOUT the 'true', as it currently is? If you use 'true' two times, that should be redundant?

Correct. That was a piece of left over code, as I had tried altering the userLoggedIn functionality (to no avail as I will explain in next section. I forgot to remove the parameter here when I reverted the code change to the function.

Effectively this means if a user has logged in via https, the http portion of the blog will not recognize him as logged in, right?

IMHO that is desired functionality - if people want their login to be recognized on the frontend, they need to access the frontend via HTTPS. That any link performing admin actions will not work when being accessed via HTTP is IMHO the same way as we have now, so I think that's perfectly fine.

Actually yes and no. The login is recognized on the front end, but to perform admin actions (actions requiring passing through serendipity_admin.php) they must use the same protocol (http/https) that was used for logon. So the latter part is correct but I will elaborate on the former.

The front part needs to recognize the logon. For example, if someone logs on with an OpenID, the system must recognize this so that the user can add a comment without dealing with the captcha and depending upon settings their name/email passed at login time. This doesn't mean though that when they are logged on they can do admin tasks. If the user does not have accessed to alter anything, then they would not see any of the edit links on the front section (desireable behavior). If the user does have access, then they would see the edit links, but in order to perform any of those admin actions - they *MUST* access admin via same protocol as login (http/https). I had tried playing around with userLoggedIn() but altering the login checks would break a lot of plugins that use it, so I had to abondon that idea.

Now, imo the behavior is not bad. Worse case is that a user would see the edit links if they had enough permissions based on their login (even though the proper protocol must be used to access that functionality). This had been the behavior until 1.1 so would not be mind altering to people if it went back that way (really only noticeable to those using the SSL login). If this does bother people and the behavior must remain, here are a couple of my suggestions on how this can be worked around:

1 - enable some flag in admin that disables the displaying the Edit links on frontpage.

2 - in reality, unless the site is a subscribers only site, very few users (assuming OpenID/InfoCards have been implemented) will or should have enough permissions to even trigger the frontpage links. If this does bother someone under HTTPS that much and they do have some type of subscriber site where security is an issue, they really should be running admin under its own url anyways to insure there is no possible way of data bleed between the front and backend (this is a good case for mod_rewrite).

This also brings up the question of those edit links. Right now I am assuming they remain either as is or using option 1 above. Is it possible to add a setting in admin to force them to point to either http or https? Right now they point to the same protocol the page is being viewed in. This means if a user logs in via https, the http links do them no good because of the new token code. The user *MUST* access them via https to be able to perform any action. This only pertains to functionality pointing to admin. Any functionality a plugin might provide directly in the frontend would not be affected.

Lastly, I am not sure what your schedule for doing a 1.2 release is, but I want to make sure that this code is well tested and thought out before it were to be released. So basically if a 1.2 release was planned to be released soon, I would rather this code be held off being committed until after the release. I am just nervious that a rush commit of this would put things back to were they are with that https code from 1.1 (works but not optimal, changed behavior, yet needs to be changed again in another release).

Rob

Hi!

Thanks for elaborating, then I would just remove the TRUE part and leave the rest as is?

The way you described the frontend is now clearer to me, but it sums up to what I previously believe it would.

In the long run we should make the login put a session variable that indicates if a user is http or https authenticated, and make Edit-Links and comment links use the corresponding login. But IMHO this is non-critical functionality and can be added later on.

Currently the edit links have no easy mean to use a full URL, as it always uses the current one. So all parts where the links are emitted would need to be patched up, same goes to plugins.

Serendipity 1.2 has still a longer time to go. Until now we have no real features that would ratify a new "major" release. Bugfixes in the 1.1.1 branch should soon be released, but I see 1.2 drifting in beta-status for at least 2 more months. The OpenID things would be a feature rectifying a release, so any effort put into this should be worth delaying a release.

Thus, I am going to commit your code soon and am going to put up a blog + forum entry asking users for feedback on it.

Best regards,
Garvin

Hi!

Just to let you know: I just committed your patch to SVN trunk.

Best regards,
Garvin

Just to let you know: I just committed your patch to SVN trunk.

Thanks,

And to think I had just finished updating that patch for you

Rob