Sign a commit if ssh secret key is provided

[yanganto]

This Pull Request fixes #1149

It changes the following:

• A ssh key path in user.signingKey of gitconfig will be used to sign the commit

I followed the checklist:

• [ ] I added unittests
• [ ] I ran make check without errors
• [ ] I tested the overall application
• [x] I added an appropriate item to the changelog

[extrawurst]

based on your last comment in the previous PR:

Just hard to find another crate with the same function but using syn2. This is the same issue as the previous point.

still cannot follow what you refer to here. gitui now only depends on syn2, there is no old syn1 in the dependencies anymore.

what I am asking is: what is needed to make CI not fail?

I think we need some slightly simpler UX for providing this. Is there a standard way how this key is provided for example to git (the normal cli)? maybe env variables? or is it also purely via cli args?

I will happy to use GITUI_SSH_KEY_PATH and GITUI_SSH_KEY_PASSWORD for the first iteration and moving forward from this.

I think we have some misunderstanding here. I am not saying lets introduce a bespoke env var that is gitui specific instead of a cli arg. My question is: how do other tools get fed the SSH key/pwd? Primarily the regular git cli and why can't we use the same way?

[yanganto]

based on your last comment in the previous PR:

Just hard to find another crate with the same function but using syn2. This is the same issue as the previous point.

still cannot follow what you refer to here. gitui now only depends on syn2, there is no old syn1 in the dependencies anymore.

what I am asking is: what is needed to make CI not fail?

https://github.com/extrawurst/gitui/actions/runs/7794027383/job/21254692240 Yes, it was an issue in previous PR, but it is okay now.

I think we need some slightly simpler UX for providing this. Is there a standard way how this key is provided for example to git (the normal cli)? maybe env variables? or is it also purely via cli args?

I will happy to use GITUI_SSH_KEY_PATH and GITUI_SSH_KEY_PASSWORD for the first iteration and moving forward from this.

I think we have some misunderstanding here. I am not saying lets introduce a bespoke env var that is gitui specific instead of a cli arg. My question is: how do other tools get fed the SSH key/pwd? Primarily the regular git cli and why can't we use the same way?

If the cli is good, I will pick these back and drop the env::var here.

The best practice for using an ssh password is to ask interactively with UI, such that the user can avoid writing down the password in a file or env. And the default ssh key will be ~/.ssh/id_rsa or ~/.ssh/id_ed25519, which will depend on the way the user generates his key. I think for the first iteration it is good to have a password here. If you think this is bad, I can remove the password feature, because password is not a requirement for ssh key.

[extrawurst]

And the default ssh key will be ~/.ssh/id_rsa or ~/.ssh/id_ed25519, which will depend on the way the user generates his key. I think for the first iteration it is good to have a password here. If you think this is bad, I can remove the password feature, because password is not a requirement for ssh key.

Ok lets make things easy then and scan for the default keys, no arg or env for now. Is there a git config that tells us that an ssh key is supposed to be used so we can show an error if we are supposed to use ssh and cannot find a key?

also if we know we shall encrypt via ssh and we find a default key do we know if that key requires a pwd? so that we can fail for the MVP in that case because we don't know the pwd

[extrawurst]

@yanganto but mostly we need to get the CI green

[yanganto]

And the default ssh key will be ~/.ssh/id_rsa or ~/.ssh/id_ed25519, which will depend on the way the user generates his key. I think for the first iteration it is good to have a password here. If you think this is bad, I can remove the password feature, because password is not a requirement for ssh key.

Ok lets make things easy then and scan for the default keys, no arg or env for now. Is there a git config that tells us that an ssh key is supposed to be used so we can show an error if we are supposed to use ssh and cannot find a key?

You misunderstand the mechanism. The commit can be signed with any ssh secret key, but only one. Github will use verify the commit only if the corresponding pub key is already set in Github by user. So we can not scan all the ssh key to select some one who is valid, because all key are valid and possible in use. The default key path will be different because user can use different kinds of crypto mechanism. We can have a default value as now, but we need a interface let user change it easier.

also if we know we shall encrypt via ssh and we find a default key do we know if that key requires a pwd? so that we can fail for the MVP in that case because we don't know the pwd

No, if we do not have a password, we still can sign the commit but it will not verified by GITHUB, because the bytes will not be different from a real one.

[extrawurst]

You misunderstand the mechanism

then lets go back to square one: how does it work with the git-cli, how does it get configured and why cant we use the same method?

[extrawurst]

as far as i can see it has to be configured like this: https://docs.github.com/en/authentication/managing-commit-signature-verification/telling-git-about-your-signing-key#telling-git-about-your-ssh-key

so we can just use these git configs to know what signing key to use and whether signing is supposed to happen: so lets use that

[yanganto]

git commit use following option to sign a commit with GPG git-commit -S[<keyid>], --gpg-sign[=<keyid>]

And the ssh sign is using ssh secret key but the header still gpgsig, and GITHUB or other repository will using the pub key.

There is not exactly an interface in git-cli for the ssh key. The way we are using now is -s. I think it is good enough now.

[yanganto]

as far as i can see it has to be configured like this: https://docs.github.com/en/authentication/managing-commit-signature-verification/telling-git-about-your-signing-key#telling-git-about-your-ssh-key

so we can just use these git configs to know what signing key to use and whether signing is supposed to happen: so lets use that

You can see the config is a pub key for verification not a secret key for a sign. Do we have an assumption that there is a secret key and the key is the pub key path remove .pub?

[yanganto]

By the way, the way to set git config with gpg.format ssh will not work as what you see on the web page.

└─[$] <git:(ssh*)> git commit -m 'test ssh with git'
error: unsupported value for gpg.format: ssh
fatal: bad config variable 'gpg.format' in file '.git/config' at line 28
└─[$] <git:(ssh*)> git --version
git version 2.32.0

So I think nobody write config in that way for now

[yanganto]

You may wonder why we need Clone trait on CliArgs Because clippy will cry on passing too many fields of CliArgs in run_app function. https://github.com/extrawurst/gitui/actions/runs/7888377158/job/21525757826

[yanganto]

@extrawurst It now is ready to review

[extrawurst]

You can see the config is a pub key for verification not a secret key for a sign.

this is exactly what every source I find about ssh-commit-signing tells you to do. to sign with your public key. what am I missing?

but don't take my word for it, see Gitlab docs, tower (slightly popular git GUI app) and this high ranking blog post here

[yanganto]

You can see the config is a pub key for verification not a secret key for a sign.

this is exactly what every source I find about ssh-commit-signing tells you to do. to sign with your public key. what am I missing?

but don't take my word for it, see Gitlab docs, tower (slightly popular git GUI app) and this high ranking blog post here

A good sign mechanism is to use a secret key and pass the public key to verify whether it is true or not. If you sign with a public key, then you need to pass the secret key to Github for verification. :roll_eyes: The commit in the current PR is signed by my SSH secret key with this code, and I set the public key on GitHub, so you can see there are green verified flags.

The blog post here is signed with a secret key, but he put the content of key in the signingKey, That is the same as what I did in this PR.

The ssh setting will need a git after 2.34. The legacy git will be broken when using it, so people may not set up the gitconfig with ssh. (I already use gitui to sign the commit, why do I need to update the git to the specific version?) Also, there is a lot of possible value in signingKey, the path to the pub key, the path to the secret key, the content of the SSH secret key, or possibly some content of gpg keys.

Does current gitui handle a global git config and overlayed with a local git config? I don't think the first iteration needs to handle these variations.

[extrawurst]

The blog post here is signed with a secret key, but he put the content of key in the signingKey, That is the same as what I did in this PR.

It literally says in the link above Simply use your public key

The ssh setting will need a git after 2.34.

Fine with me.

Lets use signingKey config no cli param for now. if user wants to use ssh singing signingKey and gpg.format ssh have to be configured.

Does current gitui handle a global git config and overlayed with a local git config

yes git2-rs (libgit2) make that pretty easy

[yanganto]

Hi @extrawurst I believe this is good to go.

[extrawurst]

@yanganto can you rebase again to make it up to date?

[yanganto]

Hi @extrawurst The master is merged into this branch, so the sign can be retained. :smile:

[yanganto]

KISS (Keep it simple and stupid) will be the best minuteness way I think.

This already is extendable to another kind of sign and is also welcome to refactor in the future.

	let id = match (
		config.get_entry("gpg.format").ok(),
		fetch_ssh_key(&config),
	) {
		(Some(f), Some(sk)) if f.value() == Some("ssh") => {},
		_ => (),
	}

The Sign trait in #1544 is beautiful and much more abstract, but it already takes more than one year. I hope we will not take another year from now. Also, I like to adapt the implementation of the Sign trait if it is already in the mainstream.

We are building a daily tool for developers and not a rocket to Mars. I believe having a good release pace, iterating, and easier customization for users will be great for this project.

[extrawurst]

So far this project was developed with high standards and I believe this is what makes it successful. I won’t stop now.

merging signing in any shape is a big responsibility and I am open to do it without the abstraction in order to refactor later but then we still need to make it work in all places that require signing to not confuse users that trust the quality of gitui

[thePanz]

I am testing this PR, here my workflow:

git clone [email protected]:yanganto/gitui.git --depth=1 --branch=ssh-sign-2
cd gitui
cargo build
./target/debug/gitui

from there I add a change to the repo, and try to commit, but the following error is displayed:

commit.gpgsign=true detected deactivate in your repo/gitconfig to be able to commit without signing

And my Git settings for that repository are the following:

[user]
    email = [email protected]
    name = thePanz
    signingkey = ~/.ssh/github.pub
[gpg]
    format = ssh
[commit]
    gpgsign = true

Is this expected? According to GIT docs, the commit.gpgsign=true tells GIT to automatically sign commits, which I guess should drive the usage of the signing feature or not. This PR looks like is ignoring that setting, and always signing when the signingKey is provided

[yanganto]

Hi @thePanz

Remove this setting that will work.

[commit]
    gpgsign = true

And I am surprised to see this restriction. That means if a user uses other git tools with gpgsign at the same time, gitui will refuse to work. 🤨

Anyway, this is fixed now.

[extrawurst]

@yanganto like you argued earlier we should not require the user to make changes in their config that works with vanilla git cli

[yanganto]

Yes, so I fixed this. I am just really surprise we have the disable alert, I through we did not have something like that.

[yanganto]

Hi @thePanz After the fix, you don't need to change your gitconfig. The previous commit is also signed with the same config as yours and I put it in the global.

[yanganto]

If I can design the behavior for the alert, I will just let the commit box in red with the title unsigned commit prompt the user but not restrict the user to do that. If @extrawurst you think this is a good one, please open an issue on it.

[extrawurst]

@thePanz please test it again

[thePanz]

@yanganto @extrawurst tested again:

1. the committing is now working, no error message is reported
2. the signature is not attached the commit tho

See the screenshot:

• the first commit (54d5f58) is done with gitui
• the second commit (86aba81) was done with git commit -m " Testing commit from git CLI"

the graph is obtained with git tree --show-signatures

[yanganto]

@extrawurst What is the next action item needs to be done in this PR?

[patka-123]

@yanganto doesn't the testing of @thePanz conclude that the signing is not working? They say "the signature is not attached the commit tho". So might want to fix that first.