Modify

Opened 8 years ago

Closed 7 years ago

#1011 closed enhancement (wontfix)

Authentication Form cookie verification

Reported by: prunille Owned by: mgood
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: Form Auth Input
Cc: freesoitj@… Trac Release: 0.10

Description

I try to set up a SSO front of Trac and it is no real SSO but only credentials replay with Vulture NG. The system submits the received cookie and some inputs but not all since I believe it can not send dynamic hidden inputs.

Can someone explain why dynamic hidden inputs need to be inserted in the authent form?

Regards,
Prunille

Attachments (0)

Change History (7)

comment:1 Changed 8 years ago by coderanger

This is part of the CSRF protection introduced in 0.10.2. All POSTs must be accompanied by a token which is verified by the server.

comment:2 Changed 8 years ago by prunille

Thanks a lot. I didn't know about this one! I guess this is especially bad in a SSO environnement. But I still don't understand how the token can prevent CRSF queries.

If the malicious code go directly to the POST, it doesn't get the cookie. If the script can get the form first, it has the cookie and the input. It can send them both.

So, I would think the cookie is enough to prevent direct access to the POST and input is not enough to prevent anything more since the script must had done a GET first to get the cookie. It is then possible to get the inputs as well.

Anyway, do you know a mean to go over?

comment:3 Changed 8 years ago by coderanger

TracForge already implements SSO for Trac, have you tried using that. Depending on your exact needs it might be a good start. If you have further questions you can usually find me on #trac.

comment:4 Changed 8 years ago by prunille

  • Type changed from defect to enhancement

No, I hadn't even had a look at TracForge but I may need it in the future. For now, I am only concerned with SSO between a trac environnement and some other applications like a webmail, a portal and so on... So my need is to use a product like vulture to spread the word to every system.

I changed the ticket as enhancement since you explained to me why this was done even if I don't completely understand (see my previous post). So the enhancement could be to give the admin a configuration option to enable or disable this checking.

Regards,

comment:5 Changed 8 years ago by mgood

A CSRF attack is done by a malicious Javascript posting data from the user's browser and exploits the fact that the browser does have the cookie it needs to authenticate. See trac:#4049 and the links it provides for an explanation of the issue.

I'm always reluctant to provide workarounds for security measures, so I'm not committed to committing this yet, but I believe this patch should enable bypassing the token checking if you add "disable_csrf_protection = true" under the "account-manager" section in trac.ini:

  • acct_mgr/web_ui.py

     
    1616 
    1717from trac import perm, util 
    1818from trac.core import * 
    19 from trac.config import IntOption 
     19from trac.config import IntOption, BoolOption 
    2020from trac.notification import NotificationSystem, NotifyEmail 
    21 from trac.web import auth 
    22 from trac.web.api import IAuthenticator 
    23 from trac.web.main import IRequestHandler 
     21from trac.web import auth, IAuthenticator, IRequestFilter, IRequestHandler 
    2422from trac.web.chrome import INavigationContributor, ITemplateProvider 
    2523from trac.util import Markup 
    2624 
     
    337335 
    338336class LoginModule(auth.LoginModule): 
    339337 
    340     implements(ITemplateProvider) 
     338    implements(ITemplateProvider, IRequestFilter) 
    341339 
     340    disable_csrf_protection = BoolOption('account-manager', 'disable_csrf_protection') 
     341 
     342    def pre_process_request(self, req, handler): 
     343        if handler is self and self.disable_csrf_protection: 
     344            req.args['__FORM_TOKEN'] = req.form_token 
     345        return handler 
     346 
     347    def post_process_request(self, template, content_type): 
     348        return (template, content_type) 
     349 
    342350    def authenticate(self, req): 
    343351        if req.method == 'POST' and req.path_info.startswith('/login'): 
    344352            req.environ['REMOTE_USER'] = self._remote_user(req) 

comment:6 Changed 8 years ago by coderanger

What you really want is not related to AccountManager. You want to make a new authentication plugin that works with Vulture. Vulture looks somewhat similar to CAS, so you could use the TracCasPlugin as a starting point.

comment:7 Changed 7 years ago by mgood

  • Resolution set to wontfix
  • Status changed from new to closed

Yes, I think I agree with coderanger here. You're welcome to patch your AccountManager if you want, but I don't think it's a good idea to hack around this in the official release.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from mgood. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.