Call for Input: Add Security Consideration to ERC-1271

[SamWilsn]

Call for Input

Decision	Do we add REDACTED to the security considerations and reference implementation of ERC-1271?
If Affirmed	The security considerations section and reference implementation are updated.
If Rejected	ERC-1271 is not changed.
Method	Rough Consensus
Deadline	November 30, 2023

Background

@frangio, one of the original authors of ERC-1271, wants to add a new security consideration to the Final proposal.

[bumblefudge]

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

[frangio]

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.

[SamWilsn]

Why is there a strict emphasis on immutability of the non-normative parts of the spec?

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.

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.

We absolutely need somewhere to put security considerations that appear after finalization. https://github.com/ethereum/EIPs/pull/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:

1. EIP Editors decide what is/isn't a factual vulnerability. Importantly, this requires EIP Editors become technical experts.
2. We publish every vulnerability, factual or not.
3. We publish no vulnerabilities.

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.

[bumblefudge]

but there's another option, IMHO:

4. [One of the] Author(s) of original EIP/ERC submits new informational EIP/ERC and adds a pointer to it from the frontmatter of their original EIP/ERC in the same PR, without otherwise changing one byte of final EIP/ERC. No editorial judgment on validity is implied or required.

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 addendum or bis? This would basically be equivalent to W3C errata publications (which do go through editorial review but not normative/technical review) and IETF "bis" (which is like an addendum, not limited to errata, that similarly goes through only-editorial review after a final draft).

[Dexaran]

@frangio

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.

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.

[bumblefudge]

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)

[Dexaran]

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:

1. Making "Security Considerations" section of EIPs mutable.
2. Creating a set of security rules in a form of a new "Security Guideline" EIP. Every new EIP must be benchmarked against the security guideline. If some of the older EIPs violate security guideline rules then it can be added to the security considerations section when someone requests it.
3. Making a change to EIP-1 to require "new EIPs must satisfy the rule set described in security guideline".

[Dexaran]

@SamWilsn

The problem, as I see it, with doing security disclosures though the EIP process is that we must choose one of these policies:

1. EIP Editors decide what is/isn't a factual vulnerability. Importantly, this requires EIP Editors become technical experts.
2. We publish every vulnerability, factual or not.
3. We publish no vulnerabilities.

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.

[SamWilsn]

@bumblefudge

4. [One of the] Author(s) of original EIP/ERC submits new informational EIP/ERC and adds a pointer to it from the frontmatter of their original EIP/ERC in the same PR, without otherwise changing one byte of final EIP/ERC. No editorial judgment on validity is implied or required.

I don't have a problem with pointers back from the new EIP to the old one, (like requires.) A supersedes preamble field is a very popular request. In that framework, you could release a new copy of ERC-1271 with the updated information that supersedes the old one.

It would not be a permissioned field though. Anyone could claim that their EIP supersedes 1271.

[abcoathup]

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.

[frangio]

we don't really define any sections as normative or non-normative officially

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?

We need a Wiki for each Final EIP/ERC to include information such as this.

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.

[frangio]

Please comment on the modiifed proposal to only retract the reference implementation:

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.

[SamWilsn]

@lightclient

Probably [don't want to update the EIP].

@xinbenlv

We should treat Core and ERC separately. Whether we merge this depends on the specific security issue. Would be okay with a separate informational EIP.

@gcolvin

I don't have an opinion at this point.

[bumblefudge]

I don't have a problem with pointers back from the new EIP to the old one, (like requires.) A supersedes preamble field is a very popular request. In that framework, you could release a new copy of ERC-1271 with the updated information that supersedes the old one.

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 supercede it, then supersedes could be a useful fix for cases like these.

It would not be a permissioned field though. Anyone could claim that their EIP supersedes 1271.

This alters the semantics a tiny bit-- if anyone can claim to supercede my EIP without my permission, perhaps it's more like update or extension? Seems more realistic than explicitly requiring approval from superceded-document's author to add a frontmatter claim, tho.

Similarly, if the older doc was a normative EIP or ERC, and the later one is just a BCP or other informational one, is supercede really the word? I perhaps update or extension makes more sense for this reason as well.

[frangio]

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:

1. Simply add a "RETRACTED" message at the top of the reference implementation section.
2. Delete the reference implementation section and replace it with "[RETRACTED]".
3. Delete it, and leave a message explaining the reasons it was retracted.

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.

[bumblefudge]

Similarly, if the older doc was a normative EIP or ERC, and the later one is just a BCP or other informational one, is supercede really the word? I perhaps update or extension makes more sense for this reason as well.

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).

[SamWilsn]

This is well past its deadline, so last call for comments from editors: @gcolvin, @xinbenlv, @axic, @g11tech.

[g11tech]

No opinion at this time

[SamWilsn]

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.