-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Handle unexpected end of stream errors from KMS. #1849
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
base: main
Are you sure you want to change the base?
Conversation
JAVA-6015
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds handling for unexpected end-of-stream errors from KMS providers during encryption operations. The key change is detecting when a KMS provider closes a connection prematurely (returning -1 from read operations) and throwing a descriptive MongoException instead of allowing undefined behavior.
Key changes:
- Added EOF detection in KMS stream reading logic for both sync and reactive drivers
- Extracted SSL context building utilities to a shared test fixture to support new test infrastructure
- Added test case that simulates KMS server abruptly closing connections
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| driver-sync/src/main/com/mongodb/client/internal/Crypt.java | Added EOF check when reading from KMS provider streams and updated exception wrapping logic |
| driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/internal/crypt/KeyManagementService.java | Added EOF detection in async stream reading callback |
| driver-sync/src/test/functional/com/mongodb/fixture/EncryptionFixture.java | Extracted SSL context and KeyManagerFactory creation utilities for test reuse |
| driver-sync/src/test/functional/com/mongodb/client/auth/AbstractX509AuthenticationTest.java | Refactored to use shared SSL context utilities from EncryptionFixture |
| driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideEncryptionKmsTlsTest.java | Added test simulating KMS server EOF scenario with fake SSL server |
| .evergreen/run-kms-tls-tests.sh | Added server keystore generation and configuration for KMS TLS testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideEncryptionKmsTlsTest.java
Outdated
Show resolved
Hide resolved
driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideEncryptionKmsTlsTest.java
Outdated
Show resolved
Hide resolved
driver-sync/src/test/functional/com/mongodb/client/AbstractClientSideEncryptionKmsTlsTest.java
Show resolved
Hide resolved
|
Assigned |
|
|
||
| @Override | ||
| public void completed(final Integer integer, final Void aVoid) { | ||
| if (integer == -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just checked , looks like there is no a dedicated constant variable that indicates -1 as an end of stream
server.pemis used to start the MongoDB server, and the truststore is used by the Java tests to trust that certificate. Since thetruststoreis already configured to trustserver.pem, it can be reused for the fake KMS/KMIP servers as well - the JDK TLS verifier will validate the chain/signature successfully.This is also how driver-evergreen-tools uses it:
In setup-secrets.sh (line 16):
export CSFLE_TLS_CERT_FILE=${CSFLE_TLS_CERT_FILE:-"$PARENT_DIR/x509gen/server.pem"}
In start-servers.sh, it’s passed to:
KMIP server (port 5698, line 44):
kms_kmip_server.py --ca_file $CSFLE_TLS_CA_FILE --cert_file $CSFLE_TLS_CERT_FILE --port 5698
The other HTTP servers (ports 9000/9001) intentionally use expired.pem and wrong-host.pem to exercise failure cases.
JAVA-6015