Community
Participate
Working Groups
Setting 'user.signingkey' to a subkey returns the master key for that subkey instead. For example my current GPG key is as follows: > pub 4096R/90815215 2019-06-12 [expires: 2020-10-19] > uid Roan Hofland <roan.hofland@hotmail.com> > sub 4096R/CD04E21C 2019-10-20 Here 90815215 is my master key, the private key for my master key is stored offline and is thus not available. Therefore I have my signing key set to CD04E21C in .gitconfig. > [user] > name = Roan Hofland > email = roan.hofland@hotmail.com > signingkey = CD04E21C Using git on the command line this setup works just fine. > git commit -a -S -m "Test" > > You need a passphrase to unlock the secret key for > user: "Roan Hofland <roan.hofland@hotmail.com>" > 4096-bit RSA key, ID CD04E21C, created 2019-10-20 (main key ID 90815215) However Eclipse using JGit and EGit will instead ask me for the password to key 90815215. Which is the master key for the subkey I configured to use for signing. To further debug this issue I downloaded and setup a workspace for EGit and JGit. I then narrowed down the issue to the 'findPublicKeyByKeyId' method in the org.eclipse.jgit/src/org/eclipse/jgit/lib/internal/BoundCastleGpgKeyLocator class. For a given KeyBlob this method will run though all the KeyInformation objects in it. Then once an information object is found with a fingerprint that ends with the requested signing key it'll return the first public key for the KeyBlob. From my testing it seems that the first public key for such a KeyBlob is always the public key of the master key and not the subkey that was requested. If I change the following line of code in that method: > return getFirstPublicKey(keyBlob); To this: > return ((PublicKeyRingBlob) keyBlob).getPGPPublicKeyRing() > .getPublicKey(keyInfo.getFingerprint()); Then it will correctly return the public key for the subkey that was requested. If this is actually an acceptable solution then I don't mind figuring out how to submit a patch. However as it stands I don't know why this was setup to always return the public key for the first key. Therefore before I go ahead and submit a patch (that might otherwise introduce a bug of its own) I would like to bring this up for discussion.
Looks like an oversight in BouncyCastleGpgKeyLocator, and I think you're on the right track. I think instead of getFirstPublicKey() there should be two variants: private PGPPublicKey getPublicKey(KeyBlob blob, byte[] fingerprint) { return ((PublicKeyRingBlob) blob).getPGPPublicKeyRing() .getPublicKey(fingerprint); } and private PGPPublicKey getSigningPublicKey(KeyBlob blob) { // For the userId case. Unclear to me; probably needs to iterate // over the keys and return the first S key found; otherwise an // SC key? And null if neither found?? } @Gunnar: does that sound right? @Roan: if you could supply a patch that'd be great. Please not that we can accept patches only via Gerrit. See https://wiki.eclipse.org/EGit/Contributor_Guide#Contributing_Patches for some help with that.
New Gerrit change created: https://git.eclipse.org/r/151413
New Gerrit change created: https://git.eclipse.org/r/151412
I believe I should have made a change request for both variants. The first one is the key lookup as suggested. For the second one I went with your suggestion. For any user ID the first public key was used. Now the first key that is not the master key and has the signing flag set is returned. Only if such a key doesn't exist the master key is returned. If the master key doesn't have the signing flag set either then null is returned. I'm especially uncertain about the implementation for this one since I'm not that familiar with bouncycastle. Both changes together resolve the issue I was having with my key (the first when user.signingkey was set, the other when it was not set). As this is the first time I'm contributing anything, please tell me if I did something incorrectly aswell.
Sounds good to me! Thanks Roan for putting in the effort and contributing a patch. :)
Gerrit change https://git.eclipse.org/r/151412 was merged to [master]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=68b8317a09705aa90fc4a4ff1321b7cb4bd726e7
Gerrit change https://git.eclipse.org/r/151413 was merged to [master]. Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=0902e060f737324434ec019f7d68d1320f733416
(In reply to Roan Hofland from comment #4) > I believe I should have made a change request for both variants. No, that was perfect like this. That way we could revert the changes individually if either of them should cause problems. (Not that I think they will, looks all reasonable to me.) > As this is the first time I'm contributing anything, please tell me if I did > something incorrectly aswell. All perfect! Thanks a lot, Roan!
Glad I could help out :)
