Page MenuHomePhabricator
Differential D3633

[services] Backup - pull backup reactor - initialize get reactor
ClosedPublic
Actions

Authored by • karol on Apr 6 2022, 7:38 AM.
Tags
None
Referenced Files
F6235479: D3633.diff
Thu, Apr 24, 2:27 PM
Unknown Object (File)
Thu, Apr 17, 2:41 PM
Unknown Object (File)
Thu, Apr 17, 10:03 AM
Unknown Object (File)
Thu, Apr 17, 9:54 AM
Unknown Object (File)
Thu, Apr 17, 8:14 AM
Unknown Object (File)
Thu, Apr 17, 5:59 AM
Unknown Object (File)
Tue, Apr 15, 9:01 PM
Unknown Object (File)
Mon, Apr 14, 1:41 PM
View All Files
Subscribers
• abosh
• adrian
ashoat
atul
• benschac
tomek

Details

Reviewers
tomek
• max
Commits
rCOMM56ca2282dde4: [services] Backup - pull backup reactor - initialize get reactor
Summary

Depends on D3632

Adding basic structure for PullBackupReactor as well as the logic for initializeGetReactor.

Test Plan

The same as in D3535

Diff Detail

Repository
rCOMM Comm
Branch
asd
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• karol created this revision.Apr 6 2022, 7:38 AM
Herald added subscribers: • abosh, • benschac, atul and 3 others. · View Herald TranscriptApr 6 2022, 7:38 AM
• karol edited the summary of this revision. (Show Details)Apr 6 2022, 7:56 AM
• karol edited the test plan for this revision. (Show Details)
• karol added reviewers: tomek, • max.
• karol added a parent revision: D3632: [services] Backup - Implement get method in service blob client.
• karol added a child revision: D3637: [services] Backup - pull backup reactor - compaction - initialize.
Harbormaster completed remote builds in B7925: Diff 11107.Apr 6 2022, 8:38 AM
• karol requested review of this revision.Apr 6 2022, 8:38 AM
• karol planned changes to this revision.Apr 7 2022, 7:49 AM

rebase

• karol updated this revision to Diff 11179.Apr 7 2022, 7:50 AM
• karol edited the summary of this revision. (Show Details)

rebase

Harbormaster completed remote builds in B7975: Diff 11179.Apr 7 2022, 8:49 AM
tomek requested changes to this revision.Apr 8 2022, 2:55 AM
tomek added inline comments.
services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h
15

Are we using this? Please make sure that the imports are relevant

57

Let's include more details - describe what is missing

67–69

What's the purpose of this function? It is not implemented and not used in this diff.

This revision now requires changes to proceed.Apr 8 2022, 2:55 AM
• karol added inline comments.Apr 11 2022, 1:36 AM
services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h
15

This can be removed

57

sure

67–69

It is used from the parent class, this is a virtual method that is being called once when the object is initialized. You're right, there's no body of this function and it will throw. We may introduce it in a later diff.

• karol updated this revision to Diff 11333.Apr 12 2022, 1:19 AM

update

services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h
67–69

Ok I'm getting linker errors without this, I don't think it matters that much to spend more time on this, the body of this function is added in the next diff in this stack.

Harbormaster completed remote builds in B8106: Diff 11333.Apr 12 2022, 1:24 AM
tomek accepted this revision.Apr 12 2022, 9:13 AM
This revision is now accepted and ready to land.Apr 12 2022, 9:13 AM
Closed by commit rCOMM56ca2282dde4: [services] Backup - pull backup reactor - initialize get reactor (authored by • karol). · Explain WhyApr 13 2022, 5:22 AM
This revision was automatically updated to reflect the committed changes.
• karol added a commit: rCOMM56ca2282dde4: [services] Backup - pull backup reactor - initialize get reactor.
ashoat added inline comments.Apr 18 2022, 11:32 AM
services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h
30–32 ↗(On Diff #11410)

What's the reason for using shared_ptr for all of these? It seems unnecessary...

Revision Contents
Changeset List

PathSize
services/
backup/
docker-server/
contents/
server/
src/
Reactors/
server/
52 lines

Diff 11179

View Options

services/backup/docker-server/contents/server/src/Reactors/server/PullBackupReactor.h

  • This file was added.
// @flow
const ENCRYPTION_ALGORITHM = 'AES-GCM';
type EncryptionResult = {
bartekUnsubmitted
Not Done
Inline Actions

What about calling it EncryptedData? See my comment in decryptDatabaseFile

bartek: What about calling it `EncryptedData`? See my comment in `decryptDatabaseFile`
+iv: BufferSource,
+cipher: ArrayBuffer,
bartekUnsubmitted
Not Done
Inline Actions

I'd call it ciphertext to be consistent, but up to you

bartek: I'd call it `ciphertext` to be consistent, but up to you
};
function generateDatabaseCryptoKey(): Promise<CryptoKey> {
return crypto.subtle.generateKey(
{
name: ENCRYPTION_ALGORITHM,
length: 256,
},
false,
['encrypt', 'decrypt'],
);
}
function generateIv(): BufferSource {
bartekUnsubmitted
Not Done
Inline Actions
);
}
- function generateIv(): BufferSource {
+ function generateIV(): BufferSource {
return crypto.getRandomValues(new Uint8Array(12));

Nit

bartek: Nit
return crypto.getRandomValues(new Uint8Array(12));
}
async function encryptDatabaseFile(
data: ArrayBuffer,
key: CryptoKey,
): Promise<EncryptionResult> {
const iv = generateIv();
const cipher = await crypto.subtle.encrypt(
{
name: ENCRYPTION_ALGORITHM,
iv: iv,
},
key,
data,
);
return {
cipher,
iv,
};
}
async function decryptDatabaseFile(
cipher: ArrayBuffer,
key: CryptoKey,
iv: BufferSource,
bartekUnsubmitted
Not Done
Inline Actions
async function decryptDatabaseFile(
- cipher: ArrayBuffer,
+ { ciphertext, iv }: EncryptionResult,
key: CryptoKey,
- iv: BufferSource,
): Promise<ArrayBuffer> {

How about connecting cipher and iv by using the EncryptionResult type you defined above (and perhaps renaming the type)? In GCM mode, they always need to be provided together anyways

bartek: How about connecting `cipher` and `iv` by using the `EncryptionResult` type you defined above…
): Promise<ArrayBuffer> {
return crypto.subtle.decrypt(
{
name: ENCRYPTION_ALGORITHM,
iv,
},
key,
cipher,
);
}
export { generateDatabaseCryptoKey, encryptDatabaseFile, decryptDatabaseFile };