| Message ID | YRASs0QxlMc4hho5@phare.normalesup.org |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,fateserver] Cleanup and security strengthening | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure | warning | Failed to apply patch |
Nicolas George (12021-08-08): > Here is a patch series for fateserver, to fix warnings and enable Perl's > taint checks, thus protecting against a whole class of security issues. I would appreciate somebody looks at the code before I deploy it and we re-enable the server. Regards,
On Sun, Aug 15, 2021 at 11:24:47AM +0200, Nicolas George wrote: > Nicolas George (12021-08-08): > > Here is a patch series for fateserver, to fix warnings and enable Perl's > > taint checks, thus protecting against a whole class of security issues. > > I would appreciate somebody looks at the code before I deploy it and we > re-enable the server. noone in the whole community cares about the server or everyone trusts you to never make a mistake. At least when limited to people knowing perl and having time. seriously, if someone has time and knows perl, please look over this, even if by the time you do, this has already been applied. The sooner someone goes over this the sooner the fateserver is online again Thanks [...]
On 8/22/2021 11:18 AM, Michael Niedermayer wrote: > On Sun, Aug 15, 2021 at 11:24:47AM +0200, Nicolas George wrote: >> Nicolas George (12021-08-08): >>> Here is a patch series for fateserver, to fix warnings and enable Perl's >>> taint checks, thus protecting against a whole class of security issues. >> >> I would appreciate somebody looks at the code before I deploy it and we >> re-enable the server. > > noone in the whole community cares about the server or everyone trusts you > to never make a mistake. At least when limited to people knowing perl and > having time. > > seriously, if someone has time and knows perl, please look over this, > even if by the time you do, this has already been applied. > > The sooner someone goes over this the sooner the fateserver is online > again It mostly looks good (from a perl perspective). Just 3 questionable items.. -<>-<>- -if ($ENV{HTTP_ACCEPT_ENCODING} =~ /gzip/) { - print "Content-Encoding: gzip\r\n"; +if (ready_for_gzip) { $cat = 'cat'; } The old code outputs "\r\n", where ready_for_gzip() outputs "\r\n\r\n". Will this be an issue, or should it have done that in the first place? -<>-<>- +sub ready_for_gzip() { + my $ae = $ENV{HTTP_ACCEPT_ENCODING}; + if (defined($ae) && $ae =~ /gzip/) { It is checking for 'gzip' as a substring, rather than a whole word. If it was passed a [hypothetical] encoding type contains the substring gzip (e.g. "bigzip"), it could trigger in incompatible output encoding. However, it's not any worse than it was previously. Perhaps changing it to match /\bgzip\b/ ? -<>-<>- sub ready_for_gzip() { + # Under CGI, $PATH is safe + ($ENV{PATH}) = $ENV{PATH} =~ /(.*)/s; It is untainting the PATH as "hidden" side effect of calling ready_for_gzip(). While technically it works, it feels a little kludgy compared to untainting it at the beginning of each taint-enabled script.
On Sun, Aug 22, 2021 at 01:35:26PM -0700, Chad Fraleigh wrote: > On 8/22/2021 11:18 AM, Michael Niedermayer wrote: > > On Sun, Aug 15, 2021 at 11:24:47AM +0200, Nicolas George wrote: > > > Nicolas George (12021-08-08): > > > > Here is a patch series for fateserver, to fix warnings and enable Perl's > > > > taint checks, thus protecting against a whole class of security issues. > > > > > > I would appreciate somebody looks at the code before I deploy it and we > > > re-enable the server. > > > > noone in the whole community cares about the server or everyone trusts you > > to never make a mistake. At least when limited to people knowing perl and > > having time. > > > > seriously, if someone has time and knows perl, please look over this, > > even if by the time you do, this has already been applied. > > > > The sooner someone goes over this the sooner the fateserver is online > > again > > It mostly looks good (from a perl perspective). > > Just 3 questionable items.. Thanks for the review! [...]
Chad Fraleigh (12021-08-22): > It mostly looks good (from a perl perspective). Thanks for the comments. > > Just 3 questionable items.. > > -<>-<>- > > -if ($ENV{HTTP_ACCEPT_ENCODING} =~ /gzip/) { > - print "Content-Encoding: gzip\r\n"; > +if (ready_for_gzip) { > $cat = 'cat'; > } > > The old code outputs "\r\n", where ready_for_gzip() outputs "\r\n\r\n". Will > this be an issue, or should it have done that in the first place? There is a “-print "\r\n";” just a little below, that balances out. > > -<>-<>- > > +sub ready_for_gzip() { > + my $ae = $ENV{HTTP_ACCEPT_ENCODING}; > + if (defined($ae) && $ae =~ /gzip/) { > > It is checking for 'gzip' as a substring, rather than a whole word. If it > was passed a [hypothetical] encoding type contains the substring gzip (e.g. > "bigzip"), it could trigger in incompatible output encoding. However, it's > not any worse than it was previously. > > Perhaps changing it to match /\bgzip\b/ ? I wanted minimalistic changes, but this is better. > > -<>-<>- > > sub ready_for_gzip() { > + # Under CGI, $PATH is safe > + ($ENV{PATH}) = $ENV{PATH} =~ /(.*)/s; > > It is untainting the PATH as "hidden" side effect of calling > ready_for_gzip(). While technically it works, it feels a little kludgy > compared to untainting it at the beginning of each taint-enabled script. You are right. Changed. I will try to deploy the changes this shortly. Regards,
Nicolas George (12021-08-23):
> I will try to deploy the changes this shortly.
To deploy the changes cleanly, I would need write access to the
fateserver Git repository. I already have access to the FATE server
itself, but not the public repository.
Thanks in advance.
Nicolas George (12021-08-23): > To deploy the changes cleanly, I would need write access to the > fateserver Git repository. I already have access to the FATE server > itself, but not the public repository. Any objection to me doing: New release '20.04.2 LTS' available. Run 'do-release-upgrade' to upgrade to it. before deploying? At lease we will be good for two more years. Regards,
On 8/23/2021 9:20 AM, Nicolas George wrote: > Nicolas George (12021-08-23): >> To deploy the changes cleanly, I would need write access to the >> fateserver Git repository. I already have access to the FATE server >> itself, but not the public repository. > > Any objection to me doing: > > New release '20.04.2 LTS' available. > Run 'do-release-upgrade' to upgrade to it. > > before deploying? At lease we will be good for two more years. Assuming it's from 20.04.x to 20.04.2, it should be fine. But shouldn't this kind of thing be done by the system admin? Is there one? > > Regards, > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
James Almer (12021-08-23): > Assuming it's from 20.04.x to 20.04.2, it should be fine. But shouldn't this It is a bit older than that: Welcome to Ubuntu 18.04.5 LTS (GNU/Linux 4.15.0-151-generic x86_64) That makes it all the more necessary but also all the more risky. > kind of thing be done by the system admin? Is there one? I am not sure, I am prone to understand there is not really one. I hope at least there is a backup of the data. Regards,
On 8/23/2021 9:51 AM, Nicolas George wrote: > James Almer (12021-08-23): >> Assuming it's from 20.04.x to 20.04.2, it should be fine. But shouldn't this > > It is a bit older than that: > > Welcome to Ubuntu 18.04.5 LTS (GNU/Linux 4.15.0-151-generic x86_64) > > That makes it all the more necessary but also all the more risky. > >> kind of thing be done by the system admin? Is there one? > > I am not sure, I am prone to understand there is not really one. > > I hope at least there is a backup of the data. > > Regards, An upgrade from 18.04 to 20.04 is riskier than a simple point release upgrade, so yes, a backup is the bare minimum we should have before we attempt something like it.
On Mon, Aug 23, 2021 at 02:18:03PM +0200, Nicolas George wrote: > Nicolas George (12021-08-23): > > I will try to deploy the changes this shortly. > > To deploy the changes cleanly, I would need write access to the > fateserver Git repository. I already have access to the FATE server > itself, but not the public repository. you should have write access to git@git.ffmpeg.org:fateserver i see this key there: 4096 SHA256:JMjmh7/1eynBVRAprwk96+NfcFjmla4v/G/IlwVuuxQ cigaes@ssecem (RSA) Thx [...]
Michael Niedermayer (12021-08-23): > you should have write access to git@git.ffmpeg.org:fateserver > i see this key there: > 4096 SHA256:JMjmh7/1eynBVRAprwk96+NfcFjmla4v/G/IlwVuuxQ cigaes@ssecem (RSA) ~/prog/fateserver $ git fetch origin-ssh ERROR:gitosis.serve.main:Repository read access denied fatal: Could not read from remote repository. I think the key gives me access to some repositories (the main one, of course), but another bit of configuration is necessary to give me access to this one. Regards,
On Mon, Aug 23, 2021 at 04:32:02PM +0200, Nicolas George wrote: > Michael Niedermayer (12021-08-23): > > you should have write access to git@git.ffmpeg.org:fateserver > > i see this key there: > > 4096 SHA256:JMjmh7/1eynBVRAprwk96+NfcFjmla4v/G/IlwVuuxQ cigaes@ssecem (RSA) > > ~/prog/fateserver $ git fetch origin-ssh > ERROR:gitosis.serve.main:Repository read access denied > fatal: Could not read from remote repository. Please make sure you use git@git.ffmpeg.org:fateserver not git@source.ffmpeg.org:fateserver thx [...]
Michael Niedermayer (12021-08-23): > Please make sure you use git@git.ffmpeg.org:fateserver not > git@source.ffmpeg.org:fateserver My bad. I fixed it and it worked. Thanks. Regards,
> On Aug 23, 2021, at 17:41, Nicolas George <george@nsup.org> wrote: > > Michael Niedermayer (12021-08-23): >> Please make sure you use git@git.ffmpeg.org:fateserver not >> git@source.ffmpeg.org:fateserver > > My bad. I fixed it and it worked. Thanks for looking into these issues and bringing the fate site back online. However, currently when viewing the main listing page, http://fate.ffmpeg.org, the page is truncated at the point of the first test report that contained errors. This seems like a regression to me, although it’s probably been a little while since I last fetched the full listing before the downtime too. // Martin
On Tue, Aug 31, 2021 at 10:44:57PM +0300, Martin Storsjö wrote: > > On Aug 23, 2021, at 17:41, Nicolas George <george@nsup.org> wrote: > > > > Michael Niedermayer (12021-08-23): > >> Please make sure you use git@git.ffmpeg.org:fateserver not > >> git@source.ffmpeg.org:fateserver > > > > My bad. I fixed it and it worked. > > Thanks for looking into these issues and bringing the fate site back online. > > However, currently when viewing the main listing page, http://fate.ffmpeg.org, the page is truncated at the point of the first test report that contained errors. This seems like a regression to me, although it’s probably been a little while since I last fetched the full listing before the downtime too. you can use http://fatebeta.ffmpeg.org/ which is timothys rewrite of fate it doesnt use the perl code and thus should be free of both the security issues as well as any new bugs. Sadly its unmaintainted too AFAIK thx [...]
diff --git a/FATE.pm b/FATE.pm index 50b5c69..27bd960 100644 --- a/FATE.pm +++ b/FATE.pm @@ -28,9 +28,9 @@ BEGIN { @EXPORT = qw/split_header split_config split_rec parse_date agestr split_stats load_summary load_report load_lastpass start end tag h1 span trow trowa trowh th td anchor - head1 head2 head3 footer - fail $fatedir $recent_age $ancient_age $hidden_age href - $gitweb/; + head1 head2 head3 footer href + fail + $fatedir $recent_age $ancient_age $hidden_age $gitweb/; } our $fatedir = "/var/www/fateweb";