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

Support new key types #346

Closed
nemchik opened this issue Oct 10, 2022 · 9 comments · Fixed by #347 or #352
Closed

Support new key types #346

nemchik opened this issue Oct 10, 2022 · 9 comments · Fixed by #347 or #352
Assignees
Labels
enhancement The issue is a feature request

Comments

@nemchik
Copy link

nemchik commented Oct 10, 2022

Is your feature request related to a problem? Please describe.
Currently only ssh-rsa key types are supported by ssham. There are a handful of other valid key types.

Describe the solution you'd like
Specifically, I would like to see support added for ed25519 key types. There are also sk- prefixed key types, which are generated using security keys, such as Yubikeys. These types of keys should be allowed to be imported, but not created within ssham (since creation would have to be done with the security key attached to the machine using a special flag with the ssh-keygen command).

The key types can be validated using regular expressions, as described here https://github.com/nemchik/ssh-key-regex

Describe alternatives you've considered
Not using ssham 😦
All of our keys are currently ed25519 rather than rsa and we'd like to try out ssham, but don't want to downgrade key types.

Additional context
I searched the repo and didn't really see where keys were being checked to confirm they are valid, but I received a message in the UI:

The public key must be a valid public key.

So I can see there is a check happening.

@pacoorozco pacoorozco self-assigned this Oct 13, 2022
@pacoorozco pacoorozco added the enhancement The issue is a feature request label Oct 13, 2022
@pacoorozco
Copy link
Owner

pacoorozco commented Oct 13, 2022

I never thought about supporting this type of keys, but It seems easy to include them, so I'm going to take a look in the next weeks to implement, at least, a PoC.

There are several places to change the support for "ed25519" keys, not only the validation. But your link to regex expressions will be definitely worth.

Feel free to contribute to it...

@pacoorozco
Copy link
Owner

Hi @nemchik

I've added the support for ed25519 public key... It's available since release v0.16.0.

Let me know how it works 🙏🏼

@nemchik
Copy link
Author

nemchik commented Oct 19, 2022

Hi @nemchik

I've added the support for ed25519 public key... It's available since release v0.16.0.

Let me know how it works 🙏🏼

I've just attempted to test this out (sorry for the delay) and it seems I am unable to create a new ed25519 key or import an existing ed25519 key.

Just to provide as much info as possible, here is what I did:

  • cd ssham (the directory where i cloned ssham when following the instructions https://ssham.pacoorozco.info/installation/docker-compose/
  • docker compose down
  • git pull
  •   docker run --rm --interactive --tty \
        --volume $PWD:/app \
        --user $(id -u):$(id -g) \
        composer install
  • docker-compose build
  • docker-compose up -d
  • Navigate to http://server.ip.address/keys/create (note: the bottom right of the screen shows Powered by SSH Access Manager v0.16.0
  • Attempt to create a new key: result is an rsa key (no option to create an ed25519 key
  • Attempt to import an existing ed25519 key: result is The public key must be a valid public key.

@pacoorozco pacoorozco reopened this Oct 21, 2022
@pacoorozco
Copy link
Owner

Hi @nemchik

ssham can only import ed25519 keys (it can't create them). I'm using this key to test the system. If your key is not working I'd like to take a look to it, to see if it can be a formatting problem. Do you mind to send me your public key?

@nemchik
Copy link
Author

nemchik commented Oct 24, 2022

The key I am trying to use is:

ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMvGJd4ZOE4FGRRjE845A6onBqJFyIqXmSWsQOWXxdss dt20038\dt20038@DT20038

I also tried with the key you linked:

ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIJWuYOcBGX/sfsSLBweKaIQAkzhnw3rqLiPddoqxj74z phpseclib-generated-key

and got the same error

image

@pacoorozco
Copy link
Owner

pacoorozco commented Oct 25, 2022

Hi @nemchik

Both keys are working using the code that resides in the main branch. Have you tried to use the provided docker-compose?

Can you follow these instructions? I want to be sure that is not something related your system.

Screenshot_20221025_150948

@nemchik
Copy link
Author

nemchik commented Oct 25, 2022

I am using the provided docker compose. I initially setup my environment using the instructions you linked. Per my comment, after the setup the only thing I have done outside of the instructions is git pull inside my ssham directory to update from v0.15.2 to v0.16.0 after you had released it, and then rebuild the containers.

@nemchik
Copy link
Author

nemchik commented Oct 25, 2022

I repeated the steps in my comment from earlier and it seemed like the composer step ran more actions than previous (I assume it updated dependencies). After that I was able to successfully add my ed25519 key!

One thing I noticed is the SSH public key is displayed in the UI with an incorrect comment. No matter what key is added, the comment is always phpseclib-generated-key

image
image

Note that when I added the first key (top screenshot) I entered it as exactly:

ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMvGJd4ZOE4FGRRjE845A6onBqJFyIqXmSWsQOWXxdss dt20038\dt20038@DT20038

and the page then shows it as:

ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMvGJd4ZOE4FGRRjE845A6onBqJFyIqXmSWsQOWXxdss phpseclib-generated-key

The comment is really only cosmetic, but useful if you use it to identify which machine the key was created on and/or user accounts or email addresses.

@pacoorozco
Copy link
Owner

As you have already said, the key comment (phpseclib-generated-key) was not intended to be used. So that's why is not preserved. Maybe it would be worth to have some control over it, and I'm thinking in two changes:

  • Using the key name in order to generate the key comment.
  • Allow more freedom to set the key name (which currently is a username)

I'm going to implement the first one in #353 and maybe we can discuss the second one in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants