Skip to content

afsocket/transport-mapper: fix auto transport with tls#482

Closed
OverOrion wants to merge 2 commits intoaxoflow:mainfrom
OverOrion:fix-tls-auto-protocol
Closed

afsocket/transport-mapper: fix auto transport with tls#482
OverOrion wants to merge 2 commits intoaxoflow:mainfrom
OverOrion:fix-tls-auto-protocol

Conversation

@OverOrion
Copy link
Contributor

@OverOrion OverOrion commented Jan 30, 2025 •
edited
Loading

This patch makes sure that TLS transport is used for reading, not plain-text.

transport(auto) and tls() didn't work in conjunction.

@version: 4.9

log {
  source{syslog(flags(no-parse) port(5143) transport(auto) tls(
    cert-file(/server-cert.pem)
    key-file(/server-key.pem)
    ca-file(/ca-cert.pem)
    peer-verify(optional-untrusted)
  ));};
  destination{stdout();};
  };

Without this patch it fails:

[2025-01-30T20:44:35.119972] Transport switch requested; active-transport='none', requested-transport='stream-socket'
[2025-01-30T20:44:35.119972] Transport switch successful; new-active-transport='stream-socket'
[2025-01-30T20:44:35.119972] Syslog connection accepted; fd='14', client='AF_INET(127.0.0.1:33288)', local='AF_INET(0.0.0.0:5143)'
[2025-01-30T20:44:35.122761] Unable to detect framing on RFC6587 connection, falling back to simple text transport; fd='14', detect_buffer='\x16\x03\x01\x01 \x01.\x01'
[2025-01-30T20:44:35.122936] Incoming log entry; input='\x16\x03\x01\x01 \x01', msg='0x7e88bc0126f0', rcptid='5820'

(use loggen -P -r 1 -n 1 --debug -U localhost 5143)

TLS transport was not used as the last else branch constructs a logproto from the transport (auto in this case), require_tls_when_has_tls_context takes care of this.

After the patch:

[2025-01-30T20:50:03.722272] Transport switch requested; active-transport='none', requested-transport='tls'
[2025-01-30T20:50:03.722272] Transport switch successful; new-active-transport='tls'
[2025-01-30T20:50:03.722272] Syslog connection accepted; fd='14', client='AF_INET(127.0.0.1:56162)', local='AF_INET(0.0.0.0:5143)'
[2025-01-30T20:50:03.726768] Auto-detected octet-counted-framing on RFC6587 connection, using framed protocol; fd='14'
[2025-01-30T20:50:03.726886] Incoming log entry; input='<38>1 2025-01-30T20:50:03+01:00 localhost prg00000 1234 - - seq: 0000000000, thread: 0000, runid: 1738266603, stamp: 2025-01-30T20:50:03 PADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPAD\x0a', msg='0x72eec001a0e0', rcptid='5868'
[....]
[2025-01-30T20:50:03.727673] Outgoing message; message='Jan 30 20:50:03 localhost <38>1 2025-01-30T20:50:03+01:00 localhost prg00000 1234 - - seq: 0000000000, thread: 0000, runid: 1738266603, stamp: 2025-01-30T20:50:03 PADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPAD\x0a'
[2025-01-30T20:50:03.727675] Syslog connection closed; fd='14', client='AF_INET(127.0.0.1:56162)', local='AF_INET(0.0.0.0:5143)'
[2025-01-30T20:50:03.727675] Closing log transport fd; fd='14'
Jan 30 20:50:03 localhost <38>1 2025-01-30T20:50:03+01:00 localhost prg00000 1234 - - seq: 0000000000, thread: 0000, runid: 1738266603, stamp: 2025-01-30T20:50:03 PADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPADDPAD

@OverOrion
afsocket/transport-mapper: fix auto transport with tls
95577f4 
This patch makes sure that TLS transport is used for reading, not plain-text

Signed-off-by: Szilard Parrag <[email protected]>
@OverOrion
news: add entry for axoflow#482
a2c7a39 
Signed-off-by: Szilard Parrag <[email protected]>
@bazsi
Copy link
Member

bazsi commented Jan 30, 2025

My original plan was to auto-detect if we are receiving TLS in addition to auto-detect framing, see #361

I understand that that PR needs to be rebased and perhaps split, but could we discuss before we merge this?

@OverOrion OverOrion requested a review from bazsi January 31, 2025 13:58
bazsi added a commit to bazsi/axosyslog that referenced this pull request Feb 1, 2025
@bazsi
afsocket: refactor TransportMapperInet transport setup logic
03d365b 
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 added a commit to bazsi/axosyslog that referenced this pull request Feb 2, 2025
@bazsi
afsocket: refactor TransportMapperInet transport setup logic
f3a2902 
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 added a commit to bazsi/axosyslog that referenced this pull request Feb 2, 2025
@bazsi
afsocket: refactor TransportMapperInet transport setup logic
8acb805 
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 bazsi mentioned this pull request Feb 2, 2025
Merged
bazsi added a commit to bazsi/axosyslog that referenced this pull request Feb 4, 2025
@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]>
@OverOrion OverOrion closed this Feb 7, 2025
@MrAnno
Copy link
Contributor

MrAnno commented Feb 7, 2025

We closed this in favor of #361, it contains almost exactly the same but on a different base.

@HofiOne HofiOne mentioned this pull request Apr 4, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bazsi bazsi Awaiting requested review from bazsi

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

@OverOrion @bazsi @MrAnno