git: Allow signing of commits - #376

Do you want to review this by itself, or in the context of the entire keys add/rm/validation functionality? We do not plan other changes to this signing of the commits part.

Hey, sorry I've been quite disrupted lately.

Yes, this would be better with the entire thing. With only this it doesn't enable a lot while creating more failure opportunities so I'm wary of merging it without the rest. I'll do a quick review though.

Not sure if you came across #130 and #55, have a look :)

cc'ing from #383 "DOC,UBY: What do I specify for avatarURL? Gravatar? W3C DID?"

https://www.w3.org/TR/did-core/ ::

Decentralized identifiers (DIDs) are a new type of identifier that enables verifiable, decentralized digital identity. A DID identifies any subject (e.g., a person, organization, thing, data model, abstract entity, etc.) that the controller of the DID decides that it identifies. These new identifiers are designed to enable the controller of a DID to prove control over it and to be implemented independently of any centralized registry, identity provider, or certificate authority. DIDs are URLs that associate a DID subject with a DID document allowing trustable interactions associated with that subject. Each DID document can express cryptographic material, verification methods, or service endpoints, which provide a set of mechanisms enabling a DID controller to prove control of the DID. Service endpoints enable trusted interactions associated with the DID subject. A DID document might contain semantics about the subject that it identifies. A DID document might contain the DID subject itself (e.g. a data model).

This document specifies a common data model, a URL format, and a set of operations for DIDs, DID documents, and DID methods.

https://github.com/topics/decentralized-identifiers?l=go

https://github.com/decentralized-identity/github-did#what-can-i-do-with-my-did

Use your DIDs to test Linked Data Signatures, such OpenPgpSignature2019 which is currently being developed. When DID Documents are signed, they include a proof attribute, which is used to provide proof that someone controlled the private key associated with the public key listed in the did document at the created datetime.

... GPG keyservers could also solve for PKI in a centralized way.

GitHub and GitLab already expose SSH and GPG public keys if they're added to a username:

• API: https://api.github.com/users/defunkt
  • avatar_url, gravatar_id
• SSH: https://github.com/defunkt.keys
• GPG: https://github.com/defunkt.gpg
• GPG: https://api.github.com/users/defunkt/gpg_keys

GitLab

• API: https://docs.gitlab.com/ee/api/users.html
• SSH: https://gitlab.com/username.keys
• GPG keys url: https://gitlab.com/gitlab-org/gitlab/-/issues/21584

Keybase

• API: https://keybase.io/docs/api/1.0/call/user/pgp_keys.asc
• GPG: https://keybase.io/chris/pgp_keys.asc
• SSH: SSH public keys keybase/keybase-issues#710

@MichaelMure
I added a 3rd commit "Verify signed identities".

I'm not sure what verification logic would be common to git-bug and git-ticket (hah, good thing the name is different). I pushed the verification logic we'll use so you can have a look. It's not yet merged in git-ticket. If there is something common and reusable, I could separate the logic from this 3rd commit.

I introduced go-git to be able to read the commits to be verified. The go-git repo is added as repo2 along repo repository.ClockedRepo in RepoCache, could not come up with anything better.

The very first commit in the identities history is very important. It is of course a commit which creates a new identity. The validation logic requires the newly created identity to have a key and be signed with that key. As long as the commits are signed, it's not strictly necessary for the key to be introduced in the very first commit, but I changed git-bug user create to be able to specify a key and have it included in this first commit. When the identities are validated, the fingerprint of the first key is reported like this:

$ b validate
first commit signed with key: C77E1D7542889EC0E45BA88899DA3BE167DA2410

When checking a git reference in the main repo, the key introduced by the first commit guarantees that everything is coming from us.

I made a small change to Identity.Commit to increase the lamport time so each identity version has a different lamport time so they can be sorted. The logic requires the lamport time for each version introducing/removing keys to be distinct. This might bite us, but we'll see.

The validate logic then sorts the identity versions by lamport time and iterates them. A keyring is built gradually and used to verify each commit before applying the key changes to the keyring.

The keyring is built as an openpgp.EntityList which is just a list of openpgp.Entity. Each Entity holds a single key. Technically, we could use a single Entity per git-bug Identity and add multiple keys to it and revoke them, but I found it simpler to have each Entity responsible for a single key.

When building the keyring, the time of the commit adding the key ends up in Identity.SelfSignature.CreationTime. The time of the commit removing a key, if any, is used to calculate the key lifetime stored in Identity.SelfSignature.KeyLifetimeSecs. The creation time and the expiry time are used in verifyCommitSignature for checking both identities and normal commits (from the main repo). This means for identities both the lamport time and the unix time are checked.

