Feature/recreate previous encryption #79
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #79 +/- ##
==========================================
+ Coverage 58.47% 58.86% +0.38%
==========================================
Files 6 6
Lines 1103 1128 +25
==========================================
+ Hits 645 664 +19
- Misses 330 334 +4
- Partials 128 130 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…e a new function NewCrypt4GHWriterWithRands() which is the one using previously generated WriterRands
emm1R
force-pushed
the
feature/recreate-previous-encryption
branch
from
6591be0
to
ceaf6c4
Compare
model/headers/headers.go
Outdated
| if h.Nonces != nil { | ||
| headerPacket.Nonce = h.Nonces[0] | ||
| h.Nonces = h.Nonces[1:] | ||
| } | ||
| marshalledHeaderPacket, err := headerPacket.MarshalBinary() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| h.Nonces = append(h.Nonces, headerPacket.Nonce) |
There was a problem hiding this comment.
Maybe copy h.Nonces to a local variable and operate on that instead of removing and adding back?
|
(I'm not sure I think it's a good idea with the It's also unclear/non-obvious to me what currently happens if a file is read and then written and if whatever does is the desired outcome. |
… a header/body as later there was append()
I assume that in that scenario NewCrypt4GHWriterWithRands() is not called so WriterRands is not used in anything. I personally am not dealing with that scenario right now. |
I can change that |
emm1R
force-pushed
the
feature/recreate-previous-encryption
branch
from
72bd62d
to
490c5ca
Compare
| @@ -213,8 +213,8 @@ func (h Header) MarshalBinary() (data []byte, err error) { | |||
| if err != nil { | |||
| return nil, err | |||
| } | |||
| for _, headerPacket := range h.HeaderPackets { | |||
| marshalledHeaderPacket, err := headerPacket.MarshalBinary() | |||
| for i := range h.HeaderPackets { | |||
There was a problem hiding this comment.
Not wrong, but this change does not seem needed if no longer keeping the nonces in the header.
There was a problem hiding this comment.
It is needed because otherwise the new Nonce is not reachable in other function.
There was a problem hiding this comment.
Changes in headerPacket are not reflected in h.headerPackets
|
I don't understand the use case here. It should be possible to calculate the checksum at the same time as encrypting and streaming the file? The ingest service does this, it copies (by streaming) a file from inbox (an io.Reader) to internal storage (an io.Writer) while calculating the checksum? https://github.com/neicnordic/sda-pipeline/blob/master/cmd/ingest/ingest.go#L244-L334 |
In our case, we don’t want to save the entire encrypted file anywhere in memory. We are streaming it away with POST request. If the encryption was performed only once, the checksum could only be calculated after the entire file had been sent but we want to send it at the same time. That’s why we hoped we could perform the enryption twice and get the same result each time. |
|
If that (checksum for sent file) is the only use case you have for now, I'm confident it can be done in a better way without these changes. The way interfaces are used in golang means it's real easy to hook in an intermediate/passthrough Writer that does something (e.g. sends output to hash computation as well) without any more memory being used. If it's still not clear how that could look, let me know and I'd be happy to cook up an example. |
|
I took some minutes proactively and wrote up an example at https://github.com/pontus/writerdemo |
This PR will make it possible to recreate a previous encryption. This is needed if one wishes to stream the encrypted results but also calculate a checksum for the file. The data key, and header and segment nonces prevented this from happening so these are now exposed through Crypt4GHWriter.Rands. To recreate the same encryption, these values need to be passed to NewCrypt4GHWriterWithRands().