-
Notifications
You must be signed in to change notification settings - Fork 37
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
Call for Input: Add Security Consideration to ERC-1271 #291
Comments
Would a new ERC of type "Informational" be adding new sec and informational sections? Here is a good example of where a frontmatter key for Errata which is allowed to contain links to newer docs could point to the new thing without changing the immutable original :D |
Why is there a strict emphasis on immutability of the non-normative parts of the spec? I would argue that the Security Considerations section in particular should be allowed to evolve. Very often we only learn what can go wrong after specs are Final and deployed to production. |
There are a couple reasons. First and foremost, we don't really define any sections as normative or non-normative officially. The whole EIP goes to final, so the whole EIP is immutable. We do have two specific exceptions to the immutability guarantee: errata and non-normative clarifications. This change doesn't qualify as an errata, because the EIP as written was not incorrect and this wasn't a mistake. This is wholly new information. This also isn't a non-normative clarification. It isn't explaining something that was ambiguous in the original text, again, this is wholly new information.
We absolutely need somewhere to put security considerations that appear after finalization. ethereum/EIPs#7915 is probably the first of this kind. The problem, as I see it, with doing security disclosures though the EIP process is that we must choose one of these policies:
Option 1 damages our credible neutrality. We could publish or withhold a change based on personal gain, instead of based on the facts. Out of options 2 and 3, I'd prefer 3. Because rules-as-written this is not an errata or clarification, and because I don't want EIP Editors embroiled in "is it" / "is it not" debates, I am going to have to oppose changing ERC-1271. |
but there's another option, IMHO:
I know i'm simplifying a bit, but this seems like a good example of how something like 7915 combined with a new (optional) frontmatter variable could work. Would it help to open a PR on a new frontmatter field called |
This is very reasonable. It is also important to allow for "security disclosures" not only from the authors of the EIP. Anyone should be allowed to disclose a vulnerability in any EIP at any point of time, not only before it's finalized. Because you know... a vulnerability can be found anytime. For me its self-evident. |
Right, I didn't mean to imply an informational addendum to a final EIP/ERC is any less valuable if it comes from someone else. Just that authors of a final EIP/ERC should be able to link to later addenda (or, IMHO, even new, superceding normative EIP/ERCs) from the final EIP/ERC, since all they are changing is frontmatter and not "content" (assuming we add a new optional frontmatter property) |
It is clear that "security disclosures in EIPs" aspect require an improvement. I'm proposing a modification of EIP process that would allow for better treatments of security issues and I would encourage everyone to participate https://ethereum-magicians.org/t/modification-of-eip-process-to-account-for-security-treatments/16265 If this would get sorted out we would not have to deal with all the discussions related to modifying EIPs for security reasons. I would be in favor of:
|
We can create a security guideline as I've proposed above. In this case we only need to agree on what "security vulnerability" looks like only once and after that EIP editors can simply benchmark proposals against the described rule set (obviously we need to develop the rules with explicit explanations of how they apply and how a person can understand if some proposal satisfies the rule or not). It does require some degree of technical expertise but I think its a reasonable requirement for an EIP editor. Anyways if there is a vulnerability disclosure then someone is championing it - and therefore the burden of proving that some EIP violates some rule is placed on the champion of the disclosure. |
I don't have a problem with pointers back from the new EIP to the old one, (like It would not be a permissioned field though. Anyone could claim that their EIP supersedes 1271. |
I'm against (but don't have a vote) for the reason that final is final. We need a Wiki for each Final EIP/ERC to include information such as this. |
This is fair. For the record, I think the Specification section should be considered the only normative part of an ERC. I understand the wider concerns, but I would like the conversation to focus on the fact that the published reference implementation has a concrete vulnerability. The inability to correct this is a serious problem. One option would be to retract the reference implementation, without making any other changes to the document. Concretely, we would remove the reference implementation and put in its place a small note that we feel comfortable doesn't change the nature of the ERC. How do people feel about that?
I completely agree with this. An immutable Security Considerations section will not reflect our latest understanding of the security issues around an ERC and is bound to be incomplete and unreliable. I would go as far as saying that the entire section should be replaced with a live document. |
Please comment on the modiifed proposal to only retract the reference implementation:
|
|
If the publication pipeline added a little banner to the top of, e.g., ERC-1271 that announced (in blinking red letters) that a more recent, accepted EIP claims to
This alters the semantics a tiny bit-- if anyone can claim to Similarly, if the older doc was a normative EIP or ERC, and the later one is just a BCP or other informational one, is |
There is no point in creating a superceding ERC to ERC-1271... Developers are already familiar with ERC-1271 as an identifier and as a concept. There is no problem with the interface that it specifies, only a problem in the reference implementation. We need to be able to address this without creating a whole separate ERC. I was not able to make it to the last meeting, but based on the comments here no one seems to have directly addressed my modified request to retract the reference implementation of ERC-1271. Please say how this violates the spirit of EIP immutability, if it does. I don't think it does. Depending on how strict we want to be about immutability we can choose from a range of options:
My preference is for the 3rd option but I do not care at this point which one of these we choose. I believe conflating this with the ERC-20 discussion is a mistake. This is a completely different situation that should be a lot simpler to resolve. Similarly I don't think this needs to be blocked by the lack of a solution to the broader question of ERC security advisories. |
I am actually the least committed to immutability of EIPs of all the opinion-sharors in this thread, so it's really not me that you have to convince to snip a little text. I was offereing compromise solutions because I think the editors (a group that does not include me, and that decides all of this) is fairly committed to the immutability of final docs. My proposal was to use mutable frontmatter or, failing that, even the publication pipeline's parsing of all later EIPs/ERCs with backrefs to 1271 when rendering 1271, to direct the reader's attention to pertinent later documents, whether long or short. I generally think the BCP workflow from IETF is a great precedent to follow (thanks @xinbenlv for the idea), provided we have some way of making clear to people when they look up the older, immutable doc where "updates" like these live (e.g., with an unmissable bright-colored banner across the top linking to any future documents by any author claiming to update this one). |
No opinion at this time |
With myself (and @lightclient weakly) opposed to this change, and no Editors in support, I'm going to have to close this without making the change. It's probably small consolation that I still believe we need a process for these kind of notices. |
Call for Input
Do we add REDACTED to the security considerations and reference implementation of ERC-1271?
The security considerations section and reference implementation are updated.
Background
@frangio, one of the original authors of ERC-1271, wants to add a new security consideration to the Final proposal.
The text was updated successfully, but these errors were encountered: