-
Notifications
You must be signed in to change notification settings - Fork 15
feat: chunks certification #24
base: main
Are you sure you want to change the base?
Conversation
src/main.rs
Outdated
@@ -348,6 +372,40 @@ async fn forward_request( | |||
} else { | |||
None | |||
}; | |||
|
|||
let decoded_body = decode_body(&http_response.body, encoding.clone()); | |||
let body_valid = match (certificate.clone(), tree.clone()) { |
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.
Is some kind of zip-bomb attack possible here, so that a max size needs to be prescribed?
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.
Thank you! Added MAX_BYTES_SIZE_TO_DECOMPRESS
param
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.
can we go further actually and not pass the body around, but just the sha?
What I'm thinking is we just decode 1K chunk of the body, add that 1K to the sha. Lather rinse and repeat up to EOF or MAX_BYTES_SIZE_TO_DECOMPRESS
(or maybe do 0..MAX_CHUNKS_TO_DECOMPRESS
). Then return the body_sha
and pass that into validate_body
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.
Great idea!
Left the possibility of partial hash calculation
let chunk_tree_digest = chunk_tree.digest(); | ||
|
||
if chunk_tree_digest != tree_sha { | ||
slog::trace!( |
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.
Maybe this is a warning not just trace?
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.
Same in 639 below.
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.
A similar design is already used nearby. If it's really necessary, then I'll change it :)
src/main.rs
Outdated
|
||
let decoded_body = decode_body(&body, encoding.clone()); | ||
validate_body( | ||
&certificate, |
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.
So many parameters. Maybe there's a ChunckedBody object there?
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.
Added Certificates
struct
Tests are not passing due to PR to agent-rs |
I really think we should separate the unzip change from this. Ideally also the refactors like Really appreciate all this work though! |
@Daniel-Bloom-dfinity body decoding is separated to another PR |
…nto feature/certified_chunks
Are there still plans to add this feature? |
That is a part of proposal about chunks certification (with backward compatibility).
These improvements are a proposal to improve the certification infrastructure around IC and might be considered as a recommendation for dfinity-team.
Goal
Make it possible to certify asset chunks. Validate chunk certificates on the service-worker and icx-proxy.
Why
At the moment, the service-worker and icx-proxy does not support the certification of chunkified files. Moreover, right now it is not possible to correctly stream chunkified and large audio and video files to the front-end.
This problems could be solved independently if it would be possible to install an additional service-worker in the certified zone of the domain
ic0.app
(for 206 partial http-request handling). But is is impossible because there is unable to place custom worker onic0.app
domain.Making your own custom player for audio and video is extremely difficult due to the large number of formats and non-native implementation.
Details
To make this possible, support for HTTP-range requests for
http_request
query method has been added. This is done to support native htmlaudio/video
element (which uses 206 partial http-request) and to determine the index of the chunk throught 206 partial http-request.Using 206 partial http-requests allows you to focus only on certification in the worker and icx-proxy.
Steps