[Pkg-mailman-hackers] [Mailman-cabal] Potential security flaw in Postorius

Mon Dec 25 21:44:46 UTC 2017

(Adding debian maintainers to the CC list)

There was a vulnerability found in Postorius (all released versions) which would set a users password to their Name (display_name attribute of Django User object) in Mailman Core.

Note that Core and Django have separate notion of users and passwords and are loosely coupled using their email addresses only. When a user signs up in Django, a corresponding user record may be created in Core, with the email and display name of the user. However, this bug caused the display name to be None and password be set to display name in Core.

Currently, there are no use cases of a user's password in Core. When using only Postorius, users aren't even aware of this password. However, it may find a use case sometime in future, because of which, I have added a new command in Postorius to reset those passwords. This command should be run in the upgrade process.

     $ python manage.py reset_passwords

The patch also includes the fix for the vulnerability.

thanks,
Abhilash

On Mon, Dec 25, 2017, at 2:24 AM, Abhilash Raj wrote:
> On Sun, Dec 24, 2017, at 7:49 PM, Stephen J. Turnbull wrote:
> > Abhilash Raj writes:
> > 
> >  > I have created a fix locally (patch added to this email), which
> >  > sets the password as None when creating users from Postorius. When
> >  > Core gets a None for password in REST, it creates a random password
> >  > for the user (this is the current behavior, so we don't need to
> >  > change anything there).
> > 
> > That sounds good to me.
> > 
> >  > I also write a script[1] that would iterate over all existing user
> > 
> > "wrote" (but maybe that's a typo?)
> 
> Yeah, that was a typo. I changed sentences in my head and forgot to re-
> write entire thing ;)
> 
> > 
> >  > records and reset their passwords to a random value (generated
> >  > using secrets module).
> >  > 
> >  > Any suggestions on the patch or script would be awesome!
> > 
> > The script should run (with confirmation) in the upgrade procedure.
> > The notice should emphasize that this is a serious problem, and that
> > if these passwords are used by users instead of social auth they can
> > be recovered in the usual way.
> 
> We aren't setting the password in Postorius to display_name, it's the 
> password that is maintained by Core. If it was Postorius' password, it 
> would have been caught easily as the user wouldn't be able to login with 
> their usual password.
> 
> Core and Postorius maintain separate passwords for authentication in 
> their own databases. Postorius uses allauth (by default), so those 
> passwords that are _actually_ useful are not affected by this.
> 
> The passwords for every user in Core, which has much less use cases 
> (none that I can remember right now), is what is set to display name of 
> then user when creating a user record in Core by Postorius.
> 
> > In fact, I would say that since the bad values are known (the users'
> > display names), can the script check for password == display name?  If
> > that's possible, the "reset all" mode should just check for the bad
> > case.  Yes, it's possible the user changed their display name and it's
> > still set to the old one, but to exploit that would require a stalker,
> > and I doubt Mailman 3 accounts would be of that great interest or use
> > to a stalker.
> > 
> > Perhaps all users with admin privileges should be reset regardless.
> > I'm not sure of the equities in that case (the main issue I can think
> > of is the possibility than an attacker has changed the primary mail
> > address), but of course they're the obviously valuable targets to
> > riff-raff.
> 
> It is possible to check for passwords that are set to display_name, I 
> guess if I can find what exact function is used to encrypt the 
> passwords.
> 
> But, I think that would be an overkill for now. Right now, since that 
> password isn't used, I think we can just reset it to a random value so 
> that it isn't exploitable in future. 
> 
> It is not even possible to change this from anywhere in Postorius right now.
> 
> >  > I am not sure about the exact procedure to follow to disclose this
> > a > issue, do we put out a release and then send an announcement to
> >  > mailman-users/mailman-announce list? Also, should we co-ordinate
> >  > with debian-maintainers to get the fix in debian packages before we
> >  > make an announcement?
> > 
> > Barry and Mark would be authoritative on how we've done this in the
> > past, I guess.  My preferences (somewhat informed by 20 years of
> > watching security announcements in several venues):  We should
> > coordinate with any known downstream package maintainers directly or
> > perhaps through security channels (Debian, Fedora are the ones that
> > seem likely).  Then we put out a release.  The script maybe should be
> > packaged as a wheel on PyPI as well as be added to the main distribution.
> > 
> >  > [1]: 
> > https://gitlab.com/maxking/mailman-scripts/blob/master/reset-passwords/reset-user-passwords.py
> > 
> > This looks like a useful utility in its own right.  How about adding
> > it to the distribution as such, while the default to requiring a user
> > and a password, defaulting the password to something random with a
> > --generate-password, and doing the iteration over all users (or users
> > with password == display name) when --reset-all-passwords is present?
> 
> I can add this as a django command to postorius, so debian/other 
> packages can run it as a part of the upgrade process with confirmation.
> 
> --generate-random isn't quite needed as setting the password to None 
> (which is what my patch does, for any new user), would set it to a 
> random value by default.
> 
>      $ python manage.py reset-mailman-passwords
> 
> is how the command would look like.
> 
> > 
> > An alternative, more paranoid UI for the default "reset on user
> > request" mode would be to always generate the password, but to require
> > that the ownership be confirmed by providing the primary email
> > address.  If that doesn't match the user's Mailman User (which seems
> > likely given the frequency with which users complain that they can't
> > unsubscribe because they don't know the subscribed address!) further
> > communication between admin and user would be necessary.  What I'm
> > worrying about here is that somebody has already gone into the user
> > account and short-circuited the normal password reset process by
> > changing the primary email.  Again I don't claim to fully understand
> > the equities here, just brainstorming.
> 
> I don't think there is any way to hijack a user's Postorius (Django) 
> account since the login session is based on password set in Postorius, 
> or social auth.
> 
> The only thing I can think of is that *if* the password is used for 
> something like "Approved" header, a user can use it bypass moderation if 
> they can get Admin's password.
> 
> > 
> > diff --git a/src/postorius/models.py b/src/postorius/models.py
> > 
> > -        return self.create(user.email, user.get_full_name())
> > +        return self.create(
> > +            email=user.email, password=None, display_name=user.get_full_name())
> >  
> > Using keyword arguments makes things clear.
> > 
> > Perhaps any function that touches user credentials and other sensitive
> > stuff based on function arguments should be given a keyword-only API.
> 
> Is there any way to enforce "call by keyword arguments" for a function 
> in Python?
> 
>       def func(**kwargs):
>          user = kwargs.get('username')
>          password = kwargs.get('password')
> 
> This might work I guess? What do you think?
> 
> 
> I will go ahead and add the password-reset script to Postorius as a 
> command. I will also go ahead and add the debian maintainers to this 
> thread, don't think there are any other downstream packages yet except 
> container images (which I will updated as soon as we have a fix).
> 
> 
> -- 
> thanks,
> Abhilash Raj
> _______________________________________________
> Mailman-cabal mailing list
> Mailman-cabal at python.org
> https://mail.python.org/mailman/listinfo/mailman-cabal

-- 
thanks,
Abhilash Raj
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-create_user.patch
Type: text/x-patch
Size: 3365 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-mailman-hackers/attachments/20171225/1109320f/attachment.bin>