-
Notifications
You must be signed in to change notification settings - Fork 21
feat(#448): support encrypting submission payload #766
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
Open
garethbowen
wants to merge
23
commits into
main
Choose a base branch
from
448-submission-encryption
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
7b1f510
feat(#448): support encrypting submission payload
garethbowen 721dea4
backup
garethbowen 4b3a81c
progress
garethbowen 1c5763a
iv works
garethbowen d0d8bd0
correct instanceid
garethbowen 5aedd2f
successful decryption!
garethbowen 085e470
quaratine the dangerous code, and clean things up
garethbowen 399aa84
fix compile step
garethbowen f48efc7
encrypt attachments also
garethbowen 7fd202f
make test pass
garethbowen 9b0a118
refactor
garethbowen f10b284
add end of file new line
garethbowen bcdc8c3
lint
garethbowen 78d1796
define significant bytes to remove erroneous padding for attachments
garethbowen ad727e5
cleanup
garethbowen 0b4a7c1
changeset and feature matrix
garethbowen e18c44b
typos
garethbowen ec47969
review feedback
garethbowen f6b384d
review feedback
garethbowen 463aa02
lint
garethbowen 711f1fb
review feedback
garethbowen 2e24db8
remove empty test
garethbowen 2e9c7a3
review feedback
garethbowen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| '@getodk/xforms-engine': minor | ||
| '@getodk/scenario': minor | ||
| '@getodk/common': minor | ||
| '@getodk/web-forms': minor | ||
| --- | ||
|
|
||
| Added support for submission encryption |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <?xml version="1.0"?> | ||
| <h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema"> | ||
| <h:head> | ||
| <h:title>encrypted</h:title> | ||
| <model> | ||
| <submission base64RsaPublicKey="MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsrq8xrFswX/Ht1X4UVeLjupqz8hCMo+GR3hwqZdx7BQjvw9KBcCL+J6CI/2yyTqprVmeWZZu/i9qjuYcyWir3ZonZIDaVvClP8LN7e0+0SgQgEV+v9bjGVTDMQIKY2vq2ZNEbuy4UAHFLJCwGaUE370w76r/Da4YbAgfGVQn1sHarJ8Zp1o/6RE1IckxA8L2spo8oSU23KnttLIaR2qIS7mY+BkZPItyyNjulpJUZlxf4AgO7T8S4grmOC5TW4laB25vjbPw4KzB3L8bm+oK5JjlocazOiyUVDz8UwYMQke4ybEwSJbu3gl7DJzlwwQ1u3AbtjZk2T7LKUotrkVzAQIDAQAB"/> | ||
| <instance> | ||
| <encrypted id="encrypted"> | ||
| <question/> | ||
| <meta> | ||
| <instanceID/> | ||
| </meta> | ||
| </encrypted> | ||
| </instance> | ||
| <bind nodeset="/encrypted/question" type="string"/> | ||
| <bind jr:preload="uid" nodeset="/encrypted/meta/instanceID" readonly="true()" type="string"/> | ||
| </model> | ||
| </h:head> | ||
| <h:body> | ||
| <input ref="/encrypted/question"> | ||
| <label>Question 1</label> | ||
| </input> | ||
| </h:body> | ||
| </h:html> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import { getBlobText } from '@getodk/common/lib/web-compat/blob.ts'; | ||
| import { | ||
| bind, | ||
| body, | ||
| head, | ||
| html, | ||
| input, | ||
| label, | ||
| mainInstance, | ||
| model, | ||
| t, | ||
| title, | ||
| } from '@getodk/common/test-utils/xform-dsl/index.ts'; | ||
| import type { XFormsElement } from '@getodk/common/test-utils/xform-dsl/XFormsElement.ts'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { Scenario } from '../src/jr/Scenario.ts'; | ||
|
|
||
| describe('Form submission encryption', () => { | ||
| const DEFAULT_INSTANCE_ID = 'uuid:TODO-mock-xpath-functions'; | ||
|
|
||
| // prettier-ignore | ||
| type SubmissionFixtureElements = | ||
| | readonly [] | ||
| | readonly [XFormsElement]; | ||
|
|
||
| interface BuildSubmissionPayloadScenario { | ||
| readonly submissionElements?: SubmissionFixtureElements; | ||
| } | ||
|
|
||
| const buildSubmissionPayloadScenario = async ( | ||
| options?: BuildSubmissionPayloadScenario | ||
| ): Promise<Scenario> => { | ||
| const scenario = await Scenario.init( | ||
| 'Encrypted', | ||
| html( | ||
| head( | ||
| title('Encrypted'), | ||
| model( | ||
| mainInstance(t('data id="encrypted"', t('inp', 'test'), t('meta', t('instanceID')))), | ||
| ...(options?.submissionElements ?? []), | ||
| bind('/data/inp').required(), | ||
| bind('/data/meta/instanceID').calculate(`'${DEFAULT_INSTANCE_ID}'`) | ||
| ) | ||
| ), | ||
| body(input('/data/rep/inp', label('inp'))) | ||
| ) | ||
| ); | ||
|
|
||
| return scenario; | ||
| }; | ||
|
|
||
| it('includes a form-specified `base64RsaPublicKey` as encryptionKey', async () => { | ||
| const base64RsaPublicKey = | ||
| 'MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwkP+HQEqkyb4HPLOekvn6imYW6Ze2dF2sLCspnzimOnbiF7C1mcd01xiau+9WgU23kM35URhBQVbDHtbQMgZL/Ol+xdA0zdbcUW00Z7EkYmM4sGu4wwJA2eQ6yhBbY2np+kDTvmVHlhP8DDYsXJKqtm+8bXlI36qjVgkVPXjT9YNAA4vRxPReP5wuXHrMGjclPyU6SlFZZm8QLknYV9cmGh1CquKxK7/hIoGIZ3j+edh2GZg8XJo3ZkgAwOwNUqF9b4kXw+tnbpqLXfcETX3fp6iXqLqNMt3E1MXXMnePfDqsa9wrcykUMKfxLXF/EyhIZ+2+iBoyRKeIkExwJRMdQIDAQAB'; | ||
| const scenario = await buildSubmissionPayloadScenario({ | ||
| submissionElements: [t(`submission base64RsaPublicKey="${base64RsaPublicKey}"`)], | ||
| }); | ||
| const submissionResult = await scenario.prepareWebFormsInstancePayload(); | ||
|
|
||
| expect(submissionResult.submissionMeta).toMatchObject({ | ||
| encryptionKey: base64RsaPublicKey, | ||
| }); | ||
|
|
||
| expect(submissionResult.data.length).to.equal(1); | ||
| const entries = submissionResult.data[0].entries(); | ||
|
|
||
| const [submissionFilename, file] = entries.next().value!; | ||
| expect(submissionFilename).to.equal('xml_submission_file'); | ||
| const submission = await getBlobText(file); | ||
| expect(submission).to.contain( | ||
| '<data xmlns="http://opendatakit.org/submissions" encrypted="yes" id="encrypted">' | ||
| ); | ||
| expect(submission).to.contain('<encryptedXmlFile>submission.xml.enc</encryptedXmlFile>'); | ||
| expect(submission).to.contain( | ||
| '<meta xmlns="http://openrosa.org/xforms"><instanceID>uuid:TODO-mock-xpath-functions</instanceID></meta>' | ||
| ); | ||
|
|
||
| const [encodedFilename] = entries.next().value!; | ||
| expect(encodedFilename).to.equal('submission.xml.enc'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
15 changes: 15 additions & 0 deletions
15
...ges/xforms-engine/src/lib/client-reactivity/instance-state/quarantine/README.md
|
garethbowen marked this conversation as resolved.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Encryption | ||
|
|
||
| > [!CAUTION] | ||
| > Custom encryption is a bad idea. Do not use this unless absolutely necessary. | ||
|
|
||
| > [!CAUTION] | ||
| > Modification of this code requires great care as a bug in the encryption algorithm will make submissions unrecoverable. | ||
|
|
||
| The code in this directory implements the [ODK Spec](https://getodk.github.io/xforms-spec/encryption) which is very particular about how it's done so as to be compatible with other implementations. | ||
|
|
||
| ## Implementation | ||
|
|
||
| The symmetric encryption parts of the spec are implemented using CryptoJS because the particular algorithm required by the spec is not supported by Subtle Crypto, and we use CryptoJS elsewhere. | ||
|
|
||
| The asymmetric components of the spec are implemented using the [Subtle Crypto web spec](https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto) because CryptoJS doesn't implement asymmetric encryption. |
44 changes: 44 additions & 0 deletions
44
packages/xforms-engine/src/lib/client-reactivity/instance-state/quarantine/asymmetric.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| /** | ||
| * WARNING: DO NOT USE | ||
| * | ||
| * More info: README.md | ||
| */ | ||
| const ASYMMETRIC_ALGORITHM = 'RSA-OAEP'; | ||
| const HASH_FUNCTION = 'SHA-256'; | ||
| const KEY_FORMAT = 'spki'; | ||
|
|
||
| // Equivalent to "RSA/NONE/OAEPWithSHA256AndMGF1Padding" | ||
| const rsaEncrypt = async (symmetricKey: Uint8Array<ArrayBuffer>, publicKey: CryptoKey) => { | ||
| const encrypted = await crypto.subtle.encrypt( | ||
| { name: ASYMMETRIC_ALGORITHM }, | ||
| publicKey, | ||
| symmetricKey | ||
| ); | ||
| return btoa(String.fromCharCode(...new Uint8Array(encrypted))); | ||
|
alxndrsn marked this conversation as resolved.
|
||
| }; | ||
|
|
||
| const generatePublicKey = async (encryptionKey: string) => { | ||
| const binaryKey = atob(encryptionKey); | ||
| const data = new Uint8Array(binaryKey.length); | ||
| for (let i = 0; i < binaryKey.length; i++) { | ||
| data[i] = binaryKey.charCodeAt(i); | ||
| } | ||
| return await crypto.subtle.importKey( | ||
| KEY_FORMAT, | ||
| data, | ||
| { | ||
| name: ASYMMETRIC_ALGORITHM, | ||
| hash: HASH_FUNCTION, | ||
| }, | ||
| false, | ||
| ['encrypt'] | ||
| ); | ||
| }; | ||
|
|
||
| export const getEncryptedSymmetricKey = async ( | ||
| encryptionKey: string, | ||
| symmetricKey: Uint8Array<ArrayBuffer> | ||
| ): Promise<string> => { | ||
| const publicKey = await generatePublicKey(encryptionKey); | ||
| return await rsaEncrypt(symmetricKey, publicKey); | ||
| }; | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
There doesn't seem to be any test here that the encrypted content can be de-crypted. Is that expected?
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.
Yes that is the glaring hole in this PR... The reason is that WF doesn't currently have the testing infrastructure to start a Central instance to test against. I think we should build that out for this and many other tests that would benefit from full e2e testing. However if the code ends up in the central-frontend repo a lot of this work is already done, so I'm delaying it till then. Issue raised to capture this: #775
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.
What about copy/pasting the central code here? Or cloning the central repo, and running the relevant code directly with node?