Skip to content

[HAPROXY] use $SOURCEIP instead of $PROXIED_SRCIP#361

Merged
MrAnno merged 24 commits intoaxoflow:mainfrom
bazsi:haproxy-use-sourceip-instead-of-proxied-srcip
Feb 7, 2025
Merged

[HAPROXY] use $SOURCEIP instead of $PROXIED_SRCIP#361
MrAnno merged 24 commits intoaxoflow:mainfrom
bazsi:haproxy-use-sourceip-instead-of-proxied-srcip

Conversation

@bazsi
Copy link
Member

@bazsi bazsi commented Nov 2, 2024 •
edited
Loading

This is a large refactor of the HAProxy support, and preparations for protocol auto detection. It also changes the HAProxy support to use the standard source/destination addresses in LogMessage, instead of a proxy specific values e.g. $SOURCEIP instead of $PROXIED_SRCIP.

Short summary of the patches:

  • small refactors and cleanups (first 5 patches)
  • simplify TLS compression setup code, previously this crossed a lot of layers, whereas it can be a lot simpler
  • LogTransportTLS is becoming an adapter, e.g. instead of going directly to file descriptors, go through another layer of the LogTransport interface. This prepares for protocol auto detection.
  • introduces LogTransportStack level aux data, making it possible to enrich messages from the any of the Transports
  • adds the $SOURCEPORT macro
  • LogTransportHAProxy is changed so it invokes a log_transport_stack_switch() instead of doing this internally
  • has a big refactor of TransportMapperInet, as its various settings and options were becoming too difficult to read. This patch also adds a large light testcase to cover all possible transport() cases
  • contains an alternative fix for afsocket/transport-mapper: fix auto transport with tls #482 (although pretty similar)
  • and few smaller patches

@bazsi bazsi force-pushed the haproxy-use-sourceip-instead-of-proxied-srcip branch 2 times, most recently from cdfd3b6 to c2c1e8f Compare November 2, 2024 21:09
@bazsi bazsi mentioned this pull request Nov 8, 2024
Merged
@bazsi bazsi force-pushed the haproxy-use-sourceip-instead-of-proxied-srcip branch 4 times, most recently from 58f020f to dff7494 Compare November 11, 2024 19:51
@bazsi bazsi force-pushed the haproxy-use-sourceip-instead-of-proxied-srcip branch 2 times, most recently from 672e0c2 to 1c9e896 Compare December 3, 2024 17:30
@bazsi bazsi force-pushed the haproxy-use-sourceip-instead-of-proxied-srcip branch from 1c9e896 to 5ee3ffd Compare January 27, 2025 08:55
@bazsi bazsi mentioned this pull request Jan 30, 2025
Closed
@bazsi bazsi force-pushed the haproxy-use-sourceip-instead-of-proxied-srcip branch 6 times, most recently from b3b5bc7 to e8322f7 Compare February 2, 2025 16:21
@bazsi bazsi requested a review from MrAnno February 2, 2025 17:04
@bazsi
Copy link
Member Author

bazsi commented Feb 2, 2025

This is now ready for review and I think would be a better solution than #482 with the same fix.

@bazsi
Copy link
Member Author

bazsi commented Feb 3, 2025

@OverOrion @MrAnno could you re-review this, so we can merge this instead of #482?

@MrAnno MrAnno requested a review from OverOrion February 3, 2025 12:46
MrAnno
MrAnno reviewed Feb 3, 2025
View reviewed changes
@MrAnno
Copy link
Contributor

MrAnno commented Feb 3, 2025

I'm reviewing it.

OverOrion
OverOrion reviewed Feb 3, 2025
View reviewed changes
@MrAnno MrAnno self-requested a review February 3, 2025 16:21
bazsi added 17 commits February 4, 2025 13:08
@bazsi
transport/tls-context: simplify compression setup
5fa97df 
Previously TLS compression was enabled using an overly complicated mechanism
crossing a number of layers (TransportMapperInet -> TransportFactoryTLS ->
TLSSession -> SSL). This can be a lot simpler, which this patch
implements.

NOTE: compression will not work in most cases due to OpenSSL security
levels and this patch adds a warning about it.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport-tls: convert to a LogTransportAdapter
2b3296a 
Instead of going to the fd directly, wrap the lower-level LogTransport
instance into a BIO and use that. This implements proper stacking
for LogTransportTLS.

