Skip to content
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

Send messages to multiple recipients, optionally introducing some of them to all the others #144

Closed
wants to merge 35 commits into from

Conversation

burdges
Copy link
Contributor

@burdges burdges commented Nov 27, 2014

This is a preliminary pull request. I'm happy to implement any suggestions.

Ram, Alan, and I originally added the ability for contacts to introduce their contacts to one another by sending each a message containing the other's name and public identity, and proposed PANDA shared secret. This basic functionality is visible in 9d341e1 and avoids modifying any .proto files.

There is a risk that someone could exploit this introduce function to MITM two users whom they introduce to one another simply by giving them different shared secrets. I therefore added three new records introducedBy, verifiedBy, and introducedTo that record your local social graph as visible from messages containing introductions, which should aid users in exposing any MITM attacks. These tools are functioning by e7d77e5 but note they require changing client.proto in 1c64ebb and rebuilding client.pb.go in a4e91a0. The last commit adds a checkbox to disable keeping this social graph information for users who consider keeping it a security risk.

There were several rounds of attempts at a GUI rebased into 9d341e1, some of which are visible in https://github.com/burdges/pond/commits/sendmany I ultimately decided the best approach to introductions was to just do multiple recipient messages, so now a682b07 mostly blows away our earlier work in 9d341e1.

Sending messages to multiple recipients required adding multiple recipients to the Draft struct, so the Draft.to uint64 has been replaced by Draft.toNormal and Draft.toIntroduce []uint64s, which also required adding them to client.proto in b377135 and rebuilding it in 5016a0a.

All commits should build and run without crashes except for 1c64ebb and b377135 which require their subsequent commits that rebuild client.pb.go. In particular the mammoth commit 6026acc that replaces Draft.to with Draft.toNormal and Draft.toIntroduce postponed modifications to disk/client.proto so that it can be built and tested without the state file changes.

Our short-term TODO list :

  • Fix usageString() so that it displays a more correct size over estimation.
  • Repair any tests I broke with my GUI changes in composeUI.
  • Write our own tests for the multiple recipient functionality and introductions.
  • Add sending to multiple recipients to the CLI and make introductions in the CLI use it.
  • Expose GTK's information dialog boxes in client/gtk.go
  • Make dialog boxes that explain anything that needs explaining, like the risks of keeping the social graph data, what to do if an introduction creates a second contact with a similar name, etc.
  • Allow CLI users to disable collecting social graph information like e56da9e for the GUI

We'd appreciate feedback on these questions :

  • How should introduction messages be encoded? I'm overly fond of our current format, which is a URI of the form pond-introduce-panda://NAME/SECRET/IDENTITY/ Instead, we could either build introduction messages into the pond.Message proto format, or else choose a more informative URI, like maybe pond-introduce:NAME?pandaSecret=SECRET?identity=IDENTITY
  • I've had a request to allow renaming contacts when doing introductions so that someone can be introduced using a different name than the one used locally.
  • Is an unint64 sufficient information for the social graph data introduceBy, verifiedBy, and introducedTo? I decided yes by considering the various additional data one might record : dates feel relatively unhelpful for uncovering MITM attacks but excessively helpful to authorities who seize a pond account. Aliases users were introduced by are useful for spreading information about discovered MITM attacks, well think about the user name "Fake Alice maybe owned by Eve", but recording who distributed that name seems bad, so we'd just add a separate Contact.aliases []string field instead.
  • Should verifiedBy really be called verified? Currently if Eve introduces a fake Alice to Bob and Carol then Bob and Carol see the fake Eve as being verified by each other. Is another word better?

Our longer-term TODO list :

  • A new outbox message is created for every recipient in this patch. Ideally, we should replace queuedMessage.to with toNormal and toIntroduce fields as well, but this requires allowing a message to live in both client.queue and client.outbox simultaneously. We can make this happen by using the same queuedMessage.id field and leaving the queuedMessage.message pointer nil in one of them, but it's another huge commit like 6026acc, so I want some feedback on this before proceeding.
  • A reply all function would begin to make sense once we've done all that, but again we should settle on how to encode introductions before doing it. Reply all require more thought about to whom we send more introductions.

…eful for various planned features, including contat invitations, messages with multiple recipants, and editable notes fields for contacts.

In fact, I considered moving inputTextBlock to cli-inout.go, but it wouldn't save many imports in cli.go to do so.
Rebased so newKeyExchange now outside beginPandaKeyExchange
…se the pond-introduce-panda urls, including the mixed fancy version.

Rebased so newKeyExchange now outside beginPandaKeyExchange
…protobuf.

Avoids screwing around with directories or editing files now.

Find gogoprotobuf at https://code.google.com/p/gogoprotobuf/
…because we save their proposed identity there.
Rebased with newKeyExchange now outside beginPandaKeyExchange
Joint effort between Jeff Burdges <[email protected]>, Alan Fay <[email protected]>, and Ramnarayan Vedam <[email protected]>
Simplifies creating messages to specific recipients and makes the code more readable.
We replace the to uint64 field in the Draft struct in client.go with two fields of
type []uint64 called toNormal and toIntroduce.  We clean up these fields when a
contact is deleted in client.go as well.  An introduction's size is approximated
in usageString() as well.

We modify sendDraft in netwrok.go to send the messages to every user id listed in
either toNormal and toIntroduce.  All users listed in toIntroduce are introduced
to all users listed in either toNormal or toIntroduce in the sent messages.

We make extensive modifications to composeUI in gui.go to enable composing drafts
to multiple recipients, optionally introducing some by listing them in toIntroduce
instead of toNormal.  We display multiple names on the ride hand sidebar whenever
possible too.

We modify cli.go to correctly report information about drafts with multiple
recipients.  We have however deffered adding the ability to edit drafts tor
multiple recipients until afer the functionality has been debugged more thuroughly.

We leave the introduction functionality in both gui.go and cli.go in tact, but
eventually this should be phased into the compose interfaces, which includes
editing drafts with no recipients in cli.go.

We modify disk.go to support the new Draft struct without changing the existing
protobuf, meaning drafts currently lose information about their multiple recipientsr
when saved.  We address this in a subsequent patch, but seperating any changes
to protobufs seems desirable.
Adds an information button to explain how introductions work to composeUI too.
@burdges
Copy link
Contributor Author

burdges commented Nov 28, 2014

@agl : Around one quarter of the line changes here come from rebuilding disk/clinet.pb.go, and that happens twice, so maybe reviewing this will be easier if you do a rebase edit on a4e91a0 and 5016a0a to rebuild disk/client.go with your own protoc configuration.

@ioerror
Copy link
Collaborator

ioerror commented Nov 28, 2014

As we discussed in Berlin, I like the idea of a uri that looks like the following:

panda://server-address?transport=HTTPS?pandaSecret=SECRET?identity=IDENTITY?name=NAME?Protocol=Pond

"I've had a request to allow renaming contacts when doing introductions so that someone can be introduced using a different name than the one used locally. "

I'm a fan of allowing someone to set the name for introduction, yes. My local petname is not your local pet name for someone, I bet.

I hope that in the future, we'll use Panda for many different protocols. I expect a bitcoin like serverless Panda exchange can be easily passed around like that too.

@burdges
Copy link
Contributor Author

burdges commented Nov 28, 2014

@ioerror Isn't our desire to use panda for more protocols precisely the reason not to use a panda:// named URI? If we go with a URI formulation then we presumably envision URI's coming form outside applications, like an IM that contains a URI that starts a PANDA key exchange for Pond, or a Pond message that contains a URI that starts a PANDA exchange in some Tahoe-LAFS front end. Applications must register URI handlers that say they want a particular URI type, so pond:// or pond-introduce:// could reasonably land in Pond's lap, while a panda:// URI would get fought over by all the different applications that use PANDA. Am I missing something here?

All I can imagine is : If Pond receives an introduction URI from another application, then Pond must record what it thinks about that application's security in the contact record, as this patch does by storing the introducing user in introduceBy. Are you arguing that using a panda:// URI would mark that URI as needing additional security information, so like an email client passing a panda:// URI to the system URI handler would add notes, like &security=email_gpg &security=email_plaintext, etc. If that were the case, then maybe the right solution is a pond-introduce-panda URI so that the system sends it to the right application and the panda: tells the sending application that it needs to make security annotations.

As an aside, there is conceivably a second type of pond introduction that avoids PANDA all together : Alice introduces Bob and Carol by giving each the other's pet name, the other's identity key, the private group signature she currently uses for the other, and some initial shared secret information, i.e. Alice kludges a kind of manual key exchange between Bob and Carol. I decided Alice exposing her private key group signature to anyone was less secure than using PANDA, not to mention more complicated. It could always be made to work though by issuing more than one private group signature key to every user or something if there were some fundamental reason not to just use PANDA. I originally called the URI pond-introduce-panda:// but removed the panda when I decided this kludging a manual key exchange would probably never be needed.

@burdges
Copy link
Contributor Author

burdges commented Nov 28, 2014

@ioerror I used insensitive entry fields for the contact names in composeUI, mostly because they looked nicer, but also because I knew we'd let users change the pet names eventually. We should however make them editable in introduceUI as well, which requires a bit more GTK mess.

At present, you can change the pet name by editing the outgoing introduce message before it gets sent : selected the message, press Abort Send, edit the name field, and press Send. We'd lose this manual option whenever we integrate multiple recipients better with the outbox though, but Reply All requires propagating pet names correctly regardless (see the longer-term TODO list).

Previously the little i that launched introducceUI from composeUI
disapeared if you deleted the line it was on.
There was an inconsistency in what Draft.inReplyTo meant between the GUI
and CLI, a minor bug for people who used both the CLI and GUI, which led
me to break testReplyACKs when adding more homogenious code.
We never create the to-box-add combobox if no valid contacts exist now,
while our old to combobox was created empty.
@burdges
Copy link
Contributor Author

burdges commented Dec 14, 2014

Just fixed all the tests I broke. yey! I caught two problems with pond in the process :

  1. burdges@b5b959d We'd never noticed that go test wasn't working correctly on any platform except Linux with TPM. Fixed now. Ad-hoc overloading is evil m'kay. http://vimeo.com/14317442#t=26m30s
  2. burdges@a7b45c4 Fixed an inconsistency in how Draft.inReplyTo is used between the GUI and CLI.

Appears github displays these recent commits in chronological rather than my rebased git ordering, btw.

We removed the rough estimation in Draft.usageString(), replacing it
with client.usageString(Draft).  Ideally, we should avoid marshalling
the entire message including attachments with every keypress, but hey.
Rough test for introductions and necessarily multirecipient message,
maybe should test more, like message contents.
@burdges
Copy link
Contributor Author

burdges commented Dec 17, 2014

I've largely settled my views on the questions I requested feedback on :

  • I'll rename Contact.verifiedBy to Contact.corroboratedBy unless anyone voices an objection, which incidentally requires rebuilding disk/client.proto again. Any thoughts?
  • We should actually communicate introduction messages into the pond.Message struct defined in disk/client.proto, but..
  • We should also support greeting a URI of the form pond-introduce:NAME?pandaSecret=SECRET?identity=IDENTITY, just in case anyone wants to send a greeting manually, via another program, etc.

@burdges
Copy link
Contributor Author

burdges commented Dec 19, 2014

At present, users who wish to clear the social graph records but not prohibit collecting them should click and then unclick the checkbox created in burdges@e56da9e We could make that more intuitive.

We fix a bug where fixProposedContactName didn't actually rename the
ProposedContact too.

Ideally we should use a fuzzy test for name similarity, maybe based on n-grams
or maybe a fast fuzzy spelling suggestion algorithm :
	https://github.com/sajari/fuzzy
or even JaroWinkler or Levenshtein from "github.com/antzucaro/matchr".
We should not call fixProposedContactName on existing contacts, but
doing so actually pointed out that I'd been using a random character
where I wanted a random lettter.
A local id prevented deleteSocialGraphRecords from deleting anything
Repalce the pond-introduce-panda://NAME/SECRET/IDENTITY/ URI format with the
opaque URI format pond-introduce:NAME?pandaSecret=SECRET&identity=IDENTITY
@burdges
Copy link
Contributor Author

burdges commented Jan 23, 2015

Pull request superseded by #161 which incorporates changes suggested by @agl

@burdges burdges closed this Jan 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants