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

limit manifest size #155

Closed
strantalis opened this issue Aug 30, 2024 · 4 comments · May be fixed by #167
Closed

limit manifest size #155

strantalis opened this issue Aug 30, 2024 · 4 comments · May be fixed by #167
Assignees

Comments

@strantalis
Copy link
Member

strantalis commented Aug 30, 2024

In the go sdk it was detected that a protocol attack could happen by reading an excessively large manifest. We should introduce the same limits within the java-sdk.

Let's do a test that recreates the test done on the Go sdk, using a tdf with a large manifest, and observe the behavior in order to determine what is needed for this ticket.

REF: opentdf/platform#1385

Acceptance Criteria:

  • Test to make sure a manifest isn't larger than 10mb
@mkleene
Copy link
Contributor

mkleene commented Sep 16, 2024

I don't think we have this exact vulnerability since all of our reads are in terms of segments but we seemingly have a related one where we allocate a buffer based on a stated segment size.

The simplest fix for this is likely to fix a maximum (ideally encrypted) segment size. One doesn't currently appear to exist but it might be a good idea to add one, I think.

@strantalis

@strantalis
Copy link
Member Author

I don't think we have this exact vulnerability since all of our reads are in terms of segments but we seemingly have a related one where we allocate a buffer based on a stated segment size.

The simplest fix for this is likely to fix a maximum (ideally encrypted) segment size. One doesn't currently appear to exist but it might be a good idea to add one, I think.

@strantalis

@mkleene This has to do more with the manifest size. I don't think we restrict that right?

@mkleene
Copy link
Contributor

mkleene commented Sep 19, 2024

@strantalis that makes sense; I guess that if someone gives us a big TDF then it will make us consume large amounts of memory and that is still bad. I was referring to the protocol aspect where you could have a tiny TDF that advertises a large manifest and forces you to use memory.

The fixes from opentdf/platform#1536 might also be relevant. I also think that adding a test for mismatched declared manifest size might be a good idea because I bet we'd just crash.

@sujankota
Copy link
Contributor

#167

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 a pull request may close this issue.

4 participants