I had to expose private fields such as Identity.Versions. Maybe it can be done more cleanly.

TODO: I should move at least identity/key_test.go to the previous commit, maybe some more small stuff.

We could have used key fingerprints instead of key ids to identify the keys, but it seemed simpler to use the key id. The key fingerprint is composed of the key id and some other bits.

TL;DR: The first two commits would make sense to merge into git-bug, maybe also some common part of validate functionality if any.

Please have a look.

Any requests?

@MichaelMure In light of our validation logic which will never make it into git-bug, could you please comment on the first two commits if they are roughly ok and could be merged in git-bug? Thanks.

Hey @aleb this is a pretty big change to review, it might take time for me to go through it.

As a preamble, I find that some of this code doesn't follow the architecture design ans "leak" through the layers. Two things comes to mind:

• Key: all the tricky handling of keys, encoding, parsing should be abstracted behind the public interface of Key instead of spreading up to the commands. I made a tentative refactoring with 1e760c3 . It's not done, not everything compile, but it should at least give you the idea.
• I'm yet to understand what this validation is, but the go-git thing should be in the repo package, Ideally behind the same Repo interface and in the same GitRepo implementation, even if that make a small Frankenstein here. The cache layer is really not meant for that. That would also be a first step towards migrating.

Speaking of this Validator thing, I'm a bit confused tbh, what is it for ? When is it supposed to run ? What should happen if the validation fail ? Why not stay with the Validate() functions everywhere and have a piece of code reload everything and validate through those ? Maybe Identity/Bug should also check the commit signature when loading the data ?

I'm not sure what verification logic would be common to git-bug and git-ticket

Unless I'm mistaken, both share a very similar need here, or at least we can find a common ground from where you can build your extra validations without breaking everything at each rebase. It seems to me that you want a different set of data/operations (Bug vs ticket) but the "framework" part could be largely shared.

The validation logic requires the newly created identity to have a key and be signed with that key.

git-bug will keep allowing identities without keys, but a flag somewhere to have a mode where they need to have at least one is entirely possible. Useful for both.

The very first commit in the identities history is very important. It is of course a commit which creates a new identity. The validation logic requires the newly created identity to have a key and be signed with that key.

Hmmm, when I originally thought about this the idea was, to add a key to an identity, add the private key in the version as well as a signature for the previous data. Here you are adding the signature in the commit itself, which is a bit different and tie git-bug more to git itself. Not sure what is the best way really. I need to think more about this.

I made a small change to Identity.Commit to increase the lamport time so each identity version has a different lamport time so they can be sorted.

Adding ordering to identities is fine, but that needs to be a distinct Lamport clock to not mess with the bugs. It looks like it's time to update the ClockedRepo to something simpler that support an arbitrary number of clocks. What about this:

// ClockedRepo is a Repo that also has Lamport clocks
type ClockedRepo interface {
	Repo

	GetClock(name string) Clock
}

type Clock interface {
	Time() lamport.Time
	Increment() (lamport.Time, error)
	Witness(time lamport.Time) error
}

And yes, it seems that LoadClocks and WriteClocks are never used outside of GitRepo itself so they should not be part of the interface. Cleanup time !

Hmmm, when I originally thought about this the idea was, to add a key to an identity, add the private key in the version as well as a signature for the previous data. Here you are adding the signature in the commit itself, which is a bit different and tie git-bug more to git itself. Not sure what is the best way really. I need to think more about this.

So I think this boils down to the question of whether or not rebase are needed. If the commit is signed by the author, rebasing and rearranging the commits is not possible anymore without invalidating the signature. If the data is signed, we are still free to do that.

In other words, if the commit is signed, the whole history is signed. If the data is signed, only this specific piece of data is signed, which only prove authorship (which is really nice already).

The problem is that at the moment, git-bug does make rebase when changes are pulled. Most of the time people use a unique central git repo so it could work because the commit that are rebased are your own, so you could sign them again. But as soon as you use more than one remote or push/pull directly this model fall apart.

What I was thinking about in #212 was to replace the rebase with merges to do the conflict resolution. This would make the data model entirely immutable and would combine very well with signing the commits.

So, short of doing this #212 (which would be awesome but a lot of work), the choice end up being either:

1. explicitly saying that fully decentralized workflow is not supported
2. working on the commit signing without fully enabling it yet and wait for the data model rework to complete
3. do data signing and change to commit signing when the data model change is completed

I think option 2 make the most sense here. What do you think ?

