Skip to content

Conversation

Kaiede
Copy link
Contributor

@Kaiede Kaiede commented Mar 11, 2024

Purpose

Backup container would like to migrate to an integrated SSH client, unfortunately the easiest option only supports EC-based key exchange rather than both RSA and EC. As it's beneficial to support a broader set of clients anyways, this change adds an EC host key, allowing clients to use either type of key exchange.

Additionally moves the hostKey.pem file to be .hostKey.pem to hide it by default. Backwards compatibility is supported by checking and reading either file, and if the old file was read and/or a missing key was added, writing both keys back out to the new file. This includes handling a pem file that is missing one or both keys.

Validation

Confirmed that with changes, the server side reports supporting ECDSA and RSA key exchange, using an SSH client library (SwiftNIO's SSH client). Also confirmed that using the library achieves the expected result: being able to connect and provide input to the process wrapped by mc-server-runner.

Confirmed that an openssh client can still connect same as without the changes.

Checked backwards compatibility scenarios around the hostKey.pem file:

  • New file with just EC key
  • New file with just RSA key
  • New file with RSA and EC keys
  • Old file with just RSA key
  • Old file with RSA and EC keys

Copy link
Owner

@itzg itzg left a comment

Choose a reason for hiding this comment

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

Thanks!

@itzg itzg merged commit ab60683 into itzg:master Mar 11, 2024
@itzg
Copy link
Owner

itzg commented Mar 11, 2024

@itzg
Copy link
Owner

itzg commented Mar 11, 2024

...and released, https://github.com/itzg/mc-server-runner/releases/tag/1.12.0

If you could PR this into the minecraft-server image, then that would be great.

@Kaiede
Copy link
Contributor Author

Kaiede commented Mar 11, 2024

Will do that tonight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants