Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove AES and SubtleCrypto from WASM #74165

Merged
merged 5 commits into from
Aug 19, 2022
Merged

Remove AES and SubtleCrypto from WASM #74165

merged 5 commits into from
Aug 19, 2022

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Aug 18, 2022 •
edited
Loading

Fix #73858

Tasks from #73858

  • Remove the worker from WasmAppBuilder
  • Remove the call from startup.ts
  • Remove the DLLImports
  • Remove the C# code that is calling this
  • Remove the special js-module-crypto in AssetBehaviours typescrypt
  • Remove js-module-crypto handling in the SDK (replace it with js-module-threads would be useful)
  • Remove managed AES implementation

@eerhardt eerhardt added arch-wasm WebAssembly architecture area-System.Security labels Aug 18, 2022
@eerhardt eerhardt requested a review from bartonjs August 18, 2022 18:15
@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost ghost assigned eerhardt Aug 18, 2022
@ghost
Copy link

ghost commented Aug 18, 2022

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #73858

Tasks from #73850

  • Remove the worker from WasmAppBuilder
  • Remove the call from startup.ts
  • Remove the DLLImports
  • Remove the C# code that is calling this
  • Remove the special js-module-crypto in AssetBehaviours typescrypt
  • Remove js-module-crypto handling in the SDK (replace it with js-module-threads would be useful)
  • Remove managed AES implementation
Author: eerhardt
Assignees: -
Labels:

arch-wasm, area-System.Security
Milestone: -

@ghost
Copy link

ghost commented Aug 18, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Fix #73858

Tasks from #73850

  • Remove the worker from WasmAppBuilder
  • Remove the call from startup.ts
  • Remove the DLLImports
  • Remove the C# code that is calling this
  • Remove the special js-module-crypto in AssetBehaviours typescrypt
  • Remove js-module-crypto handling in the SDK (replace it with js-module-threads would be useful)
  • Remove managed AES implementation
Author: eerhardt
Assignees: -
Labels:

arch-wasm, area-System.Security
Milestone: -

lambdageek
lambdageek reviewed Aug 18, 2022
View reviewed changes
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/mono and src/tasks changes lgtm

src/tasks/WasmAppBuilder/WasmAppBuilder.cs Show resolved Hide resolved
@lewing lewing mentioned this pull request Aug 18, 2022
Merged
@lewing lewing requested a review from kg August 18, 2022 21:36
@eerhardt
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara
pavelsavara approved these changes Aug 19, 2022
View reviewed changes
Copy link
Member

@pavelsavara pavelsavara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeScript changes LGTM, I didn't check the rest.

@eerhardt eerhardt merged commit 4f7a096 into dotnet:main Aug 19, 2022
@eerhardt eerhardt deleted the RemoveSubtleCrypto branch August 19, 2022 13:52
eerhardt added a commit to eerhardt/runtime that referenced this pull request Aug 19, 2022
@eerhardt
Remove AES and SubtleCrypto from WASM (dotnet#74165)
b3a2a99 
* Revert 6a02d5d

* Revert relevant parts of 4222e69 and AES pieces of 8f75cc9

* Remove Subtle Crypto interop code

Fix dotnet#73858
@eerhardt eerhardt mentioned this pull request Aug 19, 2022
Closed
@eerhardt eerhardt mentioned this pull request Aug 19, 2022
Merged
carlossanlop pushed a commit that referenced this pull request Aug 20, 2022
@eerhardt
Remove AES and SubtleCrypto from WASM (#74165) (#74246)
e897e88 
* Revert 6a02d5d

* Revert relevant parts of 4222e69 and AES pieces of 8f75cc9

* Remove Subtle Crypto interop code

Fix #73858
@rzikm rzikm mentioned this pull request Aug 24, 2022
Merged
@bartonjs bartonjs added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Aug 26, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lambdageek lambdageek lambdageek left review comments

@bartonjs bartonjs bartonjs approved these changes

@pavelsavara pavelsavara pavelsavara approved these changes

@radical radical Awaiting requested review from radical

@lewing lewing Awaiting requested review from lewing lewing is a code owner

@marek-safar marek-safar Awaiting requested review from marek-safar

@kg kg Awaiting requested review from kg

Assignees

@eerhardt eerhardt

Labels
arch-wasm WebAssembly architecture area-System.Security needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove AES and SubtleCrypto from WASM
4 participants
@eerhardt @pavelsavara @lambdageek @bartonjs