Sorry for the late reply.

As a preamble, I find that some of this code doesn't follow the architecture design ans "leak" through the layers. Two things comes to mind:

• Key: all the tricky handling of keys, encoding, parsing should be abstracted behind the public interface of Key instead of spreading up to the commands. I made a tentative refactoring with 1e760c3 . It's not done, not everything compile, but it should at least give you the idea.

I cherry-picked your commit and fix the issues.

• I'm yet to understand what this validation is, but the go-git thing should be in the repo package, Ideally behind the same Repo interface and in the same GitRepo implementation, even if that make a small Frankenstein here. The cache layer is really not meant for that. That would also be a first step towards migrating.

Sounds good. Done.

Speaking of this Validator thing, I'm a bit confused tbh, what is it for ? When is it supposed to run ? What should happen if the validation fail ? Why not stay with the Validate() functions everywhere and have a piece of code reload everything and validate through those ? Maybe Identity/Bug should also check the commit signature when loading the data ?

An identity can't fully validate itself if we want to allow others to make changes to an identity. A separate validator is needed to keep track of what keys have been added and removed and when. (It would have been simpler if only the keys added by the identity can sign the identity's changes, like you wanted(?), but we wanted something more flexible.)

The validate command verifies that the keys are in good order and that the identity changes are properly signed with the keys available at that lamport time. It will be used for example on the server-side git hook to prevent pushing bad data to the repo.

I'm not sure what verification logic would be common to git-bug and git-ticket

Unless I'm mistaken, both share a very similar need here, or at least we can find a common ground from where you can build your extra validations without breaking everything at each rebase. It seems to me that you want a different set of data/operations (Bug vs ticket) but the "framework" part could be largely shared.

The more that can be shared, the better.

git-bug will keep allowing identities without keys, but a flag somewhere to have a mode where they need to have at least one is entirely possible. Useful for both.

What would be a good place to store this flag in the repo?

The very first commit in the identities history is very important. It is of course a commit which creates a new identity. The validation logic requires the newly created identity to have a key and be signed with that key.

Hmmm, when I originally thought about this the idea was, to add a key to an identity, add the private key in the version as well as a signature for the previous data. Here you are adding the signature in the commit itself, which is a bit different and tie git-bug more to git itself. Not sure what is the best way really. I need to think more about this.

I don't completely follow. Why add the private key in the version?

I'm thinking the commits need to be signed anyway, so then we use those signatures. The whole point for us is to have every commit signed (with the "valid keys").

I made a small change to Identity.Commit to increase the lamport time so each identity version has a different lamport time so they can be sorted.

Adding ordering to identities is fine, but that needs to be a distinct Lamport clock to not mess with the bugs. It looks like it's time to update the ClockedRepo to something simpler that support an arbitrary number of clocks. What about this:

// ClockedRepo is a Repo that also has Lamport clocks
type ClockedRepo interface {
	Repo

	GetClock(name string) Clock
}

type Clock interface {
	Time() lamport.Time
	Increment() (lamport.Time, error)
	Witness(time lamport.Time) error
}

And yes, it seems that LoadClocks and WriteClocks are never used outside of GitRepo itself so they should not be part of the interface. Cleanup time !

Ok, let me see..

Hmmm, when I originally thought about this the idea was, to add a key to an identity, add the private key in the version as well as a signature for the previous data. Here you are adding the signature in the commit itself, which is a bit different and tie git-bug more to git itself. Not sure what is the best way really. I need to think more about this.

So I think this boils down to the question of whether or not rebase are needed. If the commit is signed by the author, rebasing and rearranging the commits is not possible anymore without invalidating the signature. If the data is signed, we are still free to do that.

In other words, if the commit is signed, the whole history is signed. If the data is signed, only this specific piece of data is signed, which only prove authorship (which is really nice already).

It would be good to use git's signatures system if possible, to keep things simple.

The problem is that at the moment, git-bug does make rebase when changes are pulled. Most of the time people use a unique central git repo so it could work because the commit that are rebased are your own, so you could sign them again. But as soon as you use more than one remote or push/pull directly this model fall apart.

We'll use a single remote, so it would work for us. Good to know about the local rebasing when changes are pulled.

What I was thinking about in #212 was to replace the rebase with merges to do the conflict resolution. This would make the data model entirely immutable and would combine very well with signing the commits.

So, short of doing this #212 (which would be awesome but a lot of work), the choice end up being either:

1. explicitly saying that fully decentralized workflow is not supported
2. working on the commit signing without fully enabling it yet and wait for the data model rework to complete
3. do data signing and change to commit signing when the data model change is completed

