godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.07k stars 20.19k forks source link

Print an error if incorrectly loading a public key as if it was private #78763

Open RichardEllicott opened 1 year ago

RichardEllicott commented 1 year ago

Godot version

4.0.3.stable

System information

Windows 10

Issue description

the example in the docs:

https://docs.godotengine.org/en/stable/classes/class_crypto.html

doesn't make much sense in the "encryption" part of the example as it both encrypts and decrypts with the private key

normal in RSA i encrypt with someones public key, in order to send it to them for a handshake. I found how to save a public key from Godot therefore and wrote my own example (attached)

i have included an example where the faliure can be seen, i try to encrypt with a public key, by uncommenting you can prove it works with the private key

(please note also, i had to port that example to 4.0, if i get it working maybe i can update the doc with a more useful example)

Steps to reproduce

i have included a project that trys to encrypt with public key... it shows an error

Minimal reproduction project

godot_rsa_example.zip

heppocogne commented 1 year ago

Error message:

E 0:00:00:0620   show_rsa_bug.gd:35 @ _ready(): Error parsing key '-15616'.
  <C++ Error>    Condition "ret" is true. Returning: FAILED
  <C++ Source>   modules/mbedtls/crypto_mbedtls.cpp:74 @ load()
  <Stack Trace>  show_rsa_bug.gd:35 @ _ready()

The error code -15616 seems to be MBEDTLS_ERR_PK_PASSWORD_REQUIRED, according to mbed TLS v3.1.0 documentation. So, this would be a bug.

Faless commented 1 year ago

So the error is actually MBEDTLS_ERR_PK_KEY_INVALID_FORMAT (3D00, not 3C00) and it happens while reading the public key, so my guess is that the bug is either while saving or loading public keys.

Faless commented 1 year ago

So the problem is that var key = load("rsa.key") always expects a private key

This is also documented:

Note: path should be a "*.pub" file if public_only is true, a "*.key" file otherwise.

So in your code you want to change:

-var public_key_filename = "user://rsa.public.key"
+var public_key_filename = "user://rsa.pub"

Then clear your user data (so your code will regenerate keys) and it will work.

I think we could add a check when doing key.save("some.key", true) telling you that the file will not open correctly if you then load it with var mykey = load("some.key"), and to use some.pub instead. Or even to simply fail and return an error during save (as people tend to ignore warnings until they get a script error).