Forum OpenACS Development: Adding ad_session_id_secure cookie

Collapse
Posted by Dave Bauer on

Right now we have secure variants of as_user_login cookie but not the ad_session_id cookie.

I have made a patch that adds this so in a secure conn all cookies will be sent securely.

Anyone else interested in this?

*** security-procs.tcl  2007-08-27 13:01:08.000000000 -0400
--- security-procs.tcl.orig     2007-08-27 12:56:35.000000000 -0400
***************
*** 15,21 ****
  # cookies (all are signed cookies):
  #   cookie                value                          max-age         secur
e
  #   ad_session_id         session_id,user_id,login_level SessionTimeout  no
- #   ad_session_id_secure  session_id,user_id,login_level SessionTimeout  yes
  #   ad_user_login         user_id,issue_time,auth_token  never expires   no
  #   ad_user_login_secure  user_id,random                 never expires   yes
  #   ad_secure_token       session_id,user_id,random      SessionLifetime yes
--- 15,20 ----
***************
*** 77,87 ****
  } {
      ns_log debug "OACS= sec_handler: enter"
      if { [catch { 
!       if { [security::secure_conn_p]} {
!             set cookie_list [ad_get_signed_cookie_with_expr "ad_session_id_sec
ure"]
!         } else {
!             set cookie_list [ad_get_signed_cookie_with_expr "ad_session_id"]
!         }
      } errmsg ] } {
        # Cookie is invalid because either:
        # -> it was never set
--- 76,82 ----
  } {
      ns_log debug "OACS= sec_handler: enter"
      if { [catch { 
!       set cookie_list [ad_get_signed_cookie_with_expr "ad_session_id"]
      } errmsg ] } {
        # Cookie is invalid because either:
        # -> it was never set
***************
*** 304,310 ****
      Logs the user out. 
  } {
      ad_set_cookie -replace t -max_age 0 ad_session_id ""
-     ad_set_cookie -replace t -max_age 0 ad_session_id_secure ""
      ad_set_cookie -replace t -max_age 0 ad_secure_token ""
      ad_set_cookie -replace t -max_age 0 ad_user_login ""
      ad_set_cookie -replace t -max_age 0 ad_user_login_secure ""
--- 299,304 ----
***************
*** 468,477 ****
      ns_log Debug "Security: [ns_time] sec_generate_session_id_cookie setting s
ession_id=$session_id, user_id=$user_id, login_level=$login_level"
      ad_set_signed_cookie -replace t -max_age [sec_session_timeout] \
            "ad_session_id" "$session_id,$user_id,$login_level"
-     if {[security::secure_conn_p]} {
-         ad_set_signed_cookie -secure t -replace t -max_age [sec_session_timeou
t] \
-           "ad_session_id_secure" "$session_id,$user_id,$login_level"
-     }
  }
  
  ad_proc -private sec_generate_secure_token_cookie { } { 
--- 462,467 ----
Collapse
Posted by Torben Brosten on
Is there a case where a secure connection should not have all cookies sent securely?

If this doesn't adversely affect how sessions are interpreted by browsers, why not commit it to head?

Collapse
Posted by Dave Bauer on
I found a problem with this

Because the system only checks either the ad_session_id or ad_session_id secure cookie, if you enter in HTTP or HTTPS
when you change the HTTP to HTTPS, to login, for example, the session_id cookie that was set in HTTP will not be read and the session_id will change.

This would be a problem for an ecommerce site where you saved items in your shoppinh cart in HTTP then were redirected to HTTPS to check out.

The only solution I can imagine is to send the session_id in the URL when switching from HTTP to HTTP or back.

Then the security handler would need a way to extract that from the URL.

Collapse
Posted by Torben Brosten on
Have you tested this with ecommerce?

Ecommerce has it's own session cookie which is kept same for either http/https for most cases (as far as I remember). Anyway, switching between http and https has not been a problem with ecommerce on oacs 5.1.

Collapse
Posted by Dave Bauer on
Well in this case, ecommerce is most likely also sending the session_id cookie insecurely. I have not reviewed that code, and it probably makes sense to consolidate the code anyway.
Collapse
Posted by Torben Brosten on
Yes, consolidating would improve user experience. Basically the ecommerce requirements are for smooth transitioning of sessions between http/https and session continuity (for retaining shopping basket contents etc) ie things which regular user sessions could benefit from in the context of other services.

I'd be glad to help test, but I don't have the expertise to directly modify core (all previous attempts failed to pass tests).

Here's the procs:

ec_get_user_session_id
ec_user_session_logout
ec_log_user_as_user_id_for_this_session
ec_create_new_session_if_necessary
ec_redirect_to_https_if_possible_and_necessary

cheers,
Torben