I think option 2 make the most sense here. What do you think ?

We are not interested in the fully decentralized workflow. I'm not sure about the data signing benefits, but it sounds complicated. Option 2 sounds great since we'll be able to use it already and when the data model is reworked we'll be compatible.

An identity can't fully validate itself if we want to allow others to make changes to an identity. A separate validator is needed to keep track of what keys have been added and removed and when. (It would have been simpler if only the keys added by the identity can sign the identity's changes, like you wanted(?), but we wanted something more flexible.)

The validate command verifies that the keys are in good order and that the identity changes are properly signed with the keys available at that lamport time. It will be used for example on the server-side git hook to prevent pushing bad data to the repo.

I still don't really get what you want to achieve, I going to guess you want to have some sort of centralized authority for identities. If so, why not have this centralized create the identities with 2 keys, one private to this authority, one given to the user. If you want to prevent the user to make changes to the identity, we could have two list of keys in an identity:

• keys that allow full control
• keys that only allow signing commits, bugs ... but not change the identity itself.

Or maybe we should go all in already and encode that with capabilities on each keys. That would be more extensible for the future.

That's a little change that could enable nice new usecases.

On a side note, maybe those keys should have an optional name ? That might help for complex schemes.

The more that can be shared, the better.

100%. It seems that we are aiming for a sort of framework to build decentralized app on top of git, where git-bug itself would just be an example. That's awesome.

git-bug will keep allowing identities without keys, but a flag somewhere to have a mode where they need to have at least one is entirely possible. Useful for both.

What would be a good place to store this flag in the repo?

For now, a hard-coded flag in the code would be good so we don't conflict each other. Later this could also be included in the project settings for optional enabling in git-bug.

I don't completely follow. Why add the private key in the version?

Not the private keys. I was thinking about adding the signature of the previous version with one of the valid keys but that's a flawed model. Your idea is better.

So to recap, the current idea is:

• have a chain of commit, each of them hold a version
• a version hold zero, one or many public keys
• if the first version has at least one public key, the associated commit must be signed with one of those keys
• after that, if the previous version has at least one public key, the commit of this new version must be signed with a key of the previous version. This new version doesn't have to keep the public key associated to the signature. This allow for key rotation an possibly removing entirely the keys (not sure that one would be a good thing, we can add a rule to disallow that).

Sounds good to you?

We are not interested in the fully decentralized workflow. I'm not sure about the data signing benefits, but it sounds complicated. Option 2 sounds great since we'll be able to use it already and when the data model is reworked we'll be compatible.

Option 2 it is!

FYI, I'm working on the architecture/framework changes that will make this and adding new entities (PR, config ...) easier. It also happen to make the creation of forked tool like git-ticket easier. Have a look at #411

Following closely will be a WIP PR for a go-git repository implementation, which would make signing way easier.

go-git repo PR: #412

@aleb FYI, something similar to your last changes already now exist in master: https://github.com/MichaelMure/git-bug/blob/master/repository/repo.go#L108

Yes, I noticed too late because I started a while ago..

I rebased and I think I don't have other changes planned at the moment.

Anything we should be aware of if merging this in git-ticket, so we remain compatible with upstream?

.. other than the failing test. :D

EDIT: done

Anything we should be aware of if merging this in git-ticket, so we remain compatible with upstream?

Yes.

I've been increasingly convinced that the good way to go to ultimately implement all those features (identities, signatures, ACL ...) is to:

• ditch the git CLI based Repo in favor of the go-git implementation (almost done in repository: go-git backed Repo #412): not only the CLI version is frustrating, but that will give an easier access to commit signatures and friends
• augment the repo with https://github.com/99designs/keyring to have access to the system credential storage, with a fallback on an encrypted file with password
• stop storing bridges credential in the global git config
• store identities private keys there instead of using GPG: if I'm not mistaken that means that GPG can be avoided entirely

In addition, more refactoring is need in the Identity, to store one time reference per entity (see https://github.com/MichaelMure/git-bug/blob/master/identity/version.go#L24-L27). This is so that it can be possible to say what version of an identity is valid for what element of an entity (Bug's operation, Ticket's operation, Config, PullRequest ...).

With all that we should have something that allow git-bug to grow with more entities, let git-ticket implement its own entities with minimal conflict and have an easier time implementing signatures for everyone.

Btw, thanks for spearheading this effort, it helped a lot to define what was necessary and what needed to change.

What do you think of this plan btw?

I've been increasingly convinced that the good way to go to ultimately implement all those features (identities, signatures, ACL ...) is to:

• ditch the git CLI based Repo in favor of the go-git implementation (almost done in repository: go-git backed Repo #412): not only the CLI version is frustrating, but that will give an easier access to commit signatures and friends

Great!

• augment the repo with https://github.com/99designs/keyring to have access to the system credential storage, with a fallback on an encrypted file with password
• store identities private keys there instead of using GPG: if I'm not mistaken that means that GPG can be avoided entirely

IIUC the system credential storage would be used to get the private keys for signing commits created by git-bug.

When creating normal commits in the repo, the git CLI will use GPG. Ideally the private keys from GPG can be used with 99designs/keyring.

• stop storing bridges credential in the global git config

Sounds good.

In addition, more refactoring is need in the Identity, to store one time reference per entity (see https://github.com/MichaelMure/git-bug/blob/master/identity/version.go#L24-L27). This is so that it can be possible to say what version of an identity is valid for what element of an entity (Bug's operation, Ticket's operation, Config, PullRequest ...).

In this case sorting all the Identity Versions with sort.Sort(ByLamportTime(v.versions)) won't be possible anymore, but we'd still be able to establish an order since we know exactly which Identity Version created the operation. If an Identity is disabled/canceled, then the central authority would have to prevent accepting new commits signed by it.

I think we could work with this.

IIUC the system credential storage would be used to get the private keys for signing commits created by git-bug.

When creating normal commits in the repo, the git CLI will use GPG. Ideally the private keys from GPG can be used with 99designs/keyring.

Yes. keyring can store arbitrary secrets, as long as they can be serialized into a []byte. go-git still use GPG but it's golang.org/x/crypto/openpgp which will be much easier to work with than CLI tool that may or may not be there.

In this case sorting all the Identity Versions with sort.Sort(ByLamportTime(v.versions)) won't be possible anymore, but we'd still be able to establish an order since we know exactly which Identity Version created the operation. If an Identity is disabled/canceled, then the central authority would have to prevent accepting new commits signed by it.

Not sure why you need that. Versions within an identity already have an implied logical order defined by the order of the commits in git. It would be possible to order identities by creation or edition by adding two extra Lamport clocks there but I don't see much the use of that.

In this case sorting all the Identity Versions with sort.Sort(ByLamportTime(v.versions)) won't be possible anymore, but we'd still be able to establish an order since we know exactly which Identity Version created the operation. If an Identity is disabled/canceled, then the central authority would have to prevent accepting new commits signed by it.

Not sure why you need that. Versions within an identity already have an implied logical order defined by the order of the commits in git. It would be possible to order identities by creation or edition by adding two extra Lamport clocks there but I don't see much the use of that.

We need to be able to sort the Versions across all Identities. This way it's possible to check each version has been created by somebody with a valid key at the time.

I think we'll try with a lamport clock used only for Versions.

Anything from here you can use upstream at this point?

Sorry it took me a while to digest all that and figure out what would be the right thing to do.

I think we'll try with a lamport clock used only for Versions.

Make sense to me. That would be similar to the edit Lamport time for bugs. Might as well throw another for the creation to mirror Bug.

Anything from here you can use upstream at this point?

Yes, but not as it is currently I'm afraid (sorry about that). There is a lot of good things though.

As you can see in #412, the go-git repo is pretty much ready to be merged (one missing thing is a migration for the bridge credentials in https://github.com/MichaelMure/git-bug-migration/, so nothing that concern you). Notably this repo will allow for a much more native and direct access to everything signature related. Hopefully this also allow to do less in git-bug and delegate to go-git and openpgp.

Additionally, something I'd like changed in this PR is how things are located. As of now the signature validation code is built around the core instead of integrating with it. That is, have signature validation available within bug and identities. I believe changing that would have multiple benefits:

• split the massive validate package into smaller and more understandable pieces: validating the whole repo would then just be iterating over all the entities and call the validation function
• allow to validate individual entities if the need arise
• critically, allow to validate the data while reading it and before merging remote updates

Does that make sense to you?

(also ideally this would be done in smaller digestible PRs that can be merged along the way to this goal)

@aleb FYI, #532 bring a major update to the data model that should interest you:

• allow to easily define new entities without caring about all the complex conflict resolution, reading/writing and a lot more difficult to write code (see how Bug is defined now: https://github.com/MichaelMure/git-bug/blob/master/bug/bug.go)
• support for commit signature

This should make your adventure with git-ticket much easier if you migrate as well.

As such, I'll close this PR.

Even if your code here has not been merged, it has been a very useful contribution and what is in master now is better because of it. Thank you.