Swoole's admin interface hot-loads code from a third-party server ?

[foremtehan]

Hey i just saw this comment :

heroku/heroku-buildpack-php#297

Word of warning: Swoole's admin interface hot-loads code from a third-party server; see

swoole-src/ext-src/php_swoole_library.h

Lines 7260 to 7283 in 1bc3917

	" if (!is_file($index_file)) {\n"
	" $url = 'https://business.swoole.com/static/swoole_dashboard/' . SWOOLE_VERSION . '.tar.gz';\n"
	" $download_request = Coroutine\\Http\\get($url);\n"
	" if (!$download_request or $download_request->getStatusCode() != '200') {\n"
	" $resp->end(\"download [{$url}] failed\");\n"
	" return;\n"
	" }\n"
	" $tmp_file = '/tmp/swoole_dashborad.' . SWOOLE_VERSION . '.tar.gz';\n"
	" file_put_contents($tmp_file, $download_request->getBody());\n"
	" if (!is_file($tmp_file) or filesize($tmp_file) === 0) {\n"
	" $resp->end(\"write [{$tmp_file}] failed\");\n"
	" return;\n"
	" }\n"
	" if (!is_dir($dir)) {\n"
	" mkdir($dir, 0777, true);\n"
	" }\n"
	" $sh = 'tar zxvf ' . $tmp_file . ' -C ' . $dir;\n"
	" System::exec($sh);\n"
	" $remote_addr = $req->server['remote_addr'];\n"
	" $server_port = $req->server['server_port'];\n"
	" $f = $dir . '/dist/js/app.js';\n"
	" $baseURL = 'baseURL:\"http://' . $remote_addr . ':' . $server_port . '/\"';\n"
	" file_put_contents($f, str_replace('baseURL:\"http://127.0.0.1:9502/\"', $baseURL, file_get_contents($f)));\n"
	" }\n"

https://pecl.php.net/package/openswoole looks to be a fork that wants to remediate that, but for now, this is a big no-no.

Can you elaborate what does exactly that part of code ?

[cmb69]

It seems this has been reverted: 0a86985

[matyhtf]

@foremtehan This code has been removed. The swoole dashboard is no longer provided in the current version.

The swoole dashboard is a swoole running visualization tool, we will be open sourced as an independent project in the future.

[matyhtf]

Demo video:
https://www.youtube.com/watch?v=AcQvI_g4Abs

[mnapoli]

@matyhtf Do you have any more details as to why it wasn't removed when reported initially by another Swoole maintainer? Or why that maintainer has been ejected from the project?

Source: https://news-web.php.net/php.pecl.dev/17446

After several times communicating with the 'matyhtf' who also has the Owner
permission of the git repository, he still doesn't want to remove the
security concern. Doing further my Owner permission has been removed from
the organization by matyhtf.

Did you remove the security issue only because it was reported publicly?

Any extra detail can help form a better opinion regarding what to think of Swoole and whether to trust the project in the future.

[deminy]

Some Background Information

1. Han ( @matyhtf ) started the Swoole project nine years ago. He has been leading and actively contributing to the project since then.
2. Besides contributing to the project, Bruce ( @doubaokun ) maintains the official website of Swoole, the Twitter account of Swoole, and the Slack channel of Swoole. He spends lots of effort supporting and collaborating with the Swoole community.

The New Admin Dashboard And The Security Concern

There are many great debugging, logging, and profiling tools for PHP, like Xdebug, New Relic, Blackfire, Bugsnag, etc. Unfortunately, most of them are not coroutine-friendly by default, which means they won't work directly with Swoole. Some Swoole users find code debugging/profiling is more complicated.

As the community grows, more debugging/profiling tools work with Swoole, like Apache SkyWalking, Zipkin, Prometheus, yasd, and Swoole Tracker (a free/commercial one). Meanwhile, the Swoole team is working on providing a free, open-source admin dashboard. The problem is how to deploy it. Different approaches were considered, like publishing it as a separate PHP extension, integrating it as part of Swoole, or making a cloud-based package. It's a big task, and the team chose to make it a could-based package first before merging the code to Swoole in the future. This introduced the security concern, where a Swoole-based server needs to download the package from a third-party website automatically before using it.

Preparing The Upcoming Release

Some conflicts between Han and Bruce arose when preparing release 4.8.0:

1. Bruce raised the security concern to Han recently. Although Han acknowledged it and confirmed that all concerns would be addressed before the release, Bruce might want the issue addressed differently. The involved code was removed from the release branch early today.

2. Bruce wanted to add him as a lead of the PECL extension. Han updated/reverted the commits.

What Happened This Morning

1. In a small private group, Bruce said some offensive words to Han.

