mirror of https://github.com/OISF/suricata
You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
397 lines
13 KiB
ReStructuredText
397 lines
13 KiB
ReStructuredText
************************
|
|
Contributing to Suricata
|
|
************************
|
|
|
|
This guide describes what steps to take if you want to contribute a patch or
|
|
patchset to Suricata.
|
|
|
|
Essentially, these are:
|
|
|
|
#. Agree to and sign our :ref:`Contribution Agreement<contribution-agreement>`
|
|
#. Communicate early, and use the :ref:`preferred channels <communication-channels>`
|
|
#. :ref:`claim-ticket`
|
|
#. :ref:`Fork from master <what-branch-to-work-on>`
|
|
#. Follow our :ref:`Coding Style`
|
|
#. Use our :ref:`documentation-style`
|
|
#. Stick to our :ref:`commit guidelines<commits>`
|
|
#. Add version numbers to your :ref:`Pull Requests <send-a-pull-request>`
|
|
#. Incorporate :ref:`feedback` into new PRs
|
|
#. [Work merged] :ref:`Wrap up! <wrap-up>`
|
|
|
|
The rest of this document will cover those in detail.
|
|
|
|
.. _contribution-agreement:
|
|
|
|
.. note:: Important!
|
|
|
|
Before contributing, please review and sign our `Contribution Agreement
|
|
<https://suricata.io/contribution-agreements/>`_.
|
|
|
|
.. _communication-channels:
|
|
|
|
Communication is Key!
|
|
=====================
|
|
|
|
To clarify questions, discuss or suggest new features, talk about bugs and
|
|
optimizations, and/or ask for help, it is important to communicate.
|
|
|
|
These are our main channels:
|
|
|
|
* `Suricata's issue tracker <https://redmine.openinfosecfoundation.org/
|
|
projects/suricata/issues>`_
|
|
* `Suricata's forum <https://forum.suricata.io/c/developers/8>`_
|
|
* `Suricata's Discord server <https://discord.com/invite/t3rV2x7MrG>`_
|
|
|
|
|
|
.. _claim-ticket:
|
|
|
|
Claim (or open) a ticket
|
|
========================
|
|
|
|
For features and bugs we need `tickets <https://redmine.openinfosecfoundation.
|
|
org/projects/suricata/issues>`_. Tickets help us keep track of the work done,
|
|
indicate when changes need backports etc.
|
|
|
|
They are also important if you would like to see your new feature officially
|
|
added to our tool: the ticket documents your ideas so we can analyze how do they
|
|
fit in our plans for Suricata, and, if the feature is accepted, we can properly
|
|
track progress etc.
|
|
|
|
The ticket should clearly reflect the intention as per the tracker.
|
|
For example, if the ticket is a "Bug", the title should only say what the
|
|
bug is.
|
|
|
|
**Good ticket title examples**
|
|
|
|
1. **Ticket:**
|
|
[Bug #00000] stream: segfault in case of increasing gaps
|
|
|
|
**Why is it good?**
|
|
It shows subsystem affected and exactly what the bug is.
|
|
|
|
2. **Ticket:**
|
|
[Bug #19999] dcerpc: memleak in case of invalid data
|
|
|
|
**Why is it good?**
|
|
It talks about the bug itself as the Tracker indicates.
|
|
|
|
3. **Ticket:**
|
|
[Bug #44444] stream: excess memuse in `TcpTracking`
|
|
|
|
**Why is it good?**
|
|
Title is to the point and conveys what the issue is.
|
|
|
|
.. note:: The ticket titles are used to auto generate ChangeLog with each
|
|
release. If the ticket titles are unclear, the ChangeLog does not properly
|
|
convey what issues were fixed with a release.
|
|
|
|
.. note:: If you want to add new functionalities (e.g. a new application layer
|
|
protocol), please ask us first whether we see that being merged into
|
|
Suricata or not. This helps both sides understand how the new feature will
|
|
fit in our roadmap, and prevents wasting time and motivation with
|
|
contributions that we may not accept. Therefore, `before` starting any code
|
|
related to a new feature, do request comments from the team about it.
|
|
|
|
For really trivial fixes or cleanups we won't need that.
|
|
|
|
Once work on the issue has been agreed upon:
|
|
|
|
Assign the ticket to yourself. For this, you will need to have the "developer"
|
|
role. You can ask for that directly on the ticket you want to claim or mention
|
|
that you are interested in working on `ticket number` on our `Developer's
|
|
channel on Discord <https://discord.com/channels/864648830553292840/
|
|
888087709002891324>`_.
|
|
|
|
If a ticket is already assigned to someone, please reach out on the ticket or
|
|
ask the person first.
|
|
|
|
You can reach out to other community members via `Suricata's Discord server
|
|
<https://discord.com/invite/t3rV2x7MrG>`_.
|
|
|
|
|
|
Expectations
|
|
============
|
|
|
|
If you submit a new feature that is not part of Suricata's core functionalities,
|
|
it will have the `community supported`_ status. This means we would expect some
|
|
commitment from you, or the organization who is sponsoring your work, before we
|
|
could approve the new feature, as the Suricata development team is pretty lean
|
|
(and many times overworked).
|
|
|
|
This means we expect that:
|
|
|
|
* the new contribution comes with a set of Suricata-verify tests (and
|
|
possibly unit tests, where those apply), before we can approve it;
|
|
* proof of compatibility with existing keywords/features is provided,
|
|
when the contribution is for replacing an existing feature;
|
|
* you would maintain the feature once it is approved - or some other
|
|
community member would do that, in case you cannot.
|
|
|
|
.. note::
|
|
|
|
Regardless of contribution size or complexity, we expect that you respect
|
|
our guidelines and processes. We appreciate community contributors:
|
|
Suricata wouldn't be what it is without them; and the value of our tool and
|
|
community also comes from how seriously we take all this, so we ask that
|
|
our contributors do the same!
|
|
|
|
.. _community supported:
|
|
|
|
What does "community supported" and "supporting a feature" mean?
|
|
-----------------------------------------------------------------
|
|
|
|
If a feature is *community supported*, the Suricata team will try to spend
|
|
minimal time on it - to be able to focus on the core functionalities. If for any
|
|
reason you're not willing or able to commit to supporting a feature, please
|
|
indicate this.
|
|
|
|
The team and/or community members can then consider offering help. It is best
|
|
to indicate this prior to doing the actual work, because we will reject features
|
|
if no one steps up.
|
|
|
|
It is also important to note that *community supported* features will be
|
|
disabled by default, and if it brings in new dependencies (libraries or Rust
|
|
crates) those will also be optional and disabled by default.
|
|
|
|
**Supporting a feature** means to actually *maintain* it:
|
|
|
|
* fixing bugs
|
|
* writing documentation
|
|
* keeping it up to date
|
|
* offering end-user support via forum and/or Discord chat
|
|
|
|
.. _stale-tickets-policy:
|
|
|
|
Stale tickets policy
|
|
====================
|
|
|
|
We understand that people's availability and interested to volunteer their time
|
|
to our project may change. Therefore, to prevent tickets going stale (not worked
|
|
on), and issues going unsolved for a long time, we have a policy to unclaim
|
|
tickets if there are no contribution updates within 6 months.
|
|
|
|
If you claim a ticket and later on find out that you won't be able to work on
|
|
it, it is also appreciated if you inform that to us in the ticket and unclaim
|
|
it, so everyone knows that work is still open and waiting to be done.
|
|
|
|
.. _what-branch-to-work-on:
|
|
|
|
What branch to work on
|
|
======================
|
|
|
|
There are usually 2 or 3 active branches:
|
|
|
|
* master-x.x.x (e.g. master-6.0.x)
|
|
* main-x.x.x (e.g. main-7.0.x)
|
|
* master
|
|
|
|
The ones with version numbers are stable branches. **master** is the development branch.
|
|
|
|
The stable branch should only be worked on for important bug fixes or other
|
|
needed :doc:`backports<backports-guide>`. Those are mainly expected from more
|
|
experienced contributors.
|
|
|
|
Development of new features or large scale redesign is done in the development
|
|
branch. New development and new contributors should work with *master* except
|
|
in very special cases - which should and would be discussed with us first.
|
|
|
|
If in doubt, please reach out to us via :ref:`Redmine, Discord or
|
|
forum <communication-channels>`.
|
|
|
|
.. _create-your-own-branch:
|
|
|
|
Create your own branch
|
|
======================
|
|
|
|
It's useful to create descriptive branch names. You're working on ticket 123 to
|
|
improve GeoIP? Name your branch "geoip-feature-123-v1". The "-v1" addition is
|
|
for feedback. When incorporating feedback you will have to create a new branch
|
|
for each pull request. So, when you address the first feedback, you will work in
|
|
"geoip-feature-123-v2" and so on.
|
|
|
|
For more details check: `Creating a branch to do your changes <https://redmine.
|
|
openinfosecfoundation.org/projects/suricata/wiki/GitHub_work_flow#Creating-a-
|
|
branch-to-do-your-changes>`_
|
|
|
|
|
|
Coding Style
|
|
============
|
|
|
|
We have a :ref:`Coding Style` that must be followed.
|
|
|
|
.. _documentation-style:
|
|
|
|
Documentation Style
|
|
===================
|
|
|
|
For documenting *code*, please follow Rust documentation and/or
|
|
Doxygen guidelines, according to what your contribution is using (Rust
|
|
or C). The rest of this section refers to the user and developer
|
|
documentation.
|
|
|
|
The user and developer guide documentation (what you are reading now)
|
|
is written in *reStructuredText* and rendered with `Sphinx
|
|
<https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html>`_. For
|
|
a primer *reStucturedText* please see the `reStrucutredText Primer
|
|
<https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html>`_.
|
|
|
|
When writing or updating *documentation pages*, please:
|
|
|
|
* wrap up lines at 79 (80 at most) characters;
|
|
* when adding diagrams or images, we prefer alternatives that can be generated
|
|
automatically, if possible;
|
|
* bear in mind that our documentation is published on `Read the Docs <https:/
|
|
/docs.suricata.io/en/latest/#suricata-user-guide>`_ and can also be
|
|
built to pdf, so it is important that it looks good in such formats.
|
|
|
|
Headings
|
|
--------
|
|
|
|
reStructuredText allows for flexible header order, for consistency
|
|
please use the following order:
|
|
|
|
* ``#``: for h1
|
|
* ``*``: for h2
|
|
* ``=``: for h3
|
|
* ``-``: for h4
|
|
* ``~``: for h5
|
|
* ``^``: for h6
|
|
|
|
For example, in a new documentation page:
|
|
|
|
.. code-block:: rst
|
|
|
|
Page Title
|
|
##########
|
|
|
|
Section
|
|
*******
|
|
|
|
Sub-Section
|
|
===========
|
|
|
|
Rule examples
|
|
-------------
|
|
|
|
.. role:: example-rule-action
|
|
.. role:: example-rule-header
|
|
.. role:: example-rule-options
|
|
.. role:: example-rule-emphasis
|
|
|
|
For rule documentation, we have a special container::
|
|
|
|
example-rule
|
|
|
|
This will present the rule in a box with an easier to read font size, and also
|
|
allows highlighting specific elements in the signature, as the names indicate
|
|
- action, header, options, or emphasize custom portions:
|
|
|
|
- example-rule-action
|
|
- example-rule-header
|
|
- example-rule-options
|
|
- example-rule-emphasis
|
|
|
|
When using these, indicate the portion to be highlighted by surrounding it with
|
|
` . Before using them, one has to invoke the specific role, like so::
|
|
|
|
.. role:: example-rule-role
|
|
|
|
It is only necessary to invoke the role once per document. One can see these
|
|
being invoked in our introduction to the rule language (see `Rules intro
|
|
<https://raw.githubusercontent.com/OISF/suricata/master/doc/userguide/rules/intro.rst>`_).
|
|
|
|
A rule example like::
|
|
|
|
.. container:: example-rule
|
|
|
|
:example-rule-action:`alert` :example-rule-header:`http $HOME_NET any ->
|
|
$EXTERNAL_NET any` :example-rule-options:`(msg:"HTTP GET Request Containing
|
|
Rule in URI"; flow:established,to_server; http.method; content:"GET"; http.uri;
|
|
content:"rule"; fast_pattern; classtype:bad-unknown; sid:123; rev:1;)`
|
|
|
|
Results in:
|
|
|
|
.. container:: example-rule
|
|
|
|
:example-rule-action:`alert` :example-rule-header:`http $HOME_NET any ->
|
|
$EXTERNAL_NET any` :example-rule-options:`(msg:"HTTP GET Request Containing
|
|
Rule in URI"; flow:established,to_server; http.method; content:"GET"; http.uri;
|
|
content:"rule"; fast_pattern; classtype:bad-unknown; sid:123; rev:1;)`
|
|
|
|
Example - emphasis::
|
|
|
|
.. container:: example-rule
|
|
|
|
alert ssh any any -> any any (msg:"match SSH protocol version";
|
|
:example-rule-emphasis:`ssh.proto;` content:"2.0"; sid:1000010;)
|
|
|
|
Renders as:
|
|
|
|
.. container:: example-rule
|
|
|
|
alert ssh any any -> any any (msg:"match SSH protocol version";
|
|
:example-rule-emphasis:`ssh.proto;` content:"2.0"; sid:1000010;)
|
|
|
|
Commit History matters
|
|
======================
|
|
|
|
Please consider our :ref:`Commit guidelines <commits>` before submitting your PR.
|
|
|
|
.. _send-a-pull-request:
|
|
|
|
Send a Pull Request
|
|
===================
|
|
|
|
The pull request is used to request inclusion of your patches into the main
|
|
repository. Before it is merged, it will be reviewed and pushed through a QA
|
|
process.
|
|
|
|
Please consider our :ref:`Pull Requests Criteria <pull-requests-criteria>` when
|
|
submitting.
|
|
|
|
We have 'GitHub-CI' integration enabled. This means some automated build check,
|
|
suricata-verity and unit tests are performed on the pull request. Generally,
|
|
this is ready after a few minutes. If the test fails, the pull request won't be
|
|
considered. So please, when you submit something, keep an eye on the checks,
|
|
and address any failures - if you do not understand what they are, it is fine to
|
|
ask about them on the failing PR itself.
|
|
|
|
Before merge, we also perform other integration tests in our private QA-lab. If
|
|
those fail, we may request further changes, even if the GitHub-CI has passed.
|
|
|
|
.. _feedback:
|
|
|
|
Feedback
|
|
========
|
|
|
|
You'll likely get some feedback. Even our most experienced devs do, so don't
|
|
feel bad about it.
|
|
|
|
After discussing what needs to be changed (usually on the PR itself), it's time
|
|
to go back to ":ref:`create-your-own-branch`" and do it all again. This process
|
|
can iterate quite a few times, as the contribution is refined.
|
|
|
|
.. _wrap-up:
|
|
|
|
Wrapping up
|
|
===========
|
|
|
|
Merged! Cleanup
|
|
---------------
|
|
|
|
Congrats! Your change has been merged into the main repository. Many thanks!
|
|
|
|
We strongly suggest cleaning up: delete your related branches, both locally and
|
|
on GitHub - this helps you in keeping things organized when you want to make new
|
|
contributions.
|
|
|
|
.. _update-ticket:
|
|
|
|
Update ticket
|
|
-------------
|
|
|
|
You can now put the URL of the *merged* pull request in the Redmine ticket.
|
|
Next, mark the ticket as "Closed" or "Resolved".
|
|
|
|
Well done! You are all set now.
|