Adding mandatory "Security Considerations" to EIP-1

[tintinweb]

Hi,

As discussed during Eth 1.x Berlin meeting we propose to explicitly build security into the EIP process by adding a mandatory "security considerations" section. This is a follow-up action-item to the meeting as referenced by ethcatherders/PM#44

process changes:

• EIP Authors
  • must include a mandatory section "security considerations" as per EIP-1 (see template eip-X.md)
  • must provide a statement on how security was considered with the proposed change. Initially rules are intentionally relaxed and this might be updated by providing further guidance on how to write good text for security discussions.
  • are responsible for updating this section with security relevant information throughout the proposals lifecycle.
  • must provide a statement if there are no security considerations.
• EIP Editors
  • reject new submissions without "security considerations".
  • might reject submissions without sufficient security discussion.

relevant discussions:

• Adding security considerations to EIP process ethcatherders/PM#44 (follow-up / coredev eth1x meeting berlin)
• discussion/eth-magicians
• slides/coredev eth1x meeting berlin
• initial/eip-draft_security_considerations
• other/"Security Considerations" as defined in RFC7322

follow-up

• once security considerations are part of the EIP process it is suggested to create an informational EIP "Guideline on writing security considerations" specifically adapted for the ethereum ecosystem taking RFC3552 as a reference.

cheers,
tin

[maurelian]

What's required to merge this?

[axic]

follow-up
once security considerations are part of the EIP process it is suggested to create an informational EIP "Guideline on writing security considerations" specifically adapted for the ethereum ecosystem taking RFC3552 as a reference.

@tintinweb I think merging this change to EIP-1 without a guideline may not be the best idea, because right now it is very vague what would be considered an appropriate content for the section.

[tintinweb]

Oh I thought this has been merged already 😄

@axic at this time we intentionally leave enough freedom on how to write the security considerations section. We want to make security an explicit part of the EIP process without adding too much overhead or burden on the proposals potentially impacting their creativity. The initial guidance for referees (phase 1) is to reject EIPs that do not consider security at all or are blatantly failing to deal with it. That's already more information in a digestible way and way better than what we have right now.

It will also allow us to take the security considerations texts in EIPs as an input to create tailored guidance on how to deal with this section (phase 2; both for proposals and referees)

The situation as is with a security discussion not being an explicit part of the proposal clearly puts the process at risk and does not really foster trust in a secure change management for the core layer of the system. Already collecting the security discussion in a specific section would be a first step. For reference: the initial proposal that was presented included further guidance - still lightweight to leave enough freedom - and after the core-devs meeting berlin it was decided, as a first step, to go with merging the section only, as soon as possible, learn from new EIPs and provide tailored guidance based on how it is being used or misused.

Alright, so let's push this forward. How do we proceed (//ping @axic @bmann 😄)? This PR's been open for quite a while now and I must confess it unfortunately dropped off my radar but it is back on now 😄 What is the minimum requirement to get that in asap? How do we agree on that? Who decides? IMHO The sooner we allow space for security discussions in the proposal, require to deal with it in some form and make it easily available for reviewers the better.

cheers,
tin

[bmann]

Rough consensus is required ;)

@axic can we take discussion to https://ethereum-magicians.org/t/eip-mandatory-security-considerations-for-eips/2839 https://ethereum-magicians.org/t/eip-mandatory-security-considerations-for-eips/2839

@tintinweb you may also want to promote the discussion link and ask people in the security community for +1s, as well as put it on an upcoming AllCoreDevs call agenda

[shayanb]

Discussed in All Core Devs call : ethereum/pm#111 (comment)

Final consensus is to add the security consideration section to EIP1.

[tintinweb]

good to merge.

// rebased off master to resolve merge conflict introduced by changing the templates name from eip-x to eip-template.

[axic]

Discussed in All Core Devs call : ethereum/pm#111 (comment)

Final consensus is to add the security consideration section to EIP1.

The link points to ACD#65. Looking at that I do not see any "final consensus" nor decision made.

[shayanb]

The link points to ACD#65. Looking at that I do not see any "final consensus" nor decision made.

I believe the decision was made in the last minutes of the meeting (1:34:12), but may not have been documented properly.

Security Consideration is already part of some of the new EIPs: e.g. EIP-1884

[axic]

I believe the decision was made in the last minutes of the meeting (1:34:12), but may not have been documented properly.

Just listened to the audio. Nitpicking: I do not hear any explicit consensus, rather just disinterest by nobody saying anything.

From the EIP:

[..] submissions without a sufficient security discussion may be rejected.

However my earlier question was not answered about providing some kind of guidelines. How would an EIP editor decide whether the EIP has an insufficient section?

[tintinweb]

Hi @axic,