2. Han moved Bruce out from the Swoole project on GitHub, and cleaned up some user permissions.

3. Most other team members (including me) were unaware of what happened until early this morning when Bruce and Han started disputing it in a work group. Bruce apologized for what he had said early today; however, they probably won't work together like before.

4. Bruce started a new GitHub project and published a new PECL extension "openswoole".

My Comment

Han and Bruce have contributed a lot to the Swoole project and the PHP community, and I have massive respect for both of them. However, no one is perfect, and people make mistakes. This is not the first time in the open source world, and won't be the last. I feel disappointed about what happened this morning, but I will stay with it.

[matyhtf]

@mnapoli

After Doubaokun raised concerns about the security risks of downloading dashborad static resource files, I immediately proposed that it would be fixed before the release of the version. We respect everyone's opinions, and we will try our best to resolve all issues in the final version.

I removed the permission of doubaokun for 3 reasons.

1. Doubaokun modified package.xml to change himself to leader without communicating with other members and without my consent. I had to reset his commit via push -f
   https://github.com/swoole/swoole-src/pull/4433/files
2. After I made it clear that he should not change his role as lead and I submitted a new commit to adjust him to developer, he did not communicate with me, and directly re-push overwrites my commit.
   Update version for Swoole 4.8.0 #4431
3. In the internal wechat group, he used extremely bad words to attack me.

So I had to temporarily remove all his permissions. If Doubaokun can cooperate with us friendly, I can add new permissions for him at any time.

[doubaokun]

Hi @foremtehan, @cmb69, @mnapoli & all who may see this thread,

Regarding the security issue, the initial warning was on last Saturday, the second warning was on this Tuesday. But the issue is only resolved after the Open Swoole fork. @deminy you are not in the kernel dev group, so you don't know the full picture and making comments without confirmation with me.

Regarding the 'extremely bad words' were just concern about only matyhtf has the account to do the release and silently push through the package with issues.

At this stage, all the member's releases have to use matyhtf's PECL account which is ridiculous for a project contributed by a group of people.

I was trying to stop the security issues to be published on PECL. As a major maintainer for more than 4 years, I have the right and responsibility to add as 'lead' on PECL to manage PECL packages to resolve this issue. The purpose is cleared said in the commit log: Added release permission #4433, I didn't listed myself as dev in PECL for 4 years to receive credits and I don't care neither.

Also the fact is that my commits are in the logs, everyone can see it. I didn't try or think to add myself for such a long time and did't expect these things can happen.

It is scary to see these things happening and to be removed the Owner role of the organization after you have spend tons of resources on a project for 4 years, then randomly and emotionally removed by another Owner. Then trying to define some new rules make the removing looks like 'legitimate'.

So we will start to work on openswoole and make sure all these issues won't happen.

Best,
Bruce

[nhlm]

@doubaokun a fork under the explained conditions makes no sense. Maintaining something for 4 years doesn't make you a leader. Expecting to become the leader of a (sub-)project after investing work over years is no good behavior to demonstrate your leadership skills. If you would be ready for a project lead, the entire issue here would have been solved differently and with less drama from your side.

I hereby boycott openswoole.

[doubaokun]

@nhlm only the 'lead' role can manage the PECL packages. Plus I have the Owner role of this whole organization 'previously'.

[nhlm]

@nhlm only the 'lead' role can manage the PECL packages. Plus I have the Owner role of this whole organization 'previously'.

Does not change anything. Your decisions lead to a conflict which you were unable to handle appropriate. I recommend to fix the root cause of the issue without creating another realm where the root cause still persists. Learn from it, find a solution that does not require to fork or alter directions of a project. And this time, no public drama please.

[brendt]

In the internal wechat group, he used extremely bad words to attack me.

Regarding the 'extremely bad words' were just concern about only matyhtf has the account to do the release and silently push through the package with issues.

Maybe you should just share the conversation instead of making vague accusations at each other. From the outside, I wouldn't trust either Swoole or Openswoole at this point.

[derickr]

At this stage, all the member's releases have to use matyhtf's PECL account which is ridiculous for a project contributed by a group of people.

Please don't ever share username/password information with multiple people.

If multiple people need to be able to make releases, they all need to have their own PECL account, and be lead in the package.xml file.

Being lead there means nothing more than being to release packages.

[pthreat]

If it's not well understood why people are complaining about the addition of code download on the fly:

Adding hot code download in the background inside some random file of the swoole project it's considered shady at best.

Obviously, the correct approach is to create a separate project which contains said dashboard.

No one ever could possibly believe that a group of people which have a high degree of coding skills would do this "by accident". And I'm inclined to believe this is why Bruce reacted the way he did.