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.

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

Enable mbedTLS Crypto object in official HTML5 builds #3574

Closed
Faless opened this issue Nov 19, 2021 · 4 comments · Fixed by godotengine/godot#61402
Closed

Enable mbedTLS Crypto object in official HTML5 builds #3574

Faless opened this issue Nov 19, 2021 · 4 comments · Fixed by godotengine/godot#61402

Comments

@Faless
Copy link

Faless commented Nov 19, 2021

Describe the project you are working on

The Godot Web Editor, and a Godot HTML5 game that requires cryptographic functions.

Describe the problem or limitation you are having in your project

The Crypto object is not available for HTML5 official builds since it requires the mbedTLS library which is disabled by default for the javascript platform.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The mbedTLS had originally only StreamPeerSSL and PacketPeerDTLS implementations, so it was not needed on the Web (since TLS/SSL is mostly handled by browsers), but it has expanded over time to include many useful cryptographic functions (hashing, signing, encryption) and it seems reasonable to now ship it along with Godot by default to allow using those engine features without having to recompile the export templates.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

We should simply disable the forced default in:

https://github.com/godotengine/godot/blob/master/platform/javascript/detect.py#L55

and enable it in the release builds.

There is an increased binary size cost to this decision though.

These are the size differences (in KiB) once enabled:

# Release
13252   bin/godot.javascript.opt.wasm
4116    bin/godot.javascript.opt.wasm.gz

# Relase / mbedTLS
13612   bin/godot.javascript.opt.wasm # 360 KiB increase, ~3% increase
4340    bin/godot.javascript.opt.wasm.gz # 224 KiB increase, ~5% increase

# Release + LTO
12440   bin/godot.javascript.opt.lto.wasm
3976    bin/godot.javascript.opt.lto.wasm.gz

# Release + LTO / mbedTLS
12752   bin/godot.javascript.opt.lto.wasm # 312 KiB increase, ~3% increase (2.5%)
4184    bin/godot.javascript.opt.lto.wasm.gz # 208 KiB increase, ~5% increase

If this enhancement will not be used often, can it be worked around with a few lines of script?

It's a build option.

Is there a reason why this should be core and not an add-on in the asset library?

It's a build option.

@Faless Faless added this to the 4.0 milestone Nov 19, 2021
@Faless Faless changed the title Enable mbedTLS Crypto on official HTML5 builds. Enable mbedTLS Crypto in official HTML5 builds. Nov 19, 2021
@akien-mga
Copy link
Member

I would have expected the size difference to be less than 3%, but well, it's still a reasonable increase to have feature parity with all other platforms. I agree that this should be enabled by default now that it provides features which are not readily available through the browser.

Users who want to optimize for size could still build custom templates with this module disabled, alongside many others they probably don't need for a size constrained game (e.g. all 3D code for a simple 2D game, etc.).

@RPicster
Copy link

RPicster commented Dec 7, 2021

I just have the problem that I developed on desktop and now that the first real release on WebAssembly is due, I ran into exactly this problem (on 3.4).

It's extremely annoying to miss this feature for one platform so I would greatly welcome a feature parity here, ideally with a backport 😲

@Calinou
Copy link
Member

Calinou commented Dec 7, 2021

It's extremely annoying to miss this feature for one platform so I would greatly welcome a feature parity here, ideally with a backport 😲

This will have to wait for 3.5, as patch releases are now dedicated to bug fixes only to avoid regressions. In the meantime, you can compile your own HTML5 export template 🙂

@Calinou Calinou changed the title Enable mbedTLS Crypto in official HTML5 builds. Enable mbedTLS Crypto object in official HTML5 builds Dec 7, 2021
@RPicster
Copy link

Hey there,
are there any news about this? I didn't see it in the 3.5 notes so far so I wanted to ask again how's the plan for this :)

@akien-mga akien-mga modified the milestones: 4.0, 3.x May 25, 2022
akien-mga added a commit to akien-mga/godot that referenced this issue May 25, 2022
Increases the size of the wasm by around 3% (~300-350 KiB).

This enables using the Crypto object for hashing, signing and encryption,
and therefore reduces the gap between the features of the HTML5 platform
and other platforms.

Closes godotengine/godot-proposals#3574.
akien-mga added a commit to godotengine/godot that referenced this issue May 25, 2022
Increases the size of the wasm by around 3% (~300-350 KiB).

This enables using the Crypto object for hashing, signing and encryption,
and therefore reduces the gap between the features of the HTML5 platform
and other platforms.

Closes godotengine/godot-proposals#3574.

(cherry picked from commit 3ff6d79)
Relintai pushed a commit to Relintai/pandemonium_engine that referenced this issue Jul 28, 2022
Increases the size of the wasm by around 3% (~300-350 KiB).

This enables using the Crypto object for hashing, signing and encryption,
and therefore reduces the gap between the features of the HTML5 platform
and other platforms.

Closes godotengine/godot-proposals#3574.

(cherry picked from commit 3ff6d794c0aac0365f236cd078b5e5aeea0d996e)
Riordan-DC pushed a commit to Riordan-DC/godot that referenced this issue Jan 24, 2023
Increases the size of the wasm by around 3% (~300-350 KiB).

This enables using the Crypto object for hashing, signing and encryption,
and therefore reduces the gap between the features of the HTML5 platform
and other platforms.

Closes godotengine/godot-proposals#3574.

(cherry picked from commit 3ff6d79)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants