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

New contract interface and EIPs should be required to include a privacy considerations section #5682

Closed
kdenhartog opened this issue Sep 19, 2022 · 89 comments
Labels
enhancement r-eips Relates to the formatting or another aspect of EIPs w-stale Waiting on activity

Comments

@kdenhartog
Copy link
Contributor

Proposed Change

This one was a bit obvious to me and it's only once I started reading through EIP-1 that I realized it's not required. Pretty simple to remediate. Let's just add a requirement that new EIPs being created and any EIPs entering the final call stage are required to include a Privacy considerations section. A reference to how to help developers define this section can be linked to here: https://datatracker.ietf.org/doc/rfc6973/

@Pandapip1
Copy link
Member

I think this should be an (optional?) subsection of the security considerations section. Privacy is a security consideration, and all non-obvious privacy considerations should be listed there.

There are plenty of EIPs where privacy considerations are not an issue. On the blockchain, everything is public by default, and everything should be assumed to be public except when commonly known to be private (e.g. private keys) or explicitly stated.

@kdenhartog
Copy link
Contributor Author

Privacy and security are often separate issues to be considered. This is why IETF cites the necessity for both of them. Even in the case where no known privacy or security issue is known a simple statement can be stated as such. For example, a document outlining how to define privacy considerations still defines a security considerations section even though it's defining how to write a document. https://www.rfc-editor.org/rfc/rfc6973.html#section-9

I don't think it's unreasonable to require an author to include these sections. Even if it's as simple as stating that nothing needs to be considered.

@Pandapip1
Copy link
Member

I don't think it's unreasonable to require an author to include these sections. Even if it's as simple as stating that nothing needs to be considered.

The vast majority of EIPs don't need this section. As previously mentioned, everything is public by default. Could you point to a specific EIP in which this section would be useful?

@xinbenlv
Copy link
Contributor

@kdenhartog thank you for the proposal, can you give us a bit of examples based on some EIP what their privacy section will look like?

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Sep 19, 2022

I tend to agree that core category EIPs don't seem to have any need for these considerations. I've not found one so far as I've briefly skimmed them and come up with some things that could be discussed in a few EIPs.

Starting from the beginning. Here's just a few things I'd be thinking about to enhance the privacy of these different EIPs if I was considering implementing them. There's plenty more and I can keep going if you think it would be useful to show how this would be useful.

