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

new plugin: security/crowdsec #2945

Merged
merged 7 commits into from May 10, 2022
Merged

Conversation

mmetc
Copy link
Contributor

@mmetc mmetc commented Apr 15, 2022

This is a plugin we developed to provide configuration and a basic UI for the crowdsec IDS and IPS. It depends on a couple of binaries recently added to ports.conf

Adding machines (servers, other firewalls) and advanced configuration are not managed by the UI but available from the command line.

Screenshot from 2022-04-18 23-12-32

Screenshot from 2022-04-15 09-09-30

@AdSchellevis
Copy link
Member

@mmetc it would be better to unpack the different flag options into parsable ones, it's more or less impossible to validate user input in the long term for these kind of fields. I'll try to review the rest of the plugin later.

@AdSchellevis AdSchellevis self-assigned this Apr 15, 2022
@mmetc mmetc force-pushed the crowdsec branch 2 times, most recently from dd0071c to e2b2426 Compare April 18, 2022 20:44
@mmetc
Copy link
Contributor Author

mmetc commented Apr 18, 2022

@AdSchellevis thanks, good point. I replaced the free-text *_flags options with the only two flags that are actually useful right now, and updated the screenshot.

Copy link
Member

@fichtner fichtner left a comment

Choose a reason for hiding this comment

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

overall this looks good for inclusion, nice work! let's iterate through some early feedback and then merge to work on the remainder in the tree

@mmetc
Copy link
Contributor Author

mmetc commented Apr 25, 2022

Thanks for the careful review!
I am collecting your remarks and will act upon them, if not for the initial release, for the next one.

@fichtner
Copy link
Member

Two open comments, but looks good to me. I'll leave the merge up to @AdSchellevis which could take a bit since this week is more or less a vacation week.

Co-authored-by: Franco Fichtner <franco@lastsummer.de>
@AdSchellevis
Copy link
Member

@mmetc If I'm not mistaken the only open item on the list is the setup.sh at the moment, right? if you need help, just ping me.

@mmetc
Copy link
Contributor Author

mmetc commented Apr 30, 2022

Oh thanks, I thought and closed it. I'm not doing setup.sh because the directory is needed by "configctl template reload" upon install, not by my own daemons at boot.
Thanks again for the extensive review

@mmetc
Copy link
Contributor Author

mmetc commented May 10, 2022

I don't want to push but I think this is ready to be merged, just in case I'm missing something..

@fichtner
Copy link
Member

I don’t understand the argument against setup.sh, but as said I’m ok with the current state.

@fichtner fichtner merged commit ce4469f into opnsense:master May 10, 2022
@mmetc mmetc deleted the crowdsec branch May 10, 2022 10:16
agh1467 pushed a commit to agh1467/plugins that referenced this pull request Jun 11, 2022
This is a plugin we developed to provide configuration and a basic UI for the crowdsec IDS and IPS. It depends on a couple of binaries recently added to ports.conf

Adding machines (servers, other firewalls) and advanced configuration are not managed by the UI but available from the command line.
agh1467 pushed a commit to agh1467/plugins that referenced this pull request Jun 11, 2022
This is a plugin we developed to provide configuration and a basic UI for the crowdsec IDS and IPS. It depends on a couple of binaries recently added to ports.conf

Adding machines (servers, other firewalls) and advanced configuration are not managed by the UI but available from the command line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants