Skip to content

Conversation

Kilobyte22
Copy link

@Kilobyte22 Kilobyte22 commented May 6, 2025

This PR adds support for HAProxys SPOE (Stream Processing Offloading Engine), which roughly speaking is its plugin system. Basically haproxy can offload decision making to some external program (like anubis!) which then can further be used within haproxy. A typical configuration for a tool like anubis would look like this:

  • The request enters haproxy
  • haproxy asks anubis via SPOP how to handle the request. It passes all information required to make this decision
  • Anubis reports the decision to haproxy
  • One of the following cases happens (this logic is part of the configuration file):
    • Anubis reports "pass": The request is routed straight to the correct backend (cookie is valid or a pass rule matches)
    • Anubis reports "block": haproxy directly replies with an error reply or routes it to an error-backend (a deny rule matches)
    • Anubis reports "challenge": haproxy routes the request to anubis (a challenge rule matches)

The full HTTP request is only sent to anubis in the third case and in fact the request is never proxied through anubis (if we ignore og passthrough)

I still have some issues with the implementation, further details in a comment below.

Fixes #236

Checklist:

  • Added a description of the changes to the [Unreleased] section of docs/docs/CHANGELOG.md
  • Added test cases to the relevant parts of the codebase
  • Ran integration tests npm run test:integration (unsupported on Windows, please use WSL)

tacerus and others added 3 commits May 6, 2025 08:05
To avoid having Anubis handle all traffic in scenarios where HAProxy is
intended to be the main reverse proxy, allow Anubis to act as a SPOE
informing HAProxy whether a request has already been validated. ACLs in
HAProxy can then be used to route these requests to the desired backend
without involving Anubis as a reverse proxy.

Signed-off-by: Georg Pfuetzenreuter <[email protected]>
@Kilobyte22
Copy link
Author

Kilobyte22 commented May 6, 2025

I have based this PR on the work by @tacerus. I am not happy yet with its state, but I want some feedback first, since i see some major issues ahead, which should be addressed before a merge. Hence I have not yet completed the checklist either.

Currently there is no way to meet the decision on how to proceed without providing a full http request. I therefore suggest to change to a custom data structure like this. Since this is a major change in the architecture i want some feedback first:

type RequestMetadata {
    source net.IP
    user_agent string
}

Any check would then only have the data to decide upon, so if a future release requires additional data, it would need manual configuration on the haproxy side. The checks would then return a verdict, instead of directly working with http responses and the http response would be generated at a centralized location.

Do you have any better ideas? Unfortunately you have to explicitly specify which data to send to the SPOA

@tacerus
Copy link

tacerus commented May 6, 2025

Thanks for continuing my patch!

@Kilobyte22
Copy link
Author

@Xe a quick poke for the pending design review in case you didn't take a look at it yet because it is still marked as draft :)

@Xe
Copy link
Contributor

Xe commented May 8, 2025

Hi, I'll take a look after I release v1.18.0. Thanks!

@Xe
Copy link
Contributor

Xe commented May 9, 2025

Anubis will need at minimum the following information:

  • All HTTP headers
  • Request path
  • Request query string
  • Remote IP of the request
  • HTTP request method
  • HTTP hostname the request is targeted to

Ideally everything in net/http#Request.

@Kilobyte22
Copy link
Author

Thanks for having a look!

My worry was the "all headers" part as to my knowledge (as of an hour ago) it is only possible to send specific headers, but after some more research this seems possible using the req.hdrs_bin variable in haproxy. The other fields are easy to do, so all in all, this should be pretty doable. I'll also have a look at the http request struct.

Previously rule evalution would directly work with http,Request. This
changes introduces an additional level of abstraction in anticipation of
adding SPOE support, where there is no http.Request, available, even
though most metadata it would contain is still present.
HAProxy can now perform a detailed query to find out if it should
forward the request to anubis
Include configuration options added by previous commits
@Kilobyte22
Copy link
Author

Kilobyte22 commented May 18, 2025

@tacerus since you originally submitted the patch, are you interested in testing my implementation (and documentation)?

Tests are still missing, I'll have to figure out how to properly implement useful test cases. Once thats done I can add testcases and then this PR would be ready for review.

I also have a couple failures in the playwright tests, although I am unsure why. Possibly explained by Xes comment?

    --- FAIL: TestPlaywrightBrowser/webkit/warmup (1.97s)
    --- FAIL: TestPlaywrightBrowser/webkit/firefox (13.73s)
    --- FAIL: TestPlaywrightBrowser/webkit/headlessChrome (13.87s)
    --- FAIL: TestPlaywrightBrowser/webkit/Amazonbot (14.02s)
    --- FAIL: TestPlaywrightBrowser/webkit/Amazonbot#01 (13.91s)
    --- FAIL: TestPlaywrightBrowser/webkit/PerplexityAI (14.09s)
    --- SKIP: TestPlaywrightBrowser/webkit/kagiBadIP (0.00s)
    --- FAIL: TestPlaywrightBrowser/webkit/kagiGoodIP (13.96s)
    --- FAIL: TestPlaywrightBrowser/webkit/unknownAgent (14.20s)
=== RUN   TestPlaywrightWithBasePrefix
    playwright_test.go:274: NOTE(Xe)\ these tests require HTTPS support in #364
--- SKIP: TestPlaywrightWithBasePrefix (0.00s)
FAIL
FAIL	github.com/TecharoHQ/anubis/internal/test	346.160s
FAIL

@tacerus
Copy link

tacerus commented May 18, 2025

Hi,

I briefly tested it, a few comments so far:

  • I notice the cookie debug log always contains "!BADKEY":"handle request", can the BADKEY can be avoided?
  • I wonder if the sample backend shouldn't be called anubis-spoa instead of anubis-spoe, as from my understanding the referenced listener would be referred to as agent, whilst the engine sits in HAProxy? Going by https://cdn.haproxy.com/img/containers/partner_integrations/image3-%284%29.png/b1cc934c50468269663bd889dc04113a/image3-%284%29.png.
  • I think the health checks should not get the Real-IP header overwritten.
  • Could the sample be simplified to use the default option forwardfor header, X-Forwarded-For, which is already used in other places in Anubis anyways, or is there some benefit with X-Real-IP?
  • The diagram looks nice in raw MarkDown, but not in the GitHub rendered view.
  • Should we rather have spoeBind unset by default? I assume many users will not use it, and hence originally planned to make it optional.
  • Sending invalid data to the SPOP listener causes Anubis to panic/crash, whilst the listener should of course ideally only be exposed to trusted sources, it could still allow for exploitation, maybe the input could be validated somehow?

@tacerus
Copy link

tacerus commented May 18, 2025

If needed, reproducer for my last point:

$ telnet 2a02:1748:f7df:9c80::b 9000
Trying 2a02:1748:f7df:9c80::b...
Connected to 2a02:1748:f7df:9c80::b.
Escape character is '^]'.

### press enter

8f
  status-codecmessagan unknown error occurredConnection closed by foreign host.
Anubis panic output
panic: runtime error: slice bounds out of range [:218759168] with capacity 65535

goroutine 68 [running]:
github.com/dropmorepackets/haproxy-go/pkg/buffer.(*SliceBuffer).WriteNBytes(...)
        /home/georg/go/pkg/mod/github.com/dropmorepackets/[email protected]/pkg/buffer/slicebuf.go:45
github.com/dropmorepackets/haproxy-go/spop.(*frame).ReadFrom(0xc000166600, {0xdad080, 0xc000194008})
        /home/georg/go/pkg/mod/github.com/dropmorepackets/[email protected]/spop/frame.go:59 +0x1cf
github.com/dropmorepackets/haproxy-go/spop.(*protocolClient).Serve(0xc0000a8540)
        /home/georg/go/pkg/mod/github.com/dropmorepackets/[email protected]/spop/protocol.go:72 +0x89
github.com/dropmorepackets/haproxy-go/spop.(*Agent).Serve.func2()
        /home/georg/go/pkg/mod/github.com/dropmorepackets/[email protected]/spop/agent.go:66 +0x94
created by github.com/dropmorepackets/haproxy-go/spop.(*Agent).Serve in goroutine 22
        /home/georg/go/pkg/mod/github.com/dropmorepackets/[email protected]/spop/agent.go:62 +0x119

@Kilobyte22
Copy link
Author

Kilobyte22 commented May 18, 2025

Thanks very much for the feedback!

  • I notice the cookie debug log always contains "!BADKEY":"handle request", can the BADKEY can be avoided?
    • I'll take a look at that
  • I wonder if the sample backend shouldn't be called anubis-spoa instead of anubis-spoe, as from my understanding the referenced listener would be referred to as agent, whilst the engine sits in HAProxy? Going by https://cdn.haproxy.com/img/containers/partner_integrations/image3-%284%29.png/b1cc934c50468269663bd889dc04113a/image3-%284%29.png.
    • I was wondering myself, maybe it's less confusing for new users? Idk, maybe I'm overthinking it
  • I think the health checks should not get the Real-IP header overwritten.
    • I believe anubis threw some errors when i didn't specify it, but maybe my information is wrong
  • Could the sample be simplified to use the default option forwardfor header, X-Forwarded-For, which is already used in other places in Anubis anyways, or is there some benefit with X-Real-IP?
    • Anubis already uses the X-Real-IP header. I do not believe changing that ore adding a second one provides any additional value
  • The diagram looks nice in raw MarkDown, but not in the GitHub rendered view.
    • Hm, maybe a rendered picture is probably better then
  • Should we rather have spoeBind unset by default? I assume many users will not use it, and hence originally planned to make it optional.
    • I was thinking about that as well, I think thats probably a good idea
  • Sending invalid data to the SPOP listener causes Anubis to panic/crash, whilst the listener should of course ideally only be exposed to trusted sources, it could still allow for exploitation, maybe the input could be validated somehow?
    • I'll look into that, but thats probably something the SPOE lib will need to fix upstream. I'll check and possibly open an issue there.

@tacerus
Copy link

tacerus commented May 18, 2025

I was wondering myself, maybe it's less confusing for new users? Idk, maybe I'm overthinking it

Fair, I assume for new users, the whole SPO[AEP] terminology is confusing. ;-)

Anubis already uses the X-Real-IP header. I do not believe changing that ore adding a second one provides any additional value

I thought Anubis accepts both, X-Forwarded-For and X-Real-IP equally (or rather sets one by the other), it would not be adding a new one, hence wondered about keeping the HAProxy default for simplicity. But I might be missing something, I only skimmed over internal/headers.go.

I'll look into that, but thats probably something the SPOE lib will need to fix upstream. I'll check and possibly open an issue there.

Thanks, with the original library it seems to not have been an issue, there it would disconnect the faulty client but not panic.

@darix
Copy link

darix commented May 21, 2025

I thought Anubis accepts both, X-Forwarded-For and X-Real-IP equally (or rather sets one by the other), it would not be adding a new one, hence wondered about keeping the HAProxy default for simplicity. But I might be missing something, I only skimmed over internal/headers.go.

TBH is that X- header even used as part of the SPOE handling? I would assume not.

@Kilobyte22
Copy link
Author

Kilobyte22 commented May 21, 2025

TBH is that X- header even used as part of the SPOE handling? I would assume not.

It is not used for the SPOE part, but it is used once the request gets routed to anubis to provide the challenge. (And anubis will throw an error if you don't set it)

I haven't had the time to check on this yet though and it is unlikely that i get to addressing the open issues before the end of the month. (having said that, it should be in a working state, so if anyone wants to test it and provide feedback, feel free to)

@scabala
Copy link

scabala commented Jun 16, 2025

Hello,
I would like to use this feature. Is there any help I can help move it forward?

@Kilobyte22
Copy link
Author

I've been very busy with life recently and my little free time has been consumed by some time critical projects, so it will likely be a month or two before I find time to continue this PR. Open points are mostly the things tacerus pointed out and whatever Xe would like to be changed as part of the review.

I've been in contact with the maintainer for the spoe library and they claim the crash should be fixed, that needs to be updated and tested as well.

Other than that, it should be ready to use. I am currently using it myself and it hasn't caused any issues so far, if you want to test it, feel very welcome. Maybe I can spare a minute or two later and rebase this PR, since Anubis has obviously gotten a lot of work in recently.

@darix
Copy link

darix commented Jun 29, 2025

Just curious ... if it is working. couldnt you remove the WIP flag?

@Xe
Copy link
Contributor

Xe commented Jun 29, 2025

Oh, I wasn't aware this was ready for review. It's marked as draft and WIP so I figured it was still in the cooker.

@Kilobyte22
Copy link
Author

Sorry, i wasn't sure if this was considered "ready for review" - different projects, different standards. I've removed the WIP tag.

@Kilobyte22 Kilobyte22 marked this pull request as ready for review June 30, 2025 15:44
@darix
Copy link

darix commented Jul 7, 2025

btw:

did you see the comment about performance in #236 (comment) ?

@Kilobyte22
Copy link
Author

Yes, this PR uses the library from the DropMorePackets project, one of the changes i did from the original work from tacerus

@darix
Copy link

darix commented Jul 7, 2025

that is not what your go.mod file says... well it mentions you use both. so you can clean it up maybe.

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.

spoe support for anubis
5 participants