Not that it will matter in the long scheme of things but in case the security team is interested in feedback , I'm generally in agreement with DragonWize here. I'm not in favor of holding up commits to code. The only benefit is predicated on the fact that you can tell a security vulnerability from a bug fix based on the code that you see fly by. I hope my security bugs aren't all that obvious, and as Derek says, this is all conjecture. We don't actually know wether they'd discover it from the commt logs. The drawback from my perspective is two fold: 1. It definitely increases the amount of time the vulnerability is out there. 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. 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? 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. 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 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. Anyway, won't harp, but wanted others to know that it's more than just DragonWize that sees this perspective. On Jan 16, 2008, at 1:36 PM, DragonWize wrote:
Thank you Derek for your explanations. You have said a lot but let me focus on the area where I think we are diverging. I agree with most of what you are doing and understand you explanations. I just don't fully agree with some of them.
Lets say for the sake of understanding that everything you say is 100% correct. Lets assume to be on the safe side that every hacker reads every line of code that comes through the cvs log. That hacker will know long before a fix is committed because he will know when the hole was committed. So not committing the fix does nothing to help this doomsday scenario. This is the part that I don't understand.
Either way whether code is committed I don't see the benefit you describe in the race to ensure sites are secure. The race is definitely important, I do not deny that, I just don't understand how your method helps that race. I see 3 possible out comes:
1. The hacker already knows when the SA is sent. No benefit gained.
2. The hacker doesn't know when the SA is sent but we have already told him exactly where the hole is by saying we commit at the same time. Yes they could find it extremely easily with out this but we just made it slightly easier.
3. The hacker doesn't know when the SA is sent but we haven't made a policy about when exactly the code is committed. Can they very easily find it anyway? For sure they can, I don't deny that. I also am not sugesting remove this step in your short simple guide because it provides better security. I am saying that I don't see better in security by including it so why make the process harder to understand and remember if it doesn't provide a benefit.
Thank you for your time, Alan
On 1/16/08, Derek Wright <drupal@dwwright.net> wrote:
On Jan 16, 2008, at 9:13 AM, DragonWize wrote:
3. Unless you quietly found the hole by yourself it probably has been published somewhere (issue queue, etc).
If everyone was following instructions, they'd report the hole to security@drupal.org, not the issue queue. Whenever we find security problems reported publicly in the issue queue (which sucks, but it does happen), we try to immediately unpublish the issue and move discussion back into the security team's issue tracker. Of course, it's often too late at that point (people already got emails about it if they're subscribed to the queue, someone might have already seen it, etc, but we do the best we can...
4. committing code give you and others the chance to fix the issue with out publishing the code.
I can't parse what you mean here. I'm not sure it matters, since it smells like security through obscurity to me, but perhaps you could clarify your point? How is committing the code not "publishing the code"?
Oh, and one more good reason not to just go off and commit your patch as soon as you think you fixed the problem...
The security team carefully reviews your patch, and usually audits the rest of your module at the same time. Maybe you fixed 1 hole, but missed 3 others. Maybe your "fix" is still vulnerable to some case you're not thinking of. Maybe your "fix" introduces a bug or otherwise makes life miserable for users trying to upgrade. Who knows. Point is, you want to wait for the security team to review your patch, audit your code, and propose improvements to your solution (if there are any to be made). All of this should happen privately, between you and the security team, not publicly via a stream of CVS commits.
Make sense?
Thanks, -Derek (dww)
-- Alan Doucette Koi Technology, LLC www.KoiTech.net