gcs-sidecar/bridge: Do not enforce MaxMsgSize on incoming messages#2786
gcs-sidecar/bridge: Do not enforce MaxMsgSize on incoming messages#2786micromaomao wants to merge 1 commit into
Conversation
This was discovered as causing the UVM to crash when we try to load a long fragment. Currently, the LCOW gcs does not enforce this limit, and so we never hit this in LCOW. I think it should be fine to also not enforce it for WCOW, since DoS attack from the host is outside the scope of the confidential threat model, and this avoid arbitrary length limits on policy and fragments. Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
| } | ||
|
|
||
| n := header.Size | ||
| if n < prot.HdrSize || n > prot.MaxMsgSize { |
There was a problem hiding this comment.
inbox GCS transport has a buffer limit of 64kb, so maybe we can drop this check for messages handled strictly by gcs-sidecar (policy/fragments etc) and we can ensure the size on the rest of the messages?
|
@micromaomao - I don't love this. If a policy fragment is larger than 64kb it feels like we are going to need an API that lets you send chunks of it instead of ignoring the limits on the message size. |
Unless there is some other reason why we have to limit the message size to 64 KiB (aside from what Maksim pointed out above, but that can be fixed by only enforcing this limit for some messages / checking the length of the message we are about to forward to the inbox gcs and reject the request if it ends up being too long), I don't see a benefit to doing this. We can still have a limit, realistically containerd needs a limit on the fragment size to avoid DoSing the host (not sure if it already has a limit), and so we can just use that + some headroom. Realistically 10 MiB ought to be enough. |
|
I'm not sure why its so small at 64k but the issue is that if we send messages the inbox GCS cant handle this will never work. I think gRPC has a default 4MB receive size. What is the max of one of these buffers? I think I'd be ok proposing we modify the bridge max message size to 4MB receive size everywhere I agree. Will that be big enough? |
4MB should be enough too. |
4 participants
This was discovered as causing the UVM to crash when we try to load a long fragment. Currently, the LCOW gcs does not enforce this limit, and so we never hit this in LCOW. I think it should be fine to also not enforce it for WCOW, since DoS attack from the host is outside the scope of the confidential threat model, and this avoid arbitrary length limits on policy and fragments.