EIP-20: Considerations around the usage of too large of a decimal value allowing too much precision which can be used to correlate transfers even when moving through a coin mixer (this is an issue with Z-Cash's shielded transactions). Considerations around linkability of transfers between users and well known addresses such as exchanges.

EIP-86 (and various other account abstraction works): considerations how various types of contract logic such as multi-signatures and delegation links two persistent identifiers (e.g. Ethereum accounts)

EIP-107: Fingerprinting concerns to identify the user as a web3 user.

EIP-137: linking of persistent identifiers. Fingerprinting concerns. Usage of a new persistent identifier. Basically all the did-core privacy considerations apply here too: https://www.w3.org/TR/did-core/#privacy-considerations

EIP-162: Same considerations apply as 137

EIP-173: correlating activity done as a contract owner with other on chain activity. E.g. should a personal be used when deploying the contract separate from an account used for interacting with other contracts?

EIP-181: probably lots of similar considerations as 137 and 162 here.

EIP-191: Displaying warnings to users about checking to make sure no PII is included in the transactions

EIP-600: Considerations around the ability for people with access to a public key being able to derive child path keys and link ownership of ETH accounts

EIP-601: Same as 600

EIP-627: Usage of this protocol for a P2P messaging scheme to link two communicating parties by a passive malicious actor.

EIP-634: Lol this one is funny because it's now encouraging storing PII on chain which has generally been regarded as a bad idea due to the immutability property

@xinbenlv
Copy link
Contributor

I see. Thank you for a very long list of examples. That's very helpful. I am weakly in favor of adding a privacy consideration section, and make it optional. I do think that the complexity of privacy consideration also lies in the different expectations of web3 vs web2...

@Pandapip1
Copy link
Member

Okay, I think that's a big enough list to justify it being its own 2nd level section. I still don't think it's large enough to warrant being a required section though.

@kdenhartog
Copy link
Contributor Author

I'd be in favor of requiring for Interface/ERC but optional for network and core. The purpose really is to force the authors to consider the adversarial aspects of privacy invasions and to spur the discussion. If there truly isn't any than a quick short sentence would be suffice I think.

@kdenhartog
Copy link
Contributor Author

For example EIP-190 I couldn't come up with anything for this one even though it's an ERC track. Would take all of an additional 30 seconds to state that though and it opens up the discussion for authors to start considering these aspects. I'd be concerned that if this is optional then I'm left fighting an up hill battle for each one where I want to advocate for privacy considerations. Also, I don't believe this should be a normative section. It's really just about calling out some privacy problems and tradeoffs that implementers should be thinking about here and trying to find ways to address.

@Pandapip1
Copy link
Member

I still don't think it should be a required section for ERC category EIPs, if only because the expectations of privacy on the blockchain are greatly lowered. For interface EIPs, though, I would be in favor of requiring it since most of them do introduce privacy risks when using web2.

@kdenhartog
Copy link
Contributor Author

Main reason I saw them as useful is to highlight how clients should be interacting with the ERCs. Assuming a user interacted with a single discrete contract the risk is quite minimal. However, this is an uncommon pattern in this space so making recommendations to wallet developers is that are interacting with these standard contracts will assist with how they go about implementing support for the contracts. Once the data is public I agree there's little that can be done. Including this section as a required option can open up a discussion that justifies a modification to an interface to reduce the amount of information that gets published.

For example of this see the recent privacy considerations section I've been working on with the author of account bound tokens in #5686

It significantly changes how users would interact with a contract to expand the definition beyond an interface to also include expected behavior such as including off chain URLs rather than the data directly on chain.

@SamWilsn
Copy link
Contributor

None of the active editors are lawyers, so I don't think we're qualified to review privacy concerns.


I don't think any of the listed EIPs have enough privacy concerns to justify more than a sentence in the Security Considerations section. EIPs that specifically deal with PII should certainly have a privacy subsection, but generally speaking EIPs deal with interfaces, algorithms, and pseudonymous accounts; not personal information.

@kdenhartog
Copy link
Contributor Author

kdenhartog commented Sep 21, 2022

None of the active editors are lawyers, so I don't think we're qualified to review privacy concerns

Privacy concerns don't have to have a legal element to them. In fact, that's one of the things being discussed in #5686 is that GDPR isn't a good basis for why a technical consideration is considered.

This is how W3C handles privacy considerations sections and horizontal reviews of them:

https://www.w3.org/Guide/documentreview/#how_to_get_horizontal_review

@Pandapip1
Copy link
Member

https://www.w3.org/Guide/documentreview/#how_to_get_horizontal_review

Side note - the EIP process really needs a horizontal review process like that.

@github-actions
Copy link

There has been no activity on this issue for 1 week. It will be closed after 3 months of inactivity.

@github-actions github-actions bot added the w-stale Waiting on activity label Nov 30, 2022
@Pandapip1
Copy link
Member

This is definitely an important issue and should be kept open.

@github-actions github-actions bot removed the w-stale Waiting on activity label Dec 4, 2022
@kdenhartog
Copy link
Contributor Author

I think the general consensus for this has landed on keep it optional and I'm good with that (Personally, I think requiring it would be more beneficial, but I'm willing to compromise). The part that's not clear is whether this should added as a new top level section or if this should be made a subsection of the security considerations section. It seems that making it a subsection of security implies that privacy is a subset of security considerations when often times they are considered separate. For example, correlation of an EoA account across different dApps isn't a security issue because it's a public key that has no effect on the outcome of the security model. However, it does allow for the user to be tracked across dApps which does impact privacy still. Therefore, it makes sense that if we want this section to be added to the template it should be considered a separate top level section that is optional to include.

@Pandapip1
Copy link
Member

I personally would prefer a 2nd-level "Considerations" with 3rd-level "Security", "Privacy", and "Other".

@xinbenlv
Copy link
Contributor

xinbenlv commented Dec 6, 2022

+1

Agree with @kdenhartog that we want security and privacy considerations in the same level.

Agree with @Pandapip1 that it's better to have a top level "Considerations" with security and privacy as second level section.

@kdenhartog
Copy link
Contributor Author

I'm +1 to that, that seems like something that could achieve consensus and then lead naturally to a path overtime as expertise is built up. I'm willing to update the template and Walidator bot if the other editors are in alignment with this as well.

My idea for how this might play out is that for we could make all these sections optional. Then overtime if we build up proper expertise for security/privacy (accessibility is another horizontal review process that could be useful later on) "review boards" that can help review and provide feedback to these sections. At that point it may make sense to turn these sections from optional to required, but until we have sufficient number of people with expertise willing to help I think this is the best middle ground for now.

@Pandapip1
Copy link
Member

@axic @gcolvin @lightclient @SamWilsn do we have consensus?

@github-actions
Copy link

