fix: add SSH Key format conversion (PKCS → OpenSSH)

[BolajiOlajide]

Closes #124

This PR adds optional convert-ssh-keys input to convert SSH private keys from PKCS format (-----BEGIN PRIVATE KEY-----) to OpenSSH format (-----BEGIN OPENSSH PRIVATE KEY-----).

1Password stores SSH keys in PKCS8/PKCS1 format, but many SSH tools require OpenSSH format.

Why ssh-keygen instead of custom implementation?

The Terraform provider solved this with 207 lines of custom crypto code. I chose ssh-keygen mostly because of Typescript's lack of proper cryptographic logic. Also because this is easier to maintain and ssh-keygen comes pre-installed on GitHub-hosted runners.

Usage

- uses: 1password/load-secrets-action@v3
  with:
    convert-ssh-keys: true
  env:
    OP_SERVICE_ACCOUNT_TOKEN: ${{ secrets.OP_SERVICE_ACCOUNT_TOKEN }}
    SSH_PRIVATE_KEY: op://vault/server/private_key

[BolajiOlajide]

cc @volodymyrZotov

[volodymyrZotov]

@BolajiOlajide Thank you for such a quick turn around! 🤩 Great job!

I left some comments that explained how we can simplify implementation.

[volodymyrZotov]

Let's rename this to be more explicit about what exact format we want to convert to? Convert sounds too broad, and it's not really obvious for the people who is not familiar with ssh key formats what this option is for.

What about something like open-ssh-key-format? Please update all the places with the reference to convert-ssh-keys.

[volodymyrZotov]

Instead of supporting this custom implementation could you try sshpk package?

It would help to make implementation way simpler

export const convertPkcsToOpenSsh = async (
  pkcsKey: string,
): Promise<string> => {
  try {
    const key = sshpk.parsePrivateKey(pkcsKey, 'pem');
    return key.toString('ssh-private');
  } catch (error) {
    throw new Error(`Failed to convert key: ${error.message}`);
  }
};

[BolajiOlajide]

Oh thanks! I'm just learning about the sshpk library - thanks for sharing with me.

[volodymyrZotov]

@BolajiOlajide I have a better idea for the key conversion. Instead of having the function convertPkcsToOpenSsh, we can add ?ssh-format=openssh query parameter to the op reference (aka "op://Private/ssh keys/ssh key/private key?ssh-format=openssh"), that will make it to return SSHKey in OpenSSH format. (documentation)

[BolajiOlajide]

Oh this is quite good. That'd mean there won't be a need for this PR anymore. We could just add a documentation that for SSH keys that the user can specify the format they want it retrieved. What do you think?

[volodymyrZotov]

Agree, that could be an option to go which requires way less efforts.

Though, in long term perspective I'd like action handle that internally.

You can go either way now. Feel free just to update documentation mentioning how to get SSH key by adding query parameter, or if have desire/time you can implement the code that will handle it.

[BolajiOlajide]

Apologies , I've been dealing with some life issues.

I think going the route of appending the query parameter in code might not be the best. The reason is that if we append the parameter to all secrets when the convert-ssh: true, if there's a secret that's not a SSH private key, the op read fails.

And I think this is more likely to happen and won't be a good UX. Therefore, I'll recommend adding to the documentation, that if you want to get private keys in OpenSSH format, you should append the query manually. There should be no need to havre the extra convert-ssh option else it'd lead to errors when a user passes in a non-SSH item.

[volodymyrZotov]

Could you try to use sshpk package instead?

The same for isPkcsPrivateKey and isOpenSshPrivateKey functions.

[volodymyrZotov]

Let's create ssh.ts file and put all the ssh related functions there.

[volodymyrZotov]

Now, this function becomes so tightly coupled. Could I ask you please to refactor a bit and decouple the logic?

I'd like to have 3 separate functions:

1. readSecret(envName: string): string -> which only reads the secret and returns it
2. openSSHKeyFormat(key: string): string -> the one you already have in utils.
3. exportSecretValue(secret: string, shouldExportEnv: boolean): void -> which exports secret as env var or step output and marks it as sensitive if needed.

With that, the usage in loadSecrets function will be:

for (const envName of envs) {
		let secret = await extractSecret(envName);
         if isSSHKey(secret) && shouldConvertSshKeys {
               secret = await convertToOpenSSHFormat(key)
         }
         exportSecretValue(secret, shouldExportEnv)
}

[volodymyrZotov]

Closing this PR as workaround would be to manually add ?ssh-format=openssh query param to the op path. (documentation)

@BolajiOlajide thank you for your contribution 🙌! If you'd like to create another PR that update documentation with that, I'll be happy to review it!