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

Add support for DTLS 1.3 (DTLSv1.3) through SSLContext / SSLEngine #254

Merged
merged 10 commits into from
Feb 26, 2025

Conversation

cconlon
Copy link
Member

@cconlon cconlon commented Feb 14, 2025

This PR adds DTLS 1.3 support to the wolfJSSE SSLContext class, for use through the SSLEngine interface. SunJSSE supports up to DTLS 1.2 through the SSLEngine interface, so we are matching similar behavior here for DTLS 1.3.

To support this addition, this PR also makes the following changes:

  • Adds DTLS 1.3 to the thin JNI-only layer
  • Wraps wolfSSL_DisableExtendedMasterSecret()
  • Adds support for the Java system property: jdk.tls.useExtendedMasterSecret for enabling or disabling the TLS Extended Master Secret (EMS) extension. Enabled by default unless explicitly disabled.
  • Wraps wolfSSL_send_hrr_cookie() in WolfSSLSession.sendHrrCookie() for enabling HelloRetryRequest use in SSLEngine DTLS support.
  • Wraps wolfSSL_dtls_get_drop_stats() for use in SSLEngine DTLS detection of dropped packets to properly set BUFFER_UNDERFLOW status.
  • Wrap wolfSSL_set_SessionTicket_cb() and add session ticket callback support to SSLEngine for detection of ticket being received.

wolfSSL Configuration

When testing this work, wolfSSL was configured using:

./configure --enable-jni --enable-dtls --enable-dtls13 --enable-dtls-mtu --enable-hrrcookie --enable-debug CFLAGS="-DWOLFSSL_DTLS_DROP_STATS"

Native wolfSSL's build option for --enable-jni will be updated to include these definitions by default, with wolfSSL/wolfssl#8481.

Testing

Simple JUnit tests for SSLEngine DTLSv1.3 have been added with this PR. This has also been tested against modified versions of the SunJSSE DTLS tests, specifically:

Passed: javax/net/ssl/DTLS/CipherSuite.java
Passed: javax/net/ssl/DTLS/ClientAuth.java
Passed: javax/net/ssl/DTLS/DTLSBufferOverflowUnderflowTest.java
Passed: javax/net/ssl/DTLS/DTLSDataExchangeTest.java
Passed: javax/net/ssl/DTLS/DTLSEnginesClosureTest.java
Passed: javax/net/ssl/DTLS/DTLSHandshakeTest.java
Passed: javax/net/ssl/DTLS/DTLSHandshakeWithReplicatedPacketsTest.java
Passed: javax/net/ssl/DTLS/DTLSIncorrectAppDataTest.java
Passed: javax/net/ssl/DTLS/DTLSNotEnabledRC4Test.java
Passed: javax/net/ssl/DTLS/DTLSOverDatagram.java
Passed: javax/net/ssl/DTLS/DTLSUnsupportedCiphersTest.java
Passed: javax/net/ssl/DTLS/InvalidCookie.java
Passed: javax/net/ssl/DTLS/PacketLossRetransmission.java
Passed: javax/net/ssl/DTLS/Reordered.java
Passed: javax/net/ssl/DTLS/RespondToRetransmit.java
Passed: javax/net/ssl/DTLS/Retransmission.java

A PR to the testing repo has been opened here: https://github.com/wolfSSL/testing/pull/844. A nightly Jenkins test will be created based on the test script in that PR.

Examples

JNI-level example client and server can be run with DTLS 1.3 support using:

$ ant examples
$ ./examples/server.sh -u -v 4
$ ./examples/client.sh -u -v 4

Future Work

  • Add JSSE level example client/server that supports DTLS 1.3
  • Add SSLContext('DTLS') general mapping
  • Add SSLContext('DTLSv1.2') for DTLS 1.2 support
  • Add support for "special" features of DTLS 1.3, this PR brings in basic functionality

@cconlon cconlon self-assigned this Feb 14, 2025
@cconlon cconlon force-pushed the dtls branch 2 times, most recently from 13288e4 to 61fa093 Compare February 15, 2025 00:14
@cconlon cconlon force-pushed the dtls branch 2 times, most recently from 6399a7b to 5f10363 Compare February 17, 2025 22:32
Copy link
Contributor

@JacobBarthelmeh JacobBarthelmeh left a comment

Choose a reason for hiding this comment

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

Great, worked for server/client DTLS 1.3 interop.

@JacobBarthelmeh
Copy link
Contributor

I asked for Devin to provide feedback also, am curious what it has to say on this addition. Will merge depending/after that feedback.

Copy link

Code Review for PR #254: DTLS 1.3 Support

I've reviewed the changes in this PR focusing on memory leaks, unchecked return values, and test coverage for the new DTLS 1.3 functionality. Here are my findings:

Memory Management

The code demonstrates good memory management practices:

  • Proper JNI Memory Handling: Consistent use of JNI memory management functions like and with appropriate flags (e.g., when no changes need to be copied back)
  • Error Path Cleanup: All error paths properly release allocated resources
  • Resource Cleanup: Consistent pattern of resource cleanup in all functions
  • JNI References: Proper use of for JNI references

Example of good memory management in the new code:

Return Value Handling

The return value handling is robust throughout the code:

  • Consistent Error Checking: All wolfSSL function return values are properly checked
  • Error Propagation: Consistent error propagation pattern using or
  • Null Checks: Proper null checks before dereferencing pointers
  • Error Handling: Error conditions properly propagate error codes back to Java

Example of good return value handling:

Test Coverage

The test coverage for the new DTLS 1.3 functionality is comprehensive:

  • Dedicated Tests: Specific tests for DTLS 1.3 basic connection and session resumption
  • Protocol Testing: Existing tests updated to run with all protocols including DTLS 1.3
  • Edge Cases: Tests for buffer sizes, large data transfer, and session resumption behavior
  • External Testing: Additional testing against modified SunJSSE DTLS tests

Static Analysis

Static analysis of the code shows good practices:

  • Void Pointer Arithmetic: Proper casting using for JNI pointer conversions with a consistent pattern:
  • Format Strings: No format string issues identified in the modified code
  • Unsigned Integer Handling: Proper use of type for drop count statistics and consistent casting of return values
  • Buffer Safety: No potential buffer overflows identified, with proper array length checks before accessing array elements

Improvement Suggestions

While the code is generally well-written, here are a few minor suggestions:

  1. Consider adding more detailed comments for the DTLS-specific functionality to improve maintainability
  2. The PR could benefit from a JSSE-level example client/server that demonstrates DTLS 1.3 usage (noted as future work in the PR description)

Conclusion

This PR adds DTLS 1.3 support with high-quality code that follows good practices for memory management, error handling, and includes comprehensive test coverage. I recommend approval of this PR.

@JacobBarthelmeh JacobBarthelmeh merged commit ad23ac2 into wolfSSL:master Feb 26, 2025
47 checks passed
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.

3 participants