There has been no activity on this issue for 1 week. It will be closed after 3 months of inactivity.

@github-actions github-actions bot added the w-stale Waiting on activity label Dec 16, 2022
@Pandapip1
Copy link
Member

Waiting on consensus

@github-actions github-actions bot removed the w-stale Waiting on activity label Dec 17, 2022
@SamWilsn
Copy link
Contributor

@gcolvin suggested renaming the section to Security and Privacy. That works for me.

@github-actions
Copy link

github-actions bot commented Jun 1, 2023

There has been no activity on this issue for 1 week. It will be closed after 3 months of inactivity.

@github-actions github-actions bot added the w-stale Waiting on activity label Jun 1, 2023
@kdenhartog
Copy link
Contributor Author

making this unstale. Next steps is still on me to update the template and EIP-1

@github-actions github-actions bot removed the w-stale Waiting on activity label Jun 2, 2023
@github-actions
Copy link

github-actions bot commented Jun 9, 2023

There has been no activity on this issue for 1 week. It will be closed after 3 months of inactivity.

@github-actions github-actions bot added the w-stale Waiting on activity label Jun 9, 2023
@chaals
Copy link
Contributor

chaals commented Jun 9, 2023

tralala

@github-actions github-actions bot removed the w-stale Waiting on activity label Jun 10, 2023
@SamWilsn
Copy link
Contributor

Looks like more editors are objecting to this than just me. For the time being, the section will remain "Security Considerations", with the recommendation that privacy concerns go in that section.

@Pandapip1
Copy link
Member

I, for one, do not object to the renaming.

@kdenhartog
Copy link
Contributor Author

Yeah I saw on the notes that @lightclient was the one who had some reservations on this. I've not gotten a chance to listen through the call yet, but if they could comment here with their concerns that would be awesome so we can consider what the best next steps are to progress this issue.

@github-actions
Copy link

There has been no activity on this issue for 1 week. It will be closed after 3 months of inactivity.

@github-actions github-actions bot added the w-stale Waiting on activity label Jun 28, 2023
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 17, 2023
@kdenhartog
Copy link
Contributor Author

I'm awaiting what happens with core devs since I think they're less inclined to want the additional burden from this right now. With the ERC devs and the wallet devs I think this will be more pertinent though.

@chaals
Copy link
Contributor

chaals commented Aug 17, 2023

... core devs since I think they're less inclined to want the additional burden from this ...

I understand they might not want to make their work harder (who does?). But it's a bit disappointing, given the central role they play in the ecosystem, to learn the core devs don't take this issue seriously enough that they already do it as a matter of course :(

@Pandapip1
Copy link
Member

Will reopen this. A consensus has definitely not been reached.

@Pandapip1 Pandapip1 reopened this Aug 18, 2023
@github-actions github-actions bot removed the w-stale Waiting on activity label Aug 19, 2023
@github-actions
Copy link

There has been no activity on this issue for 1 week. It will be closed after 3 months of inactivity.

@github-actions github-actions bot added the w-stale Waiting on activity label Aug 26, 2023
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2023
@chaals
Copy link
Contributor

chaals commented Oct 14, 2023

@Pandapip1 thanks for reopening before.

This feels like we're relying on "consensus" by attrition. Which is a pretty broken way to run anything, since the thing it mostly shows is that caring about getting something right isn't worth the effort.

I hope, and trust, that isn't actually the case for Eth.

@kdenhartog
Copy link
Contributor Author

My understanding is that after the split this is likely going to be something that has to be pushed via each WG. So I think there's little contention that this is necessary for the ERC and Wallet devs. Unfortunately, I think there's been a bit of push back on this for core devs. I'd still like to see core devs support this as many of their EIPs could still be improved with privacy considerations as pointed out in #5682 (comment), but we'll likely need to engage directly with them on this now instead of pushing it via EIPIP if I understand it correctly.

@Pandapip1
Copy link
Member

Yup. This is again why I'm trying to push for a meta-WG, but I feel like that's getting less and less likely.

@kdenhartog
Copy link
Contributor Author

I'd agree that would be useful, but my understanding is Core wants to wait to do that for now. That makes sense to wait until Wallet and ERCs have active WGs otherwise it just becomes ErC/wallet/editor people telling core how to write their EIPs. I can understand why that doesn't align with their goals at the moment but I think in the future it likely could based on a few discussions I've had with some of the core devs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement r-eips Relates to the formatting or another aspect of EIPs w-stale Waiting on activity
Projects
None yet
Development

No branches or pull requests

11 participants
@xinbenlv @chaals @andrey-savov @kdenhartog @Pandapip1 @SamWilsn and others