We've presented this at the CoreDevs meeting in Berlin this year and finally to the referenced CoreDevs call. There has been plenty of discussions before the meeting, after the meeting, some discussion on eth-magicians (referenced in this issue) with the overall sentiment being positive. The proposal hasn't changed since then.

I believe the decision was made in the last minutes of the meeting (1:34:12), but may not have been documented properly.

Just listened to the audio. Nitpicking: I do not hear any explicit consensus, rather just disinterest by nobody saying anything.

I believe what I would interpret as "rough consensus" (#1963 (comment)) has been achieved after 1:35:13 with one member saying ship-it (no that wasn't me 😄) followed by an explicit request for comments. No objections were raised. The call was closed.

Given that the change does not appear to be very controversial I would assume that this is sufficient, but happy to discuss otherwise.

From the EIP:

[..] submissions without a sufficient security discussion may be rejected.

However my earlier question was not answered about providing some kind of guidelines. How would an EIP editor decide whether the EIP has an insufficient section?

Security should be one of our core habits and naturally built into our processes and I was wondering why it is not explicit in the current one. Merging this change does not necessarily mean that it should put an extra burden on anyones shoulders - hence the lightweight approach allowing an EIP editor to reject submissions that do not explicitly outline security considerations AND optionally allow to reject submissions based on the EIP editors expertise. This is intentionally vague, based on trust and the EIP editors expertise. Once we learn how this section is being used we all-together will be able to derive more precise instructions for EIP editors and authors. Ideally we would get this in as soon as possible for the sake of manifesting security rather sooner than later as every new submissions that does not explicitly deal with can be a lost chance of preventing potentially severe consequences to our ecosystem. I believe this is something we all agree on.

(The proposed diff)

• The EIP does not include a "security considerations" section -> REJECT (hard requirement: provide the section)
• The EIP's "security consideration" section is blank -> REJECT (hard requirement: deal with section)
• The EIP author fails to actually discuss security implications of the change judged by the EIP editors expertise -> REJECT (soft requirement: provide useful information; not yet something we have extensive guidance for and providing guidance will likely take some time if it needs to go through this process again)

That said, stating that there was disinterest in building security into our core processes is not something I would sign. Actually, from my experience the opposite is the case. Both app-devs and core-devs care a lot about changes and especially about the impact. Some more explicit than others. This change aims to make it explicit for everyone.

Ultimately it is up to the community to decide if it makes sense and naturally I am open to any proposal that allows this to go through more easily or counter arguments why not to have it.

cheers,
tin

[axic]

I don't have time to comprehensively respond, but here's one thing:

this is intentionally vague, based on trust and the EIP editors expertise.

and

The EIP author fails to actually discuss security implications of the change judged by the EIP editors expertise

The EIP editors are not required to have any technical expertise to judge each of these topics. They are merely editors for syntax and formatting. That's why giving no guidelines for the editors may be problematic.

[tintinweb]

The EIP editors are not required to have any technical expertise to judge each of these topics. They are merely editors for syntax and formatting. That's why giving no guidelines for the editors may be problematic.

Alright, got it! Would you suggest to remove the second part of the instruction that implies that EIP's that are not sufficiently dealing with security may be rejected? Here's the controversial part of the change:

[..] EIP submissions missing the "Security Considerations" section will be rejected, submissions without a sufficient security discussion may be rejected.

[axic]

EIP submissions missing the "Security Considerations" section will be rejected, submissions without a sufficient security discussion may be rejected.

Having had some time to think about this and here are some comments.

1. It requires that every single EIP has a "Security Considerations" section, even if it is empty or useless. This is fine, it could be included in a future validator script. However I wonder if that means this PR should add a dummy section to every single EIP currently merged?

2. Instead of saying "submissions [..] may be rejected" I think the better wording is that "an EIP cannot proceed to Final status without a Security Considerations deemed sufficient by the reviewers". The current "submissions" is misleading and could be interpreted as Drafts cannot be merged without a sufficient section, which puts the onus on the editors to be [security] experts in judging the contents.

[axic]

@tintinweb can you please respond to my last comment? I'd really like to have a resolution on this PR.

[tintinweb]

@axic sorry for not getting back to you earlier and thank you for your feedback. This is so valuable and highly appreciated 🙌!

ad 1. I would want to avoid creating too much of a mess because of this change. If possible I'd suggest to require it from the point on this PR is merged. Even though this might add unwanted complexity it should be fairly easy to add this to a validator script. (Afaik this is also how it's been handled in the RFC process)

ad 2. this is excellent and makes it more clear now 👍. I have amended this PR to address your feedback. Let me know if you think it requires further clarification.

Thanks!

[axic]

@Arachnid @Souptacular @gcolvin @nicksavers any opposition merging this? If not, I'll merge on Wednesday.

[axic]

I'm merging this, since this was discussed on an ACD call months ago and nobody objected here nor on the call.