[development] Think there's a security problem in your module? Here's what to do.

[Greg Knaddison]

On Jan 17, 2008 3:08 AM, David Metzler <metzlerd at metzlerd.com> wrote:
>
> The drawback from my perspective is two fold:
>
> 1.  It definitely increases the amount of time the vulnerability is
> out there.

"Out there" is not important.  "Generally known without the fix
installed on people's sites" is important.  You need to coordinate
with the Security team, among other reasons, because the Security team
has their finger on the button for 1) Newsletter emails 2) RSS feed,
3) Update_status.  If the module owner commits the patch and announces
it on the module home page the result is that basically nobody will
know.  Once the security team uses those three methods to alert
people, there are at least 15,000 people who know (mailing list
subscribers) and probably several times that number in terms of RSS
and Update Status users.

> Vulnerabilities will be patched more slowly when they
> ONLY can be commited by the security team, who is a finite group of
> volunteer bodies. Who may or may not have the expertise to properly
> review the code, but are put in the process as if they are. This is
> not a criticism of the dedication or expertise of this wonderful
> group of people, but just a reflection of the diversity and
> complexity of all the contrib modules out there.

Sure, the insertion of the security team slows down the "commit" part
of the process, but that is not the important part of the process.
Most of the waiting that happens is due to the fact that we make
security releases in bundles.  This policy of bundling patches was
instituted to make it easier for site admins to update their site.  I
know that it has made my life easier, but we are certainly open to
feedback on this point and others.

> For example, I
> maintain a CAS single sign-on module.   Do I have to wait for a
> security team member to ramp up on what CAS is so that they can
> perform the review tasks that are inherent there to do a decent job
> of reviewing the code?

Well, in fact there is no way for the security team to prevent people
from committing code to CVS that fixes security holes - that's part of
why we're sticking with this thread to convince people that it is a
bad practice that unnecessarily puts their end users at risk.

> 2.  It seems to take the RTBC decision out of the contrib module
> owners  hands.   When is the patch tested enough? Who decides this?
> All of the handshaking and discussion regarding this with the
> security team adds to the time that the vulnerability is in circulation.

In practice this process is quite friendly. Most of the time the
module author says "here is a patch that I think is ready" and the
security team says "yeah, looks great to us to."  We rely on the
module authors to ensure functionality of the patch and are mostly
looking to see that their fix actually closes the hole and that there
are no other holes.

> When I discovered a vulnerability in my CAS module, I was in
> discussion with other maintainers, off-line of course, and not with
> the security team who had other more productive tasks to be engaged
> in.  We were the best people to make the decision about when the code
> was RTBC.  And whether other bug fixes should be included in the
> Release for the purposes of stability. Other bug fixes were included
> with the relaase that we made.

I see - this was your release:
http://drupal.org/node/153707

Which never got an SA, right?  This module is in a somewhat abnormal
place in terms of policy because I don't imagine it is used on
thousands of sites, but the fact that it didn't have an Email/RSS
announcement means that relatively few people know about it.  There
could be someone who is using this module who signed up to the
security newsletter to hear about updates and is still assuming that
this module is totally secure because they never got an SA.  I think
that's a good example of why the security team needs to be involved.

> I also agree with DragonWize's assessment that decoupling the SA/RA
> from the commit will make it harder (especially if you factor in
> other bug fixes that might be included in that release)for you to
> theoretically detect a vulnerability by sniffing commit logs.

As Drewish points out, jamming a bunch of things into a release at the
last minute is probably not a good idea in terms of stability.  It's
also considered best practice (discussed on this list either one year
ago or two years ago) to make sure your commits contain one change
each.  This makes it easier to review/roll-back later.

And again, the most important thing is that we put site admins on
equal footing with the hackers in the race to update sites before an
exploit/worm is created.  The best way to do that is to be very very
quiet about the fix right up until it's done and then tell the whole
world about it as loudly as possible at the same time.

Now, what about an alternate workflow.  I've been insisting that
"because the security team has the ability to send the SA email/feed
that the security team needs to be involved in the process."  What if
we had contrib authors publish their own SAs and what if we gave them
the ability to create releases that were tagged with "security" that
got published immediately.  Would this help or hurt the process?
Frankly, it would remove a lot of work from the security team.  We
could say "the security team can help you with this process if you
want."

Thoughts?

Greg

-- 
Greg Knaddison
Denver, CO | http://knaddison.com
World Spanish Tour | http://wanderlusting.org/user/greg