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

first persistence draft #866

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

first persistence draft #866

wants to merge 8 commits into from

Conversation

jptosso
Copy link
Member

@jptosso jptosso commented Aug 14, 2023

Adds persistence support using in-memory storage.

Current challenges

  • It only contains a test implementation. It must not be used in production
  • Right now there is no way to kill connections, we should implement WAF close in v4
  • There are race conditions for writing under high concurrency
  • I refactored collections.Map, now it implements collection.Editable, which is also implemented by collection.Persistent. That way, both persistent and map collections can be easily edited in setvar. Maps are still the same, the only difference is that some methods come from editable.

Usage

SecPersistenceEngine default
SecPersistenceTTL 3600
SecRule &REQUEST_COOKIES:session "@gt 0" "id:1,phase:1, initcol:session=%{REQUEST_COOKIES.session}"
SecRule REQUEST_URL "/login.php" "id:2, phase:4, setvar:session.username=%{RESPONSE_ARGS.json.username}"

SecAction "id:3,phase:5,log,msg:'User: %{SESSION.username}'"

@jptosso jptosso requested a review from a team as a code owner August 14, 2023 15:31
@jptosso jptosso marked this pull request as draft August 14, 2023 15:32
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Patch coverage: 74.73% and project coverage change: -0.20% ⚠️

Comparison is base (5d3b707) 81.51% compared to head (db06ddb) 81.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #866      +/-   ##
==========================================
- Coverage   81.51%   81.31%   -0.20%     
==========================================
  Files         160      165       +5     
  Lines        9033     9280     +247     
==========================================
+ Hits         7363     7546     +183     
- Misses       1418     1467      +49     
- Partials      252      267      +15     
Flag Coverage Δ
default 76.70% <72.63%> (+0.10%) ⬆️
examples 25.04% <8.83%> (-0.45%) ⬇️
ftw 45.72% <17.31%> (-1.06%) ⬇️
ftw-multiphase 47.78% <17.31%> (-1.12%) ⬇️
tinygo 74.26% <36.70%> (-0.51%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
experimental/plugins/persistence.go 0.00% <0.00%> (ø)
types/variables/variables.go 100.00% <ø> (ø)
internal/seclang/directives.go 76.20% <45.45%> (-0.70%) ⬇️
internal/actions/initcol.go 61.76% <45.83%> (-38.24%) ⬇️
internal/corazawaf/waf.go 86.78% <50.00%> (-1.46%) ⬇️
internal/variables/variablesmap.gen.go 77.03% <50.00%> (-0.09%) ⬇️
internal/persistence/persistence.go 62.50% <62.50%> (ø)
internal/corazawaf/transaction.go 78.30% <63.63%> (-0.23%) ⬇️
internal/persistence/noop.go 75.00% <75.00%> (ø)
internal/actions/setvar.go 63.55% <77.77%> (+3.36%) ⬆️
... and 2 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

// Map are used to store VARIABLE data
// for transactions, this data structured is designed
// to store slices of data for keys
// Important: CollectionMaps ARE NOT concurrent safe
type Map interface {
Keyed
Editable
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a breaking change, as it still implements Keyed and it keeps the same functions (?)

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

Successfully merging this pull request may close these issues.

1 participant