bbangert / beaker

WSGI middleware for sessions and caching
https://beaker.readthedocs.org/
Other
524 stars 143 forks source link

Documentation might lead to a session fixation attack #41

Open ghost opened 11 years ago

ghost commented 11 years ago

I am using beaker with pyramid_beaker in a pyramid application. I found out that I could forge a cookie session_id with the simple value ‘123’ and make it valid once I log in to the application. Here is the beaker configuration set in the development.ini of the pyramid application:

# Options For Sessions and Caching:
session.type = file
session.data_dir = %(here)s/../../data/sessions/data
session.lock_dir = %(here)s/../../data/sessions/lock
# Session Options:
session.key = session_id
session.secure = false
session.timeout = 3600
session.cookie_expires = true
session.cookie_domain = .mydomain.local
session.httponly = true
# Encryption Options:
session.encrypt_key = c]?wvL",ni3J.)d8(e~z8b-9Le=Anh'.QMytBj^Kukfi<79C$Cg22)cX;__xs6?S
session.validate_key = \2R('?pL]\Z_8?(o`.?.?^.RF6t*5pCh6PH`~aon%H`PX$;E}"((mu-@(?G<=!:+
# pyramid_beaker specific option
session.cookie_on_exception = true

You can notice there is no session.secret set because I followed the recomendation in the documentation: "These options should then be used instead of the secret option listed above." (http://beaker.readthedocs.org/en/latest/configuration.html#encryption-options)

And here is the login form view:

def login(self):

    message_html = _('view.login.welcome-message', default='Please log in.')
    login_url = self.request.route_url('login')
    login = ''
    password = ''
    referrer = self.request.url
    if referrer == login_url:
        referrer = self.request.route_url('home')
    came_from = self.request.POST.get('came_from', referrer)
    csrf_token = self.request.session.get_csrf_token()

    if 'form.submitted' in self.request.POST:
        login = self.request.POST.get('login')
        password = self.request.POST.get('password')
        if csrf_token == self.request.POST.get('csrf_token'):
            if login in USERS:
                manager = BCRYPTPasswordManager()
                if manager.check(USERS[login], password):
                    headers = remember(self.request, login)
                    return HTTPFound(location=came_from, headers=headers)

        message_html = _('view.login.failed-login-message', default='Login failed!')

    return {
        'message_html': message_html,
        'url': login_url,
        'login': login,
        'password': password,
        'came_from': came_from,
        'csrf_token': csrf_token,
    }

Now, my problem is that my application might be vulnerable to a session fixation attack (See http://security.stackexchange.com/a/35097/25414).

I asked how to fix that on StackOverflow (http://stackoverflow.com/q/16303414/1919510) and I received an interesting comment: "If the cookie value isn't signed then you aren't setting session.secret in your ini settings." (http://stackoverflow.com/questions/16303414/how-to-prevent-user-to-set-the-value-of-the-session-id-cookie-used-by-pyramid-be#comment23379513_16309210)

This is true, I was not using session.secret. Using both session.secret and the encryption options (encrypt_key and validate_key) prevent me to forge a cookie with value 123 and then fix the session fixation attack problem. So, is the beaker documentation clear or should I use session.secret eventually?

Javex commented 11 years ago

To load a cookie beaker knows two classes: SimpleCookie and a their special SignedCookie. Now, if you use the normal session, i.e. not a pure cookie one, then it uses the secret to validate the cookie signature. Otherwise it uses the validate_key. In your case you chose a combination of validate_key and the normal server-side session.

I think this is a security flaw with beaker: It should not be that the two different types use different parameters for their signature. Instead, both should either use the secret or validate_key for the validation of a signed cookie. But even if this can't be fixed for compatibility reasons, then beaker needs to be clear on this: The note telling you to drop the secret refer specifically to the encrypted cookie which seems to be only relevant to client-side cookies.

robertlagrant commented 9 years ago

This is still in the latest docs; was it fixed?