Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STOMP collector and output bots: consider restricting the version of stomp.py dependency to >=8.1.0 #2409

Open
zuo opened this issue Sep 14, 2023 · 3 comments

Comments

@zuo
Copy link
Contributor

zuo commented Sep 14, 2023

Rationale:

  • [security] stomp.py in versions older than 4.1.12 always uses the old ssl.wrap_socket() API which has no support for server name indication (SNI) and hostname matching (newer versions of stomp.py use more modern ssl.SSLContext-related APIs if possible).
  • [security] By default, stomp.py in versions 8.0.0. and 8.0.1 (8.x.x older than 8.1.0) mistakenly does not perform hostname matching (an ssl.SSLContext in constructed manually, with protocol set to ssl.PROTOCOL_TLS which does not provide automatic hostname matching -- contrary to ssl.PROTOCOL_TLS_CLIENT which is used in the version 8.1.0 of stomp.py).
  • In the implementation of the aforementioned bots, various versions of stomp.py require different code paths -- see the if statements checking for (or related in other ways with) versions: <4.1.20, >=4.1.21, >=5.0.0, >=6.1.0. Restricting the version of stomp.py to a newer one would make it possible to simplify the code (and probably also make it more correct/reliable...1).

Footnotes

  1. In particular, after some superficial tests it seems to me that disconnects/reconnects are not handled properly by the collector bot when the version of stomp.py is older than 4.1.20 4.1.21 (although I admit I haven't investigated it more deeply...).

@kamil-certat
Copy link
Contributor

This is an important thing and a hard problem to solve on the IntelMQ base. I think that because we are focused on keeping the compatibility, it's not easy for us to just set the required versions as higher. This is also because IntelMQ is partially shipped as a native package, where it's not always clear, which dependencies are available or expected by other software on the machine.

Thous, we have to allow dependencies to work with newer, upgraded version, but the responsibility to install them with the most secure version is on the system administrator. We can however advise doing so. What would you say about adding such a check in the bots' check method and generate a warning if the dependency (here: stomp.py) is older than recommended?

On the other hand, I see the situation differently for Docker images, where we control the environment and ship the working solution - as so, we should also keep the dependencies safe. This is not done yet, but my personal plan is to keep up-to-date (in meaning of security updates) dependencies in our images.

@zuo
Copy link
Contributor Author

zuo commented Sep 21, 2023

Thous, we have to allow dependencies to work with newer, upgraded version, but the responsibility to install them with the most secure version is on the system administrator. We can however advise doing so. What would you say about adding such a check in the bots' check method and generate a warning if the dependency (here: stomp.py) is older than recommended?

It would be, for sure, very useful. :-)

@zuo
Copy link
Contributor Author

zuo commented Oct 8, 2023

The security problems mentioned in this ticket's description would be fixed by merging the PR #2414. So then, I think, this ticket can be closed as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants