-
Notifications
You must be signed in to change notification settings - Fork 110
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
Fingerprint of public keys #175
Open
burdges
wants to merge
10
commits into
agl:master
Choose a base branch
from
burdges:fingerprint
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…protobuf. Avoids screwing around with directories or editing files now. Find gogoprotobuf at https://code.google.com/p/gogoprotobuf/
Run "go generate" in ./client, ./proto, ./panda, or ./server to recompile the associated .proto files using an appropraite protoc command. We use gogoprotobuf from https://code.google.com/p/gogoprotobuf/ because Francesc @campoy indicated that even Google uses it to goprotobuf. We previously needed a perl script to fix a path mess created by goprotobuf 29d5f5d ala protoc --proto_path=$GOPATH/src:. --go_out=. disk/client.proto perl -p -i~ -e 's/(import protos \"github.com\/agl\/pond\/protos)\/pond.pb\"/$1\"/' disk/client.pb.go
We nolonger need these scripts since go generate with gogoprotobuf is cleaner.
I recommend that @agl replace this commit by installing gogoprotobuf and running go generate in .clinet, ./protos, ./panda, ./server himself rather than reviewing it.
We stop V2 ratchets from deriving theirIdentityPublic from theirPub because that makes theirPub sensitive information. Instead we add pond.KeyExchange.SupportedVersion to initalize the contact.supportedVersion variable correctly. At 31c3 @agl commented that theirIdentityPublic should be tied to theirPub, which initially I did not understand this comment. I thus wrote commit 591c7b78f1bdd0858bbf06373bc173b5c1356aa5 (rebased) to add a signature of theirIdentityPublic by theirPub if using V2. I've rebased all this away now after realizing that newRatchet should be decoupled form the identity keys.
Added a fingerprint() method to both the client and Contact structs. Display fingerprints along with User and Contact information. Fingerprints should be safe to display publicly on sites like github or twitter. I donno if we should seed the fingerprinting though. We avoid using the identity key in the fingerprint because it's known by the server.
We modify newRatchet to use Curve25519 keys derived from theirPub exactly the same way that @agl derived theirIdentityPublic previously. As the real identity keys are nolonger derived in this way, we thus completely decouple the identity key the server knows from the ratchet. We could do this more efficently, but imho extra25519 is confusing enough adding readability warrants wasting some time and memory.
I believe 31e3538 to be what @agl meant to achieve when he originally coupled the identity key with the public key, but obviously it needs some consideration. In particular, if there was a reason to move the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We compute and display a fingerprint in User and Contact information dialogs for both the GUI and CLI.
Fingerprints are meant to be safe to display publicly on sites like twitter. At present, they're computed using a sha256, but perhaps we should incorporate additional data or a seed.
I avoided using the identity key in the fingerprint because it's known by the server. And conceivable we could remove it from the Contact struct all together.
Pull based on #174 of course, so please review that one first.