fix(sdk): DoS protection through TDF segment sizes#1536
Merged
Conversation
This fixes three DoS conditions discovered: * Specifying a very large segment size which could result in the SDK allocating a large buffer in memory * Specifyin a very small segment size which could result in excessive cpu and io overheads * Specifying a size in the manifest which will overflow, resulting in a `panic` when a negative byte slice is allocated Unit and fuzz testing was expanded to cover these conditions.
jentfoo
force-pushed
the
jent/tdf_segment_size_fix
branch
from
September 10, 2024 16:59
68de773 to
643918a
Compare
Member
|
Following Postel's law, we should probably accept a wider range of values in decrypt than we allow in encrypt. Basically, anything that doesn't break the system should be acceptable on decrypt, for some suitable definition of break. |
dmihalcik-virtru
approved these changes
Sep 17, 2024
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Oct 1, 2024
🤖 I have created a release *beep* *boop* --- ## [0.3.13](sdk/v0.3.12...sdk/v0.3.13) (2024-10-01) ### Features * **sdk:** Add namesapce grants to key splitting ([#1512](#1512)) ([d9a07f8](d9a07f8)) ### Bug Fixes * **ci:** Fix negative assertion test case false positive ([#1550](#1550)) ([ef40bdb](ef40bdb)) * **core:** Add NanoTDF KID padding removal and update logging level ([#1466](#1466)) ([54de8f4](54de8f4)), closes [#1467](#1467) * **core:** Autobump sdk ([#1513](#1513)) ([03fba13](03fba13)) * **core:** Autobump sdk ([#1577](#1577)) ([df7466b](df7466b)) * **core:** only strip nano kids to the right ([#1475](#1475)) ([ae8d8a2](ae8d8a2)) * **core:** Store attribute specific grants to key cache ([#1507](#1507)) ([8bc0a98](8bc0a98)) * **sdk:** DoS protection through TDF segment sizes ([#1536](#1536)) ([d506734](d506734)) * **sdk:** Fix nanotdf ECC mode bitfield ([#1551](#1551)) ([58a76ad](58a76ad)) * **sdk:** Fix possible panic for `AttributeValueFQN.Prefix()` ([#1472](#1472)) ([144aeda](144aeda)) * **sdk:** Granter offline mode with namespaces and keycache ([#1542](#1542)) ([ecd41f4](ecd41f4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
jentfoo
added a commit
to opentdf/java-sdk
that referenced
this pull request
Nov 27, 2024
This matches the protections introduced in the go SDK PR: opentdf/platform#1536
jentfoo
added a commit
to opentdf/java-sdk
that referenced
this pull request
Nov 27, 2024
This matches the protections introduced in the go SDK PR: opentdf/platform#1536
jentfoo
added a commit
to opentdf/java-sdk
that referenced
this pull request
Dec 3, 2024
This matches the protections introduced in the go SDK PR: opentdf/platform#1536
jentfoo
added a commit
to opentdf/java-sdk
that referenced
this pull request
Dec 4, 2024
This change includes a variety of fixes found with fuzz testing. * Protocol Exception improvements - `NullPointerExceptions` have been converted to other exception types. Please provde feedback on if other exception types should be used for these cases. In `Fuzzing.java` now also serves to define in testing what exception types are acceptable. The `catch` specifically lists the types of exceptions that were discovered for each API call, and `throws` are checked exceptions that are not expected to be possible. As a future improvement we may want to refine this list further and better document what exceptions happen under what conditions. For now I thought it was best to start with just the `NullPointerException` cases. Since these cases were numerous, these changes span multiple commits, with each commit focused on a specific area of the protocol. * Protocol DoS Fixes - The only memory consumption issue discovered was the [counterpart found in the go sdk](opentdf/platform#1536). A matching fix with the same defaults was implemented here in the java sdk. * Finally the testing itself is added as `Fuzzing.java` executed through `sdk/fuzz.sh`. This script is long running, and there are occasional Jazzer failures which are not believed to be real deficiencies (timeouts when `.position()` is called on the stream). For that reason this testing needs to be done manually, and not expected to be included in CI * A few optimizations and clarity improvements were also included, as they were noticed while generally trying to get familiar with the codebase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes three DoS conditions discovered:
panicwhen a negative byte slice is allocatedUnit testing and fuzz testing was expanded to cover these conditions. This PR fixes #1535