-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
skupper-router project initialization #12646
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
ganeshmurthy is integrating a new project: |
06c7d91
to
1c897e8
Compare
Can you share with us how large the user base is, and what interesting attack surface OSS Fuzz can help protecting? |
skupper-router is an open source project which is part of a larger project called Skupper (https://skupper.io/) skupper-router uses the qpid-proton library which is already one of the oss-fuzz projects https://github.com/google/oss-fuzz/tree/master/projects/qpid-proton skupper-router is capable of reading tcp traffic over a listening port. It sniffs the initial incoming tcp bytes to determine if the protocol being used is http1 or http2 or some other protocol. If say, the protocol is http2, it starts reading http2 frames to get some stats from the traffic like request method, url etc. OSS Fuzz can send in random data to these protocol observers and try to crash the router. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
We can merge this once the CI tests get fixed. @ganeshmurthy |
projects/skupper-router/Dockerfile
Outdated
RUN apt-get update | ||
RUN apt-get install -y cmake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the inverse of how it should be done. Each RUN
layer that installs something will need to have an update
in the same RUN
layer. Otherwise a stale update
layer could lead to issues in the later install
layer.
edit: Also, cmake is already installed anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the unnecessary cmake install and fixed up the call to apt-get update
a5facb3
to
47f6fa6
Compare
@ganeshmurthy we can merge this once the tests pass |
I think I fixed a problem that I saw in the CI test failure. I pushed up a fix for it. Let's see if the CI passes this time |
No description provided.