Don't publish private entries #1233

struktured · 2019-04-08T22:20:52Z

Please check one of the following, relating to the CHANGELOG, which should be updated if relevant

my changes to the code affect some exposed aspect of the developer experience, and I have added an item to relevant 'Added, Fixed, Changed, Removed, Deprecated, or Security' heading under the 'Unreleased' heading of the CHANGELOG, with the format - summary of change [PR#1234](https://github.com/holochain/holochain-rust/pull/1234)
my changes to the code do not affect any exposed aspect of the developer experience

This introduces a fix so that entries defined as being private are not published to the DHT. Currently entry types do not examine there own definitions to ascertain their sharing attribute- this work enhances the publishable logic accordingly.

As this is my first project with the holochain core team, it is requested that reviewers are particularly thorough and stringent before accepting the PR.

Extend EntryType with CanPublish trait

Remove can_publish function from existing entry type definition in the core_types crate.
Introduce CanPublish trait in core crate which requires a Context to properly determine whether the entry type is sharable.
Have EntryType implement new CanPublish trait by extending the entry type functionality in the core crate (even though the EntryType itself is still defined core_types). Most of this commented out code by @lucksus can be probably reused for this purpose.

Rust tests

Add test module in core which creates a just complicated enough Context to verify private entries are not published to the DHT. (consider using / extended the existing holochain test library crate).

App-spec tests

functions

Make a private entry type called memo to the blog zome which represents private notes and reminders users only write to their local chain. Add public zome functions hash_memo, create_memo, and my_memos.
Test that the new entry type is creatable and queryable with aforementioned zome functions.
Ensure other users cannot find these entries in their own local chains or the DHT.
Test that the chain header still contains the entry address and relevant metadata but without the entry itself. N/A: Considered out of scope for this project

Replace occurences of no arg can_publish with trait version which also requires a context. Log unexpected states of the context, such as missing entry type definitions.

This reverts commit 80d529d.

thedavidmeister · 2019-04-11T07:38:50Z

mmm seems like something we want

zippy

The big fix needed is the location of the can_publish check needs to be when a get request is received.

zippy · 2019-04-11T13:37:55Z

app_spec/zomes/blog/code/src/memo.rs

+                },
+                EntryValidationData::Delete{old_entry:old_memo,old_entry_header:_,validation_data:_} =>
+                {
+                   (old_memo.content!="SYS")


This looks weird to me. Why is this here? The system type validation should be internal to holochain, and not happen at the app level I think.

It's there cause it's copied from the other zome code of blog zome I believe. So if we question it here we should question it there

See commit 8d04d68.

zippy · 2019-04-11T13:38:56Z

CHANGELOG.md

@@ -50,7 +50,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `add_agent()` admin function now creates keystore file instead of just recording file in config [#1182](https://github.com/holochain/holochain-rust/pull/1182)
 - One-time-signing now takes a vector of payloads, and returns a vector of signatures. [#1193](https://github.com/holochain/holochain-rust/pull/1193)
 - Pins nixpkgs to Holo-Host channel in shell and CI [#1162](https://github.com/holochain/holochain-rust/pull/1162)
- Uses autodump configuration for pickledb which dumps per change [#1231](https://github.com/holochain/holochain-rust/pull/1231)


Why is the pickledb stuff showing up here?

zippy · 2019-04-11T13:39:40Z

cas_implementations/src/cas/pickle.rs

@@ -10,9 +10,12 @@ use std::{
    fmt::{Debug, Error, Formatter},
    path::Path,
    sync::{Arc, RwLock},
+    time::Duration,


I'm not sure why pickle stuff is at all affected by this PR, looks like files got committed by mistake?

Not sure, either- possibly an awry rebase- will ensure it's clean.

zippy · 2019-04-11T13:41:59Z

core/src/entry/mod.rs

+            .expect("DNA must be present to test if entry is publishable.");
+
+        let entry_type_name = self.to_string();
+        context.log(format!("got entry type name of {}", entry_type_name));


It looks like this logging should probably be removed and was for debugging? or needs a "debug/" prefix

core/src/entry/mod.rs

core/src/workflows/get_entry_result.rs

zippy · 2019-04-11T13:59:35Z

core/src/workflows/get_entry_result.rs

-        ))
+        ));
+
+        match entry_result.clone() {


This is happening in the wrong place. What's going on here is that if you got the value across the network, you simply aren't returning it to the caller. But that means that you actually got a value that should never have been released to you. This check needs to happen by the receiver of the get request. i.e. in handle_fetch_entry and handle_fetch_entry_result so that it never leaves the source machine.

See commit 8dd9f9e.

@zippy you mentioned handle_fetch_entry and handle_fetch_entry_result- are both required to check can_publish? I only added it for handle_fetch_entry and it worked with my tests at least. Please confirm desired behavior for handle_fetch_entry_result.

zippy · 2019-04-11T14:00:21Z

doc/holochain_101/src/faq.md

@@ -65,7 +65,7 @@ DHTs enable key/value pair storage and retrieval across many machines. The only

 In fact, since many DHTs are used for illegal file sharing (Napster, Bittorrent, Sharezaa, etc.), they are designed to protect anonymity of uploaders so they won't get in trouble. File sharing DHTs frequently serve virus infected files, planted by uploaders trying to infect digital pirates. There's no accountability for actions or reliable way to ensure bad data doesn't spread.


This file also shouldn't show up as changed in this PR right?

…nt-publish-private-entries

Co-Authored-By: struktured <[email protected]>

…olochain-rust into dont-publish-private-entries

struktured · 2019-04-12T22:21:08Z

It should be removed- it's not really testing any useful functionality here- I will remove it.

…

On Fri, Apr 12, 2019 at 5:36 PM Connor Turland ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app_spec/zomes/blog/code/src/memo.rs <#1233 (comment)> : > + validation: |validation_data: hdk::EntryValidationData<Memo>| { + match validation_data + { + EntryValidationData::Create{entry:memo,validation_data:_} => + { + (memo.content.len() < 280) + .ok_or_else(|| String::from("Content too long")) + }, + EntryValidationData::Modify{new_entry:new_memo,old_entry:old_memo,old_entry_header:_,validation_data:_} => + { + (new_memo.content != old_memo.content) + .ok_or_else(|| String::from("Trying to modify with same data")) + }, + EntryValidationData::Delete{old_entry:old_memo,old_entry_header:_,validation_data:_} => + { + (old_memo.content!="SYS") It's there cause it's copied from the other zome code of blog zome I believe. So if we question it here we should question it there — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1233 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE01opSEHpy44uTCuC7PvhjKmS9u3RgDks5vgPxRgaJpZM4cjOTQ> .

…nt-publish-private-entries

zippy

Woot woot! Looks great. Nice work.

lucksus

👏 👏 👏

struktured and others added 15 commits April 10, 2019 16:54

implement CanPublish trait on entry

dfefdb6

Implement can publish trait on entry type in core

80fe9b5

Replace occurences of no arg can_publish with trait version which also requires a context. Log unexpected states of the context, such as missing entry type definitions.

remove can publish test, for now

269a23f

Introduction of private memo entry type.

c7b632c

Add memo address public zome function.

142cb80

hash_memo/ create_memo scenarios

af02490

of the thing of -> a thing

b3e7e87

test that memo entry is actually private

0e497ea

Set hash to correct value for private memo test.

11fce6c

Remove erroneously added file (from #1125)

5176774

Revert "Merge pull request #1231 from holochain/autodump-pickledb"

325e7ea

This reverts commit 80d529d.

test to ensure memo is not in public dht.

65270e8

Filter out sys entries before checking definitions.

78ab2fa

All tests pass.

bbfa5b1

Run hc-fmt

c73d9c7

struktured force-pushed the dont-publish-private-entries branch from d4a2c3e to c73d9c7 Compare April 10, 2019 21:00

struktured added 2 commits April 10, 2019 18:01

Revert back to previous run.sh

a9cc3a0

Remove resolved TODO in author_entry.

ff44017

zippy suggested changes Apr 11, 2019

View reviewed changes

struktured and others added 6 commits April 11, 2019 11:41

Merge branch 'develop' of github.com:holochain/holochain-rust into do…

ca2ca71

…nt-publish-private-entries

Update core/src/workflows/get_entry_result.rs

9e0f871

Co-Authored-By: struktured <[email protected]>

Update core/src/entry/mod.rs

59776fc

Co-Authored-By: struktured <[email protected]>

Update core/src/entry/mod.rs

9dad2bd

Co-Authored-By: struktured <[email protected]>

Merge branch 'dont-publish-private-entries' of github.com:holochain/h…

283e1b5

…olochain-rust into dont-publish-private-entries

Move can_publish logic to fetch_entry

8dd9f9e

struktured and others added 3 commits April 12, 2019 18:52

Remove excessive validation checks for memos.

8d04d68

Can_publish trait rust unit test.

1e6e4f4

Update CHANGELOG.md with PR #1233.

c04c396

Carmelo Piccione added 2 commits April 13, 2019 03:31

Merge branch 'develop' of github.com:holochain/holochain-rust into do…

bcd1b9b

…nt-publish-private-entries

Run hc-fmt.

fe33f49

struktured changed the title ~~WIP: Don't publish private entries~~ Don't publish private entries Apr 13, 2019

Merge branch 'develop' into dont-publish-private-entries

8c18b46

zippy approved these changes Apr 14, 2019

View reviewed changes

lucksus approved these changes Apr 15, 2019

View reviewed changes

lucksus merged commit 5548511 into develop Apr 15, 2019

zippy deleted the dont-publish-private-entries branch October 4, 2019 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't publish private entries #1233

Don't publish private entries #1233

struktured commented Apr 8, 2019 •

edited

Loading

thedavidmeister commented Apr 11, 2019

zippy left a comment

zippy Apr 11, 2019

Connoropolous Apr 12, 2019

struktured Apr 12, 2019

zippy Apr 11, 2019

zippy Apr 11, 2019

struktured Apr 11, 2019

zippy Apr 11, 2019

zippy Apr 11, 2019

struktured Apr 12, 2019

struktured Apr 13, 2019

zippy Apr 11, 2019

struktured commented Apr 12, 2019 via email

zippy left a comment

lucksus left a comment

		@@ -65,7 +65,7 @@ DHTs enable key/value pair storage and retrieval across many machines. The only

		In fact, since many DHTs are used for illegal file sharing (Napster, Bittorrent, Sharezaa, etc.), they are designed to protect anonymity of uploaders so they won't get in trouble. File sharing DHTs frequently serve virus infected files, planted by uploaders trying to infect digital pirates. There's no accountability for actions or reliable way to ensure bad data doesn't spread.

Don't publish private entries #1233

Don't publish private entries #1233

Conversation

struktured commented Apr 8, 2019 • edited Loading

Extend EntryType with CanPublish trait

Rust tests

App-spec tests

thedavidmeister commented Apr 11, 2019

zippy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

struktured commented Apr 12, 2019 via email

zippy left a comment

Choose a reason for hiding this comment

lucksus left a comment

Choose a reason for hiding this comment

struktured commented Apr 8, 2019 •

edited

Loading