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

Use ed25519 instead of RSA for crypto keys #912

Open
frlan opened this issue Mar 6, 2024 · 3 comments
Open

Use ed25519 instead of RSA for crypto keys #912

frlan opened this issue Mar 6, 2024 · 3 comments
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements.

Comments

@frlan
Copy link
Contributor

frlan commented Mar 6, 2024

ed25519 is more secure than a 4096 RSA and I suggest to use it e.g. for passkey.pem.

@Half-Shot Half-Shot added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements. label Mar 6, 2024
@Half-Shot
Copy link
Contributor

Yeah, probably a good shout. We need to figure out a migration path though since everyone's data will be encoded in a certain way :)

@frlan
Copy link
Contributor Author

frlan commented Mar 6, 2024

For config.yml I made already a diff:

diff --git a/config.sample.yml b/config.sample.yml
index 7182a9b..67906b1 100644
--- a/config.sample.yml
+++ b/config.sample.yml
@@ -9,7 +9,7 @@ bridge:
   bindAddress: 127.0.0.1
 passFile:
   # A passkey used to encrypt tokens stored inside the bridge.
-  # Run openssl genpkey -out passkey.pem -outform PEM -algorithm RSA -pkeyopt rsa_keygen_bits:4096 to generate
+  # Run openssl genpkey -out passkey.pem -outform PEM -algorithm ed25519
   passkey.pem
 logging:
   # Logging settings. You can have a severity debug,info,warn,error
diff --git a/helm/hookshot/values.yaml b/helm/hookshot/values.yaml
index a4b1302..ab6f74c 100644
--- a/helm/hookshot/values.yaml
+++ b/helm/hookshot/values.yaml
@@ -212,7 +212,7 @@ hookshot:
       # secret: "!secretToken"
     passFile: passkey.pem
     # A passkey used to encrypt tokens stored inside the bridge.
-    # Run openssl genpkey -out passkey.pem -outform PEM -algorithm RSA -pkeyopt rsa_keygen_bits:4096 to generate
+    # Run openssl genpkey -out passkey.pem -outform PEM -algorithm ed25519 to generate
     #
     # bot:
       # (Optional) Define profile information for the bot user
diff --git a/src/config/Config.ts b/src/config/Config.ts
index 374c295..fa0198f 100644
--- a/src/config/Config.ts
+++ b/src/config/Config.ts
@@ -493,7 +493,7 @@ export class BridgeConfig {
     @configKey(`Permissions for using the bridge. See docs/setup.md#permissions for help`, true)
     public readonly permissions: BridgeConfigActorPermission[];
     @configKey(`A passkey used to encrypt tokens stored inside the bridge.
- Run openssl genpkey -out passkey.pem -outform PEM -algorithm RSA -pkeyopt rsa_keygen_bits:4096 to generate`)
+ Run openssl genpkey -out passkey.pem -outform PEM -algorithm ed25519 to generate`)
     public readonly passFile: string;
     @configKey("Configure this to enable GitHub support", true)
     public readonly github?: BridgeConfigGitHub;

which seems to work fine on my testing. But there are way more usages of RSA inside the code.

@Ma27
Copy link

Ma27 commented Oct 15, 2024

Btw for anybody else who tried this out and tried to upgrade to 5.3, this will break in 5.3+ with

Error reading private key: PrivateKey8(PublicKey(OidUnknown { oid: ObjectIdentifier(1.2.840.113549.1.1.1) }))

Solution until this is implemented is probably to stay on 5.2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements.
Projects
None yet
Development

No branches or pull requests

3 participants