Ratchet icon indicating copy to clipboard operation
Ratchet copied to clipboard

Unable to alter/remove the `X-Powered-By` header? SECURITY problem?

Open Rezyan opened this issue 3 years ago • 2 comments

Hi,

The X-Powered-By header sent by Ratchet describes the technologie used by the server. Therefore this info exposes the server to malicious attackers who can find vulnerabilities easier.

Below are the two files that add this header:

https://github.com/ratchetphp/Ratchet/blob/5012dc954541b40c5599d286fd40653f5716a38f/src/Ratchet/WebSocket/WsServer.php#L117-L119

https://github.com/ratchetphp/Ratchet/blob/5012dc954541b40c5599d286fd40653f5716a38f/src/Ratchet/Http/CloseResponseTrait.php#L15-L17

I have tried everything to hide this header with the decorators, but I can't achieve it. I spent my day there. The handshake negotiator is implicitly called inside the Ratchet\WebSocket\WsServer class and I can't modify the response that is directly sent. I didn't try to modify the header sent by the Ratchet\Http/CloseResponseTrait class, since I was unable to hide the one from the Ratchet\WebSocket\WsServer class...

Has anyone ever succedeed in changing/removing this header with the decorators? If so, how? Am I missing something obvious? Or is there indeed a issue in hiding this header? I ask for your help to clarify the situation, thanks!

Related issues and PRs:

  • https://github.com/ratchetphp/Ratchet/issues/932
  • https://github.com/ratchetphp/Ratchet/issues/650
  • https://github.com/ratchetphp/Ratchet/pull/524

Regards.

Rezyan avatar Apr 16 '22 01:04 Rezyan

I agree that this is a security issue. Overall, trying to change something small with the decorators in Ratchet is difficult. The Ratchet methods are huge, and often you need to copy-paste and overwrite a large piece of library code to create your own decorator version.

szado avatar Apr 16 '22 19:04 szado

I agree that this is a security issue. Overall, trying to change something small with the decorators in Ratchet is difficult. The Ratchet methods are huge, and often you need to copy-paste and overwrite a large piece of library code to create your own decorator version.

Unfortunately, copy-paste and overwrite a large piece of library code is not a relevant solution for a security issue. It would be nice if we could easily change this very annoying header. Maybe with an argument given in the constructors?

Rezyan avatar Apr 16 '22 19:04 Rezyan

I've just created a pull request to add a way to fix this issue: https://github.com/ratchetphp/Ratchet/pull/992

@cboden Could you please take a look to my solution and run the workflow? Thanks!

Rezyan avatar Jan 09 '23 23:01 Rezyan

@Bugophobia I agree that this can be a potential threat, but I don't see the problem in the whole X-Powered-By header, only in exposing the used version of Ratchet. Even without the X-Powered-By header you can still get the information that Ratchet is being used.

As I see it there are multiple options to go here. One can be to file a PR to remove the used version. As for the header itself you can either configure everything manually which involves some work or you can set up a reverse proxy like nginx and configure it to remove the upstream X-Powered-By header.

What do you think?

SimonFrings avatar Jan 11 '23 07:01 SimonFrings

@Bugophobia I agree that this can be a potential threat, but I don't see the problem in the whole X-Powered-By header, only in exposing the used version of Ratchet. Even without the X-Powered-By header you can still get the information that Ratchet is being used.

The expose_php PHP configuration set to false completely remove the X-Powered-By header from PHP, and that's for a good reason. This makes it harder for attackers to know what is going on at the server level. Any clue is a potential security vulnerability. We should do exactly the same as PHP for Ratchet. The longer it takes the attacker to find the server technology, the more secure the server will be.

As I see it there are multiple options to go here. One can be to file a PR to remove the used version. As for the header itself you can either configure everything manually which involves some work or you can set up a reverse proxy like nginx and configure it to remove the upstream X-Powered-By header.

What do you think?

If a security issue is present in the Ratched code, the Ratched code must be fixed, or at least we have to suggest a solution to fix it through PHP. If you have to change a Ngnix or Apache configuration to fix a security issue in Ratchet, it doesn't make sense in my humble opinion. It's like trying to fix a PHP code error with Apache instructions. If we assume that we can use a workaround with Apache/Ngninx, we can also assume that Ratchet support is dead and that we should perhaps think about using other libraries.


To sump up my opinion, I think the solution I propose is the easiest to implement, especially since it's not a breaking change. Rather than using arguments in constructors, we could also use the expose_php PHP configuration, if we really want to be lazy.

To conclude, the X-Powered-By HTTP header has NO BENEFIT EXCEPT TO MAKE THE CODE MORE VULNERABLE TO ATTACKS: this just serve the malicious users, the other users don't care... Let's also mention that it is intentionally added by Ratchet. If I had my way, I would have removed it directly from the code, without even offering any option to use it.

Rezyan avatar Jan 11 '23 09:01 Rezyan

Any news from the Ratchet maintainers please? Do you intend to fix this security issue (if so, in what way, I am willing to do the PR myself), or should we use forks? Thanks

Rezyan avatar Jan 17 '23 07:01 Rezyan

@Elysiome Thanks for reporting, I agree that X-Powered-By could be problematic and could potentially be used during reconnaissance to gain more information to prepare attacks and as such I agree this should be addressed.

I see you've already filed #992 and https://github.com/ratchetphp/RFC6455/pull/69 to address this, so let's continue discussion from there. This project is actively supported, but please keep in mind this is an open-source project that is mostly supported by its maintainers in their free time as good as humanly possible. Please reach out if you want to support this project.

In the meantime, you can always avoid this header by configuring your reverse proxy to remove this header. For nginx this should be as easy as:

proxy_hide_header X-Powered-By;

That said, I do not currently see that this HTTP header alone constitutes a security issue on its own. If you believe you have identified a security issue, I would like to ask you to follow responsible disclosure principles. See https://github.com/ratchetphp/Ratchet/security/policy for details.

clue avatar Feb 07 '23 10:02 clue

I see you've already filed #992 and ratchetphp/RFC6455#69 to address this, so let's continue discussion from there. This project is actively supported, but please keep in mind this is an open-source project that is mostly supported by its maintainers in their free time as good as humanly possible. Please reach out if you want to support this project.

I understand, that's why I suggested to implement myself any solution you would accept in your code.


In the meantime, you can always avoid this header by configuring your reverse proxy to remove this header. For nginx this should be as easy as:

proxy_hide_header X-Powered-By;

I've already answered to this:

If a security issue is present in the Ratched code, the Ratched code must be fixed, or at least we have to suggest a solution to fix it through PHP. If you have to change a Ngnix or Apache configuration to fix a security issue in Ratchet, it doesn't make sense in my humble opinion. It's like trying to fix a PHP code error with Apache instructions. If we assume that we can use a workaround with Apache/Ngninx, we can also assume that Ratchet support is dead and that we should perhaps think about using other libraries.

If you want to make an omelette with eggs and cheese, but the eggs are out of date, will you change the cheese? No, that's the same here.


That said, I do not currently see that this HTTP header alone constitutes a security issue on its own. If you believe you have identified a security issue, I would like to ask you to follow responsible disclosure principles. See https://github.com/ratchetphp/Ratchet/security/policy for details.

I've also answered to this:

The expose_php PHP configuration set to false completely remove the X-Powered-By header from PHP, and that's for a good reason. This makes it harder for attackers to know what is going on at the server level. Any clue is a potential security vulnerability. We should do exactly the same as PHP for Ratchet. The longer it takes the attacker to find the server technology, the more secure the server will be.


It's too bad not to want to fix a small piece of code that serves no purpose except to help malicious users and offer them time.

Much to my regret, since this problem will never be fixed in Ratchet itself, I close the issue and the pull requests.

Rezyan avatar Feb 07 '23 11:02 Rezyan

I'd still like to address this, but I do not appreciate your passive aggressive tone. I'll lock this ticket to suggest a cool down period for this discussion, see #999 for a follow-up.

clue avatar Feb 07 '23 12:02 clue