Forum OpenACS Q&A: Any access via https requires login?

Greetings,

I've been chasing a bug in the toolkit for a few weeks and am not sure how to go about fixing it without compromising the security of login and https.

This came about because I noticed that sometimes if a user went to our homepage, then clicked on our login link which goes to /register via https, none of the images on our /register page would appear. They would all show as broken links. Looking in the access log, I noticed that a redirect to /register with return_url set to each image was being generated.

However, this was not consistent. Then I discovered that if you closed and restarted the browser (which would clear the session cookies), and hit the homepage with https *first* instead of http, going to the /register page via https would show the images.

My testing has been done with a recent checkout from CVS, a minimal set of packages including the monitoring package loaded, and nsopenssl configured. I see this behavior on both standard and non-standard ports.

What appears to be happening is that if a user hits the site on http before https, then goes to https, this code in sec_handler of packages/acs-tcl/tcl/security-procs.tcl is invoked:

	# If it's a secure page and not a login page, we check
	# secure token (can't check login page because they aren't
	# issued their secure tokens until after they pass through)
	# It is important to note that the entire secure login
	# system depends on these two functions
  	if { [ad_secure_conn_p] && ![ad_login_page] } {
  	    if { [catch { set sec_token [split 
[ad_get_signed_cookie "ad_secure_token"] {,}] }] } {
  		# token is incorrect or nonexistent, so we force 
relogin.
		ns_log notice "sec_handler: token was incorrect or 
not there, need to login"
  		ad_returnredirect "/register/index?return_url=
[ns_urlencode [ad_conn url]?[ad_conn query]]"
  		return filter_break
  	    } else {
		# need to check only one of the user_id and session_id
		# if the cookie had been tampered.
		if { ![string match [lindex $sec_token 0] 
$session_id] } {
		    ad_returnredirect "/register/index?return_url=
[ns_urlencode [ad_conn url]?[ad_conn query]]"
		    return filter_break
		}
	    }
	}

So it appears that the token is "incorrect or not there" if it was first generated via http. Can the token be "upgraded" somehow if a user hits first on http? Or does that open the site to some kind of attack?

To reproduce this, just create a simple adp page in the pageroot containing just

---
<master>
Test Page
---

Start the server. Hit the root of the site with http. Hit it again with https. Then hit the test page via https. You will be asked to login.

Now close all windows of your browser (to make sure session cookies are cleared). Start the browser and hit the main page of the site with https. Then hit the test page with https. You will see the test page contents without being asked to login, but the test page will have been served with https.

I don't get it. Is this by design? I *think* the correct behavior of the toolkit is if you allow logins over http a user can login via http. If he switches to https and hits a protected page he will be asked to login again. But all my testing has to do with a non-logged in user...

Now to throw another wrench in the works I just discovered that IE 6 and Mozilla 1.0 behave differently here. IE 6 behaves as above. Moz 1.0 makes the user login regardless of if you hit the site first with http or https.

Feedback appreciated.

Success!

I was right, the root reason for different behavior depending on if the site is hit first by http or https is the presence/absence of the security token. This is definitely a bug.

In the documentation by Richard Li at doc/security-notes.html he specifically says that a secure token should never be issued unless a user has already authenticated himself over https. However, if you hit the site first with https, then sec_session_setup generates a secure token regardless of whether you are logged in or not. Oops.

So I think that can be fixed with a one-line patch to sec_session_setup that does not issue the token unless the user is logged in.

Now, there are two scenarios that don't appear to be supported by the toolkit's security model:

1. Visiting parts of a site https, but not logged in. (parts that shouldn't require login, that is).
2. A true permanent login if https is in use.

1. is disallowed by the code in Li's document that he says you can remove if you want a weaker security model. That code sends you to /register if you hit the site anywhere with https and you are not logged in. I guess the theory is "why would you want to talk to my site with https if you are not logged in?"

2. is disallowed because the security model assumes you *could* login over http or https. The tk makes you re-login if you switch from http to https (even if your target page does not require login).

So, in our case at least, we only allow logins over https. Thus if the user has a valid ad_user_login_secure cookie, we can re-issue his secure token if it is not present.

This appears to fix all of our issues.

I've generated a patch against 4.5 that fixes this and uploaded it to SDM. It's attached to the original bug report # 1549 I'm pretty sure the patch will be transparent to sites that allow unencrypted login over http, and will still enforce the requirement that a login via http should be re-requested should the user switch to https.

Allowing access to parts of a site via https that do not require login still requires that the code that Richard Li documents in /doc/security-notes.html be removed from the sec_handler proc.

Or, again in our case, we only cared about that because our images were broken on our register page. That was easy to fix (and Richard actually documented that) by changing ad_login_page to say that the /graphics directory is part of the "login" page.

Whew. What a mouthful.

Anyone out there who knows more about the security model than I do or indeed more about achieving good security with web sites, feel free to speak up if I have broken things terribly here.

--cro