Hi!
I'd like to discuss an issue about security bugs handling.
We have a security repo which I and others check into bugs from time to
time. The idea is for these to be reviewed by people having access there
before we merge them, and then merge after the release.
This, however, is not happening at all. The patches, as far as I know,
are not reviewed at all, and merging a bunch of patches last minute with
no review is extremely dangerous. I am trying my best with my patches,
but I'm only human, and I feel increasingly uncomfortable having so many
unreviewed patches in the release.
So, how we can fix it?
a. We could merge some of the patches on RC stage, even though that
might expose some issues.
b. We could somehow improve review mechanism beyond security repo we
have now - ideas?
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?
Would like to hear thoughts on this one.
Stas Malyshev
smalyshev@gmail.com
Hi Stas,
Thanks for bringing this up.
-----Original Message-----
From: Stanislav Malyshev [mailto:smalyshev@gmail.com]
Sent: Monday, October 24, 2016 7:16 AM
To: PHP Internals internals@lists.php.net
Subject: [PHP-DEV] Security issue handlingHi!
I'd like to discuss an issue about security bugs handling.
We have a security repo which I and others check into bugs from time to
time. The idea is for these to be reviewed by people having access there
before we merge them, and then merge after the release.This, however, is not happening at all. The patches, as far as I know,
are not reviewed at all, and merging a bunch of patches last minute with
no review is extremely dangerous. I am trying my best with my patches,
but I'm only human, and I feel increasingly uncomfortable having so many
unreviewed patches in the release.So, how we can fix it?
a. We could merge some of the patches on RC stage, even though that
might expose some issues.
Related to my response in another bug, we could indeed make definitions and partially merge the low severity patches already into RC. Also, with the high severity patches - it'd make sense to define some time range where new patches are acceptable for the next release. Say, as we do it now, we tag two days before. It could be defined, for example, that any security patches intended for release inclusion, have to be merged into serucity repo, ported and tested 5 days before tag. Fe Thursday/Friday in week before final, it is required to have all the security patches prepared. Until some urgent patch were already disclosed, so it would have to be applied right at the last second, having more buffer will lead to more stability.
b. We could somehow improve review mechanism beyond security repo we
have now - ideas?
IMHO it were the best option, indeed building a security response team. We need more people on this.
Maybe, it'd make sense also to have a finer privileges system on BT. Fe, several people could be allowed to review some selected tickets on BT, without having the actual security repo access. Say, any security team participant would be able to assign some new reviewer to a particular ticket.
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?
OFC it'd be ideal to have some karma holders to participate. And another option, which is IMHO eligible - we could invite several reporters. There is already a couple of people, who regularly report security issues and keep them confident until they're publicly disclosed. IMHO it is a good base for trust.
Also, having people even without iternals knowledge, that could just test, were useful as well. For that case, we could find more people even without php-src karma or access to the security repos. Providing some test binaries only wouldn't be that hard. In that case, while we couldn't disclose the actual security issues, the "blind testing" could have a positive effect in determining unforeseen issues, too.
On my side, I'm already intended to go deeper into the security patches at least a week before finals. Until it's defined otherwise, I'll be at least preparing the 5.6->7.0 ports and testing so we have some more time to discuss and more chance to avoid tag delays. I might have time to participate on security tickets, when 7.0 goes into security only mode, but unfortunately can't afford full security team participation ATM.
Regards
Anatol
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?OFC it'd be ideal to have some karma holders to participate. And another
option, which is IMHO eligible - we could invite several reporters. There
is already a couple of people, who regularly report security issues and
keep them confident until they're publicly disclosed. IMHO it is a good
base for trust.
Yes, in the end this is about getting Stas some help here. He has been
doing an incredible job for years now handling all these annoying
off-by-one and >2gb string bugs. I occasionally read through the patches,
but I haven't been doing it consistently and even though there are a few
other people on security@ who occasionally look through the patches, it
obviously isn't enough.
As a first step perhaps we just need to expand security@ a bit with the
specific call for volunteers to help review security patches?
-Rasmus
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?OFC it'd be ideal to have some karma holders to participate. And another
option, which is IMHO eligible - we could invite several reporters. There
is already a couple of people, who regularly report security issues and
keep them confident until they're publicly disclosed. IMHO it is a good
base for trust.Yes, in the end this is about getting Stas some help here. He has been
doing an incredible job for years now handling all these annoying
off-by-one and >2gb string bugs. I occasionally read through the patches,
but I haven't been doing it consistently and even though there are a few
other people on security@ who occasionally look through the patches, it
obviously isn't enough.As a first step perhaps we just need to expand security@ a bit with the
specific call for volunteers to help review security patches?
I would be happy to help with review / fixes especially for json that I
maintain and openssl that I sort of try to maintain too. But I could also
help with review of some other exts if time allows.
Cheers
Jakub
Hi!
I would be happy to help with review / fixes especially for json that I
maintain and openssl that I sort of try to maintain too. But I could
also help with review of some other exts if time allows.
Great, thanks! So besides assigning the issues for the said extensions
to you, what model for coordinating reviews would you prefer?
--
Stas Malyshev
smalyshev@gmail.com
Hi
On Sun, Oct 30, 2016 at 10:09 PM, Stanislav Malyshev smalyshev@gmail.com
wrote:
Great, thanks! So besides assigning the issues for the said extensions
to you, what model for coordinating reviews would you prefer?
I'm not sure what the current flow is but it would be great to send info
about fixed issues (e.g. patches ready for review or link on the fix in the
security repo) to sec mailing list. Then it should be easier to do the
review.
If someone could add me to the security mailing list, security repo and
possibly access to the security bugs, that would be great - it would allow
me to do the reviews or fix some of the issues.
Cheers
Jakub
Hi all,
On Sun, Oct 30, 2016 at 10:09 PM, Stanislav Malyshev smalyshev@gmail.com
wrote:Great, thanks! So besides assigning the issues for the said extensions
to you, what model for coordinating reviews would you prefer?I'm not sure what the current flow is but it would be great to send info
about fixed issues (e.g. patches ready for review or link on the fix in the
security repo) to sec mailing list. Then it should be easier to do the
review.If someone could add me to the security mailing list, security repo and
possibly access to the security bugs, that would be great - it would allow
me to do the reviews or fix some of the issues.
IMHO, assuming active developers are trustworthy, the more is better.
How about to add active developers? When someone became inactive,
then remove karama. This task would be once a year task.
Regards,
--
Yasuo Ohgaki
yohgaki@ohgaki.net
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?OFC it'd be ideal to have some karma holders to participate. And another
option, which is IMHO eligible - we could invite several reporters. There
is already a couple of people, who regularly report security issues and
keep them confident until they're publicly disclosed. IMHO it is a good
base for trust.Yes, in the end this is about getting Stas some help here. He has been
doing an incredible job for years now handling all these annoying
off-by-one and >2gb string bugs. I occasionally read through the patches,
but I haven't been doing it consistently and even though there are a few
other people on security@ who occasionally look through the patches, it
obviously isn't enough.As a first step perhaps we just need to expand security@ a bit with the
specific call for volunteers to help review security patches?
I'm gladly willing to help with GD related security issues (at least).
It's also okay for me to get assigned to these, when Pierre is busy. :-)
--
Christoph M. Becker
2016-10-24 17:19 GMT+02:00 Rasmus Lerdorf rasmus@lerdorf.com:
As a first step perhaps we just need to expand security@ a bit with the
specific call for volunteers to help review security patches?
Maybe we should make the security issues available to those who
actively contributes to PHP, like Jakub, Christoph who both replied,
Yasuo for session stuff (I'm sure he is interested) and others who are
apart of the development team with regular commits. RMs already are on
the security list.
--
regards,
Kalle Sommer Nielsen
kalle@php.net
Hi!
release. Say, as we do it now, we tag two days before. It could be
defined, for example, that any security patches intended for release
inclusion, have to be merged into security repo, ported and tested 5
days before tag. Fe Thursday/Friday in week before final, it is
That's nice but most patches right now are in security repo long before,
and still nobody reviews them. Also, the reality is that I mostly work
on patches on weekends, so that means either any of the patches I work
on one of the 4 weekends can't be merged for a month or we need a better
model.
Maybe, it'd make sense also to have a finer privileges system on BT.
Fe, several people could be allowed to review some selected tickets
on BT, without having the actual security repo access. Say, any
security team participant would be able to assign some new reviewer
to a particular ticket.
That probably can be done, but let's say it's done now. Who I would be
assigning to the new issue? I have no idea who wants/can review it.
OFC it'd be ideal to have some karma holders to participate. And
another option, which is IMHO eligible - we could invite several
reporters. There is already a couple of people, who regularly report
security issues and keep them confident until they're publicly
disclosed. IMHO it is a good base for trust.
The reporter is asked to review the patch anyway. I don't think I feel
comfortable inviting other reporters - unless I know who they are, which
right now is not true for most of them.
Also, having people even without iternals knowledge, that could just
test, were useful as well. For that case, we could find more people
even without php-src karma or access to the security repos. Providing
some test binaries only wouldn't be that hard. In that case, while we
couldn't disclose the actual security issues, the "blind testing"
could have a positive effect in determining unforeseen issues, too.
Hmm I don't know. I certainly won't have time to arrange binary builds
usable for people with no dev knowledge, but if somebody volunteers to
organize it, fine. But that again means a) disclosing security issues to
people before fix is available, so this should be people we know and b)
that should be people that want to commit substantial amount of time.
--
Stas Malyshev
smalyshev@gmail.com
Hi,
OFC it'd be ideal to have some karma holders to participate. And
another option, which is IMHO eligible - we could invite several
reporters. There is already a couple of people, who regularly report
security issues and keep them confident until they're publicly
disclosed. IMHO it is a good base for trust.The reporter is asked to review the patch anyway. I don't think I feel
comfortable inviting other reporters - unless I know who they are, which
right now is not true for most of them.
I'm gladly willing to help with some security issues if you need and want.
Hi Stas,
-----Original Message-----
From: Stanislav Malyshev [mailto:smalyshev@gmail.com]
Sent: Sunday, October 30, 2016 11:01 PM
To: Anatol Belski anatol.php@belski.net; 'PHP Internals'
internals@lists.php.net
Subject: Re: [PHP-DEV] Security issue handlingHi!
release. Say, as we do it now, we tag two days before. It could be
defined, for example, that any security patches intended for release
inclusion, have to be merged into security repo, ported and tested 5
days before tag. Fe Thursday/Friday in week before final, it isThat's nice but most patches right now are in security repo long before, and still
nobody reviews them. Also, the reality is that I mostly work on patches on
weekends, so that means either any of the patches I work on one of the 4
weekends can't be merged for a month or we need a better model.
That was the exact idea. I, at least, am going for ports a week before release, as mentioned. But even otherwise - having a cut a week before is what addresses the QA concerns in first place, that Remi and you also expressed. Say, if there is a vulnerability, which is not disclosed and present for years - while it's not good it'd go into release a month later, still it's better than a possible another bug introduced because of the last minute or unreviewed patch. It wouldn't of course affect a situation, where a fix is urgent.
Maybe, it'd make sense also to have a finer privileges system on BT.
Fe, several people could be allowed to review some selected tickets on
BT, without having the actual security repo access. Say, any security
team participant would be able to assign some new reviewer to a
particular ticket.That probably can be done, but let's say it's done now. Who I would be assigning
to the new issue? I have no idea who wants/can review it.
What I had in mind, in first place involving the karma holders and ext maintainers. Like Jakub and Christoph already expressed the willingness to do fixing regularly, other active core commiters or any other trusted person could be asked to help on demand. Of course, we don't know how it would work, until it's started to be done this way. It could be, that asking some random active and known contributor to review just one patch would be more effective, that asking the same person to do it regularly as in security team.
OFC it'd be ideal to have some karma holders to participate. And
another option, which is IMHO eligible - we could invite several
reporters. There is already a couple of people, who regularly report
security issues and keep them confident until they're publicly
disclosed. IMHO it is a good base for trust.The reporter is asked to review the patch anyway. I don't think I feel
comfortable inviting other reporters - unless I know who they are, which right
now is not true for most of them.Also, having people even without iternals knowledge, that could just
test, were useful as well. For that case, we could find more people
even without php-src karma or access to the security repos. Providing
some test binaries only wouldn't be that hard. In that case, while we
couldn't disclose the actual security issues, the "blind testing"
could have a positive effect in determining unforeseen issues, too.Hmm I don't know. I certainly won't have time to arrange binary builds usable
for people with no dev knowledge, but if somebody volunteers to organize it,
fine. But that again means a) disclosing security issues to people before fix is
available, so this should be people we know and b) that should be people that
want to commit substantial amount of time.
Just as a loud thought - when seeing same person makes authentic reports, using good tools, there's no premature disclosure, so it is a meaningful behavior. What i've mentioned earlier and plea for - it were great to have a constant security response team, some structure that allows to ensure fixes, reviews and QA on security patches regularly. It is absolutely clear, that it is much better to involve only people with the karma and contributing actively. But I've an impression, and see it on my example as well :), that people currently contributing on the constant frequency, have not necessarily interest or time on pure security patches handling. IMHO it indeed makes sense what Rasmus said, to send a dedicated call for security team participation and see what happens. And as a fallback, if no enough reaction is to see, check other ways to achieve more QA. With the binary only testing - it's likely that good people are to find in the PHP projects, and I could take on the builds but would require a machine where it could be done and shared.
Regards
Anatol
[…] And as a fallback, if no enough reaction is to see, check other
ways to achieve more QA. […]
Not directly related to this thread, but to QA in general: could
somebody please fix http://qa.php.net/reports/run_tests.php? The page
is down for months, and I'm not sure if there's another way to have a
look at test results sent by users – I'm not even sure whether these
tests results are stored anymore.
--
Christoph M. Becker
Hi!
I'd like to discuss an issue about security bugs handling.
We have a security repo which I and others check into bugs from time to
time. The idea is for these to be reviewed by people having access there
before we merge them, and then merge after the release.This, however, is not happening at all. The patches, as far as I know,
are not reviewed at all, and merging a bunch of patches last minute with
no review is extremely dangerous. I am trying my best with my patches,
but I'm only human, and I feel increasingly uncomfortable having so many
unreviewed patches in the release.So, how we can fix it?
a. We could merge some of the patches on RC stage, even though that
might expose some issues.
b. We could somehow improve review mechanism beyond security repo we
have now - ideas?
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?Would like to hear thoughts on this one.
Stas Malyshev
smalyshev@gmail.com
Hey Stas,
If it's extra volunteers that you need, I would also be happy to help
out where I can, investigating reported issues, writing and reviewing
patches.
- I have a provable interest in security
- I've submitted security issues (to PHP and other projects) in the past
- I have worked on security features for the PHP runtime in the past
- I already have karma \o/
Regards,
Leigh.
Morning,
Stas, consider Leigh vouched for, please add him to sec lists and private
bugs.
Cheers
Joe
On 24 October 2016 at 06:16, Stanislav Malyshev smalyshev@gmail.com
wrote:Hi!
I'd like to discuss an issue about security bugs handling.
We have a security repo which I and others check into bugs from time to
time. The idea is for these to be reviewed by people having access there
before we merge them, and then merge after the release.This, however, is not happening at all. The patches, as far as I know,
are not reviewed at all, and merging a bunch of patches last minute with
no review is extremely dangerous. I am trying my best with my patches,
but I'm only human, and I feel increasingly uncomfortable having so many
unreviewed patches in the release.So, how we can fix it?
a. We could merge some of the patches on RC stage, even though that
might expose some issues.
b. We could somehow improve review mechanism beyond security repo we
have now - ideas?
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?Would like to hear thoughts on this one.
Stas Malyshev
smalyshev@gmail.comHey Stas,
If it's extra volunteers that you need, I would also be happy to help
out where I can, investigating reported issues, writing and reviewing
patches.
- I have a provable interest in security
- I've submitted security issues (to PHP and other projects) in the past
- I have worked on security features for the PHP runtime in the past
- I already have karma \o/
Regards,
Leigh.
Morning,
Stas, consider Leigh vouched for, please add him to sec lists and private
bugs.
I've given him karma to look at the security (private) bugs.
cheers,
Derick
Hi,
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?
I'd be happy to help with reviewing and also setting up a private C.I.
to build and run the test suite regularly, if you think that's a good idea.
Cheers
Matteo Beccati
Development & Consulting - http://www.beccati.com/
Hi!
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?I'd be happy to help with reviewing and also setting up a private C.I.
to build and run the test suite regularly, if you think that's a good idea.
I think it's a great idea, how could this be done? I'd be happy to run
CI on security branch, it probably would remove 95% of the issues we had
with merges.
--
Stas Malyshev
smalyshev@gmail.com
Hi!
c. Get some specific people to volunteer to review patches in security
repo regularly - how? Any takers?I'd be happy to help with reviewing and also setting up a private C.I.
to build and run the test suite regularly, if you think that's a good
idea.I think it's a great idea, how could this be done? I'd be happy to run
CI on security branch, it probably would remove 95% of the issues we had
with merges.
Travis can be run in local docker containers, so given the infrastructure
to run the container, it shouldn't be a problem to use the existing travis
configuration for private CI
Hi,
-----Original Message-----
From: Stanislav Malyshev [mailto:smalyshev@gmail.com]
Sent: Saturday, November 5, 2016 8:13 PM
To: Matteo Beccati php@beccati.com; PHP Internals
internals@lists.php.net
Subject: Re: [PHP-DEV] Security issue handlingHi!
c. Get some specific people to volunteer to review patches in
security repo regularly - how? Any takers?I'd be happy to help with reviewing and also setting up a private C.I.
to build and run the test suite regularly, if you think that's a good idea.I think it's a great idea, how could this be done? I'd be happy to run CI on
security branch, it probably would remove 95% of the issues we had with merges.
At this point, what were our course of action? Seems there might be multiple tasks
- granting the willing devs security karma
- setting up a private CI
- organizing a security team
It probably would make sense, to make some plan on what is to be done, to come to the point.
Regards
Anatol
2016-11-10 0:43 GMT+01:00 Anatol Belski anatol.php@belski.net:
At this point, what were our course of action? Seems there might be multiple tasks
- granting the willing devs security karma
- setting up a private CI
- organizing a security team
It probably would make sense, to make some plan on what is to be done, to come to the point.
I'm also interested in this, it seems that we should add people to the
security ML, bugsweb (I can handle that part) and setup that private
CI and once everyone is on the security ML, mail out a link to the
private CI from there.
--
regards,
Kalle Sommer Nielsen
kalle@php.net