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

Initial implementation of imdtls and omdtls modules #5280

Merged
merged 1 commit into from Feb 26, 2024

Conversation

alorbach
Copy link
Member

@alorbach alorbach commented Nov 24, 2023

Initial implementation of imdtls and omdtls modules

  • Extracted basic OpenSSL helper functions into own module net_ossl.h/net_ossl.c
    Both are compiled into lmnsd_ossl.
  • Cleanup of OpenSSL code, fixed minor compiler and linking issues.
  • Added DTLS Sender option DTLS into tcpflood for testbench.
  • Add initial implementation of imdtls input module. Added to configure and makefile
  • Add initial implementation of omdtls output module. Added to configure and makefile
  • Add multiple basic tests for imdtls receiving data by using tcpflood.
  • Add multiple send-receive test for imdtls and omdtls based on existing tls tests.
  • Add timeout and sessionbreak tests for imdtls stress testing.

closes: #5211

@alorbach alorbach force-pushed the pr-issue-5211 branch 3 times, most recently from 28080de to df73a29 Compare November 27, 2023 15:38
Copy link
Member

@rgerhards rgerhards left a comment

Choose a reason for hiding this comment

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

I reviewed the most important files. While I found a lot, it's mostly nits (with comment issues being the top issues). However, there are some more substantial comments, but nothing really bad :-)

plugins/imdtls/imdtls.c Outdated Show resolved Hide resolved
plugins/imdtls/imdtls.c Outdated Show resolved Hide resolved
plugins/imdtls/imdtls.c Outdated Show resolved Hide resolved
plugins/imdtls/imdtls.c Outdated Show resolved Hide resolved
plugins/imdtls/imdtls.c Outdated Show resolved Hide resolved
plugins/omdtls/omdtls.c Outdated Show resolved Hide resolved
plugins/omdtls/omdtls.c Outdated Show resolved Hide resolved
plugins/omdtls/omdtls.c Outdated Show resolved Hide resolved
plugins/omdtls/omdtls.c Outdated Show resolved Hide resolved
plugins/omdtls/omdtls.c Show resolved Hide resolved
@davidelang
Copy link
Contributor

davidelang commented Nov 28, 2023 via email

@rgerhards
Copy link
Member

@davidelang
Copy link
Contributor

I haven't dug through things yet, but given that out-of-order recovery and fragmentation handling can be used for DOS purposes (valid handshake, then start sending everything but the first packet, per the dtls rfc the receiver it supposed to hang on to the later packets waiting for the first one)

there should be configuration for timeouts, but for retransmission and also for throwing away later packets in the sequence.

@alorbach
Copy link
Member Author

I haven't dug through things yet, but given that out-of-order recovery and fragmentation handling can be used for DOS purposes (valid handshake, then start sending everything but the first packet, per the dtls rfc the receiver it supposed to hang on to the later packets waiting for the first one)

there should be configuration for timeouts, but for retransmission and also for throwing away later packets in the sequence.

Good point. I will add timeout handling to imdtls module. I would suggest a default timeout of 30 minutes for a DTLS session, do you agree?

@davidelang
Copy link
Contributor

davidelang commented Jan 11, 2024 via email

@alorbach
Copy link
Member Author

This is less about timing out an entire session than timing out individual chunks. the rfc suggests a 1s timeout for lost chunks (before you are supposed to retransmit them) but says nothing about timing out n+x chunks when you haven't received chunk N yet (it says if you receive future chunks you are supposed to hold on to them and process them after you receive chunk N). If you have a proper sender who will retry chunk N quickly, this isn't bad (and avoids having to retransmit all newer chunks because one arrives out of order), but if you have a malicous sender you need to have a short timeout (single digit seconds) that you hang on to these out-of-order chunks before you throw them away and force the sender to retransmit them David Lang

On Thu, 11 Jan 2024, Andre Lorbach wrote: > I haven't dug through things yet, but given that out-of-order recovery and fragmentation handling can be used for DOS purposes (valid handshake, then start sending everything but the first packet, per the dtls rfc the receiver it supposed to hang on to the later packets waiting for the first one) > > there should be configuration for timeouts, but for retransmission and also for throwing away later packets in the sequence. Good point. I will add timeout handling to imdtls module. I would suggest a default timeout of 30 minutes for a DTLS session, do you agree?

I think this is protocol handling and it is done inside OpenSSL. And I assume it should be properly handled inside OpenSSL. We use the DTLS_method() context and the same OpenSSL API's to receive by DTLS as we use in the nsd_ossl.c code (SSL_read).

@rgerhards
Copy link
Member

I think this is protocol handling and it is done inside OpenSSL. And I assume it should be properly handled inside OpenSSL.

I would check if openSSL accepts config parameters for the timeouts @davidelang mentioned. Actually I would assume that such params exist.

@alorbach alorbach force-pushed the pr-issue-5211 branch 2 times, most recently from 1168781 to 5058ddc Compare February 23, 2024 11:55
- Extracted basic OpenSSL helper functions into own module net_ossl.h/net_ossl.c
  Both are compiled into lmnsd_ossl.
- Cleanup of OpenSSL code, fixed minor compiler and linking issues.
- Added DTLS Sender option DTLS into tcpflood for testbench.
- Add initial implementation of imdtls input module. Added to configure and makefile
- Add initial implementation of omdtls output module. Added to configure and makefile
- Add multiple basic tests for imdtls receiving data by using tcpflood.
- Add multiple send-receive test for imdtls and omdtls based on existing tls tests.
- Add timeout and sessionbreak tests for imdtls stress testing.

closes: rsyslog#5211
@rgerhards rgerhards self-assigned this Feb 23, 2024
@rgerhards rgerhards merged commit f2f9332 into rsyslog:master Feb 26, 2024
39 checks passed
alorbach added a commit to alorbach/rsyslog-doc that referenced this pull request Feb 26, 2024
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.

DTLS Support with rsyslog
3 participants