ModSecurity
ModSecurity copied to clipboard
Update libinjection & Mbed TLS
what
Update third-party dependencies (included in others directory): libinjection & Mbed TLS.
why
The versions included in ModSecurity have not been updated in a while:
- libinjection currently references commit libinjection/libinjection@bfba51f from Dec, 2020.
- This PR references the current head of the library (libinjection/libinjection@b9fcaaf), as the project isn't currently being tagged (the last one is from 2017).
- Even though the project hasn't move a lot since then, there are at least a couple of changes that may be interesting:
- libinjection/libinjection@a28aceec29932d216f487b09882667479de717ed
- libinjection/libinjection@98cf343b0c94b7e9a94aadcaa94ef779f7e16633
- A subset of Mbed TLS source code from (at least) Nov, 2016 is included in the ModSecurity repository.
- This PR references the latest tag of the library (3.6.0, Mbed-TLS/mbedtls@2ca6c28) and instead of including their source code, uses a submodule (which makes it clear the version being used and simpler to update in the future).
- A few interesting changes in the referenced subset of the library (base64, md5 & sha1) since the last update (all of them may not be relevant in ModSecurity usage):
- Mbed-TLS/mbedtls@ab043350525833314b5a2e6d0d2efcd6510a8142
- Mbed-TLS/mbedtls@1cc1fb05999aea8067e11f5c4f4fdb32dbe91036
- Mbed-TLS/mbedtls@d1c98fcf5e04b8f6628ca951131aa1f7d67283de
- Mbed-TLS/mbedtls@0963e6cfac2230d68c6ed1aa220ac41f096796ff
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code
We should indeed update these libraries from time to time. This PR dynamically uses the last version (master), right? It seems they don't provide a release version. Do you (and other people contributing to this repo, I some some other ones) consider the master version as always stable? Btw, the same should be done for v2 as well.
This PR dynamically uses the last version (master), right? It seems they don't provide a release version. Do you (and other people contributing to this repo, I some some other ones) consider the master version as always stable?
In both cases the PR references a specific commit hash in order to pin down the version included in libModSecurity until a new review and update is done.
With regards to libinjection, as the project is not currently tagged (the last tag is from 2017), I'm referencing the current head (libinjection/libinjection@b9fcaaf). Mbed TLS references the commit for tag 3.6.0 (git submodules don't currently support explicitly referencing a tag), Mbed-TLS/mbedtls@2ca6c28.
Hi @eduar-hte,
thanks again for this great PR!
Fortunately @fzipi is one of the maintainer/developer of libinjection project so I tagged him - may be he can help us.
Thanks @eduar-hte for this great work again.
A reminder to myself: add git submodule init and git submodule update commands to README.md.
What you normally do is use the checkout actions like this:
- name: Checkout
uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4
with:
submodules: recursive
That will bring you what you want.
So, we should replace the hard-coded files in v2 with submodules as well, right?
What you normally do is use the checkout actions like this: (...)
That's right, the submodules option simplifies checkout of the repository and also its submodules. However, I found out that the git describe command would not work when the submodules are initialized that way, so I replaced that (arguably simpler approach) with an additional step to do those steps manually as a workaround.
If you take a look at the build with the workaround, you'll see that the 'configure' step shows version info on MbedTLS & libInjection correctly:
Mandatory dependencies
+ libInjection ....v3.9.2-92-gb9fcaaf
+ Mbed TLS ....v3.6.0
+ SecLang tests ....a3d4405
However, if you look at an older build, you'll notice that the version information is missing:
Mandatory dependencies
+ libInjection ....
+ SecLang tests ....a3d4405
Oh, for describe you might also want to use depth: 0
Oh, for describe you might also want to use
depth: 0
You're right, fetch-depth: 0 works! (I thought I had tested that before going for the workaround 🤷♂️)
I just created a simple PR (#3185) to simplify the workflow again, thanks.