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

fix: DB helper should properly filter entries #3359

Merged
merged 1 commit into from May 22, 2023

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented May 22, 2023

Description

Previously it was assumed the sed operation was applying the sed expressions as a sequence, but it did not seem to filter entries being looked up correctly. Instead any line that matched either sed expression pattern was output (value without matching key, values split by the delimiter), then grep would match any of that causing false-positives.

Resolved by piping the first sed expression into the next.

Fixes: #3358

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Previously it was assumed the sed operation was applying the sed expressions as a sequence, but it did not seem to filter entries being looked up correctly, thus matching values from entry keys that weren't correct...

Resolved by piping the first sed expression into the next.
@polarathene polarathene added area/scripts kind/bug/fix A fix (PR) for a confirmed bug labels May 22, 2023
@polarathene polarathene self-assigned this May 22, 2023
@polarathene
Copy link
Member Author

Probably a case where an improved CI for patch releases would be handy 😅

I guess this will have to wait until v13?

@polarathene polarathene added this to the v13.0.0 milestone May 22, 2023
@georglauterbach
Copy link
Member

georglauterbach commented May 22, 2023

Probably a case where an improved CI for patch releases would be handy 😅

I think there are no maintainer capacity limits left 😂

I guess this will have to wait until v13?

Yes. But there really isn't too much that's blocking v13.0.0. I'm waiting on review from @casperklein for my PR, and the Getmail PR is waiting too. And then there is only the IPv6 PR. I don't mind changing defaults with v14.0.0 to rspamd. It seems a lot of users are already using Rspamd, so no rush with the defaults.

@polarathene
Copy link
Member Author

polarathene commented May 22, 2023

I think this is fine to merge now.

Casper already gave a 👍 to the solution shared on the reported issue earlier, any changes can be done via follow-up PR if necessary :)


I think there are no maintainer capacity limits left

💯

Yes. But there really isn't too much that's blocking v13.0.0.

👍


And then there is only the IPv6 PR.

Yep still working on that digging through upstream moby issues atm 😅 (doesn't have to block the release though)

I recently learned that due to Docker setting sysctl net.ipv4.ip_forward=1 and it's IPTables rules for published ports, you can route to 127.0.0.1 and docker network subnets private address space from another host within the same LAN. Technically another open relay 😬

Originally I was looking into a similar vulnerability for userland-proxy: false setting route_localnet=1, as this was an issue for k8s in the past, but doesn't appear to be reproducible with Dockers network management.

I've confirmed at least with firewalld active that the LAN hosts can't route to the hosts private subnets (127.0.0.1 can still be routed to though). Once the userland-proxy investigation is wrapped up (almost done), the IPv6 docs PR will resume 👍

I'd give an ETA, but I'm terrible with that these days 😬

@polarathene polarathene merged commit 1d2df8d into master May 22, 2023
7 checks passed
@polarathene polarathene deleted the fix/db-helper-filter-entry branch May 22, 2023 23:02
@casperklein
Copy link
Member

Maybe I've overlooked something, but afaik there were no breaking changes since 12.1.0. As of now, the next release could be 12.2.0?

@georglauterbach
Copy link
Member

Technically, true. I actually considered #3235 not a breaking change, but we waited with the change as @polarathene considered it to be breaking. The other major change is in #3335, which technically is not breaking either.

We do need to make sure to communicate these two big changes thoroughly though, in case we go for v12.2.0.

@polarathene
Copy link
Member Author

Or just publish as v13, doesn't have to be a big release to warrant it 😅

@georglauterbach
Copy link
Member

Or just publish as v13, doesn't have to be a big release to warrant it 😅

I'd go for v13 as well. I reall don't mind having higher numbers - I'm not Linus Torvalds..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scripts kind/bug/fix A fix (PR) for a confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug report: setup alias add may fail when recipient exists for another alias
3 participants