Skip to content

Conversation

@Nydauron
Copy link
Contributor

  • Fixes the default config to work with the default LAPI config.
  • Adds extra error handling to properly distinguish when the LAPI request 404s due to double backslashes (e.g. http://localhost:8080//v1/decisions?ip=). Before this patch, crowdsec_proxy() would return OK with the response being the empty string.

This is more or less a bandage fix and doesn't fix the underlying
problem within the module.
@blotus
Copy link
Member

blotus commented Jan 30, 2025

Hey,

Thanks for the PR !

What do you think about also removing automatically extra / at the end of the provided URL ?

That way, the bouncer will work in both case, and it limits the risk of user error.

@Nydauron
Copy link
Contributor Author

Nydauron commented Jan 31, 2025

What do you think about also removing automatically extra / at the end of the provided URL ?

That way, the bouncer will work in both case, and it limits the risk of user error.

Sure, I think that would be a good idea. It would make sense if we simply stored the schema and authority prefix without any trailing slashes inside sconf->url when the module sets it with CrowdSecURL (e.g. http://127.0.0.1).

Some other things to think about:

  • Would we want to add a parsing check for the URL to prevent apache from starting up with a bad URL?
    • An error message can then be provided that gets printed to logs
  • Is CrowdSecURL strictly expecting a <scheme>://<authority>/ format, or would we allow a say values without the scheme (e.g. <hostname>:<port>) to also be valid?

@Nydauron Nydauron force-pushed the fix/lapi-empty-response branch from b8c8870 to e4e0cdb Compare January 31, 2025 22:19
@Nydauron Nydauron force-pushed the fix/lapi-empty-response branch from e4e0cdb to 6fbf8ee Compare February 6, 2025 05:14
@Nydauron
Copy link
Contributor Author

Nydauron commented Feb 6, 2025

Went ahead and implemented a URL validation check when CrowdsecURL gets set. Since most of the other bouncers expect at least a schema and authority in the URL, it makes sense to check if the URL prefix follows <scheme>://<authority> and ignores the rest if there is any.

This simply parses the URL string to ensure the prefix contains a scheme
and an authority, following the RFC 3986 standard. If the scheme and
authority cannot be parsed due to the passed string not following the
RFC standard, Apache will fail to start and print an error to logs.

The path, query, and fragment are subsequently ignored and a warning
gets printed if anything besides the string "/" is found after the
authority.
@Nydauron Nydauron force-pushed the fix/lapi-empty-response branch from 6fbf8ee to 247d0bc Compare February 6, 2025 05:35
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.

2 participants