This adds the use of OpenSSL BIOs to wrap the lower level LogTransport
instance.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport-aux-data: add log_transport_aux_data_move()
8aeb9ff 
Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport-stack: make it possible to add aux data from the LogTranspo…
919bccd 
…rtStack level

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
logproto: call transport-stack level I/O methods
aebabff 
Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
logproto/logproto-auto-server: remove RFC6587 references from log mes…
f0704b0 
…sages

The "auto" protocol can be applied to both syslog() and network(), so
it's not strictly RFC6587 related and it does not add too much information
anyway.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
templates: add $SOURCEPORT macro
02e1fd9 
Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport-haproxy: use the normal $SOURCEIP, $DESTIP, $DESTPORT macros
bc95df8 
Instead of using proxy protocol specific name value pairs, set the
addresses in the message's saddr/daddr members.

This should be a lot faster and a lot easier to use.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport-haproxy: use the LogTransportStack->aux to save src/dst add…
dd9f318 
…resses

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport-haproxy: implement transport switch instead of changing bas…
b4f0552 
…e_index

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport-haproxy: add more information to the proxy detection failur…
d038370 
…e message

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
afsocket: refactor TransportMapperInet transport setup logic
7511b97 
This reworks the various boolean members in TransportMapperInet that
control which logproto/transport we apply to a specific connection.

With these renames, it's much easier to follow what happens and why.

NOTE: there's a followup bugfix that fixes the same bug as axoflow#482.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
afsocket: transport(auto) with tls() should immediately start using TLS
b52ecdb 
"auto" has originally been planned to auto-detect TLS as well as framing
format, but at this point it does not do TLS auto-detection.

But this means that transport(auto) with tls() options set will start reading
data without SSL, e.g. the encrypted stuff will make it into the
messages received.

This patch fixes that for both the syslog() and the network() driver. The
only change is that delegate_tls_start_to_logproto is FALSE for the "auto"
case. This will be changed once the TLS auto detection feature is also
in.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport/transport-factory-haproxy: use factory for creating HAProxy…
96cc1f1 
… transports

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
light: generalize test_pp_network and test_pp_syslog test cases
cc9260a 
Instead of just exercising the proxyprotocol try all valid transports, including
the "auto" variants.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
light: fix up files included in the dist
0a44754 
Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
news: added news file
6344ea8 
Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi bazsi force-pushed the haproxy-use-sourceip-instead-of-proxied-srcip branch from e8322f7 to 6344ea8 Compare February 4, 2025 12:22
MrAnno
MrAnno reviewed Feb 4, 2025
View reviewed changes
Copy link
Contributor

@MrAnno MrAnno left a comment

Choose a reason for hiding this comment

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

I've started manual-testing the generic- and TLS-related transport changes (I think we need thorough testing since this touches core functionality).

@MrAnno
Copy link
Contributor

MrAnno commented Feb 5, 2025

I've tested the TLS-related changes on a real network connection through multiple hops, it works as expected.

bazsi added 2 commits February 6, 2025 14:04
@bazsi
transport: remove unused files
d4a7589 
Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi
transport: free LogTransportBIO BIO_meth instance at shutdown
704ded6 
This was a one-off allocation, but it's better if it is freed.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi bazsi force-pushed the haproxy-use-sourceip-instead-of-proxied-srcip branch from d05bb69 to 704ded6 Compare February 6, 2025 13:29
@bazsi
Copy link
Member Author

bazsi commented Feb 6, 2025

@MrAnno I've resolved the memory leak and did not add BIO_set_nbio() call and resolved your comments. Let me know if you agree. Thanks

@MrAnno MrAnno merged commit fe2fdf9 into axoflow:main Feb 7, 2025
22 checks passed
fekete-robert pushed a commit to axoflow/axosyslog-core-docs that referenced this pull request Feb 15, 2025
[4.10] Documents $SOURCEPORT macro
6d541b0 
Based on axoflow/axosyslog#361
fekete-robert pushed a commit to axoflow/axosyslog-core-docs that referenced this pull request Feb 15, 2025
Documents macro changes for proxy protocol
028be92 
axoflow/axosyslog#361
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@OverOrion OverOrion OverOrion left review comments

@MrAnno MrAnno MrAnno approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bazsi @MrAnno @OverOrion