Some ideas for process improvements/changes

Message ID 11b1c515a2a0ed2af0c72ac6437aca81ba0806a7.camel@klomp.org
State Committed
Headers
Series Some ideas for process improvements/changes |

Commit Message

Mark Wielaard April 6, 2023, 4:30 p.m. UTC
  Hi hackers,

In general it feels like the elfutils community is working well, there
are regular releases with bug fixes and new features. Most patches are
reviewed fairly quickly (although there are some exceptions where
patches have been pending too long). So I don't want to change too
much. But here are some small suggestions for changes to out processes
that might be helpful:

- Get rid of ChangeLog files and trivial ChangeLog entries
  I personally love ChangeLog entries. Writing them helps me
  double check I actually intended to make the changes. And
  it is a great help reviewing patches. It helps having to
  guess if some specific change was an accident or intended.

  But patches that have changes against the ChangeLog files are
  sometimes hard to rebase or move between branches. The gnulib
  git-merge-changelog driver is awesome, but is not always able
  to help. Also some commit messages for smaller changes are
  already fine describing what changed.

  So I propose to drop ChangeLog files completely and only add
  a ChangeLog entry to the commit message for larger changes
  to help the review process.

- Use patchwork more
  All patches sent to the mailing list are tracked at
  https://patchwork.sourceware.org/project/elfutils/list/
  It has helped me a lot keeping track of patches that
  have been pending for some time. Also git-pw has been
  really nice for cherry-picking patches.
  https://patchwork.readthedocs.io/projects/git-pw/en/latest/
  
  Please let me know if you would like to help maintain the
  pending patch list and I'll add your account as maintainer
  for the elfutils project.

  For using it with git-pw use these .git/config settings:
  [pw]
    server = https://patchwork.sourceware.org/api/1.2/
    project = elfutils
    token = <hex-token>
    states = committed,accepted,superseded,deferred,rejected,under-review

  It would be nice if it was automated a bit more by have a git
  commit hook that flagged whether a patch was committed. And if
  the buildbot try-branch system would flag pass/fail on the patch.

- Don't require "real names" in Signed-off-by lines.
  Our current CONTRIBUTING guide say that you have to use your 
  your real name for the Signed-off-by line. This is sometimes
  problematic for people for who their real (legal) name is not
  how they identify themselves to others. I suggest to change
  the requirement as follows (this mimics what the linux kernel
  project did recently):

 the commit log message for you.

- "Security" bug guidance
  Here I don't have good guidance, but I have the feeling some of
  the bugs reported (especially by some fuzzers) are sometimes
  unnecessarily marked as security issues. Which causes lots of
  unnecessary work for downstream users of our code. Especially
  if someone starts assigning CVEs to them. It would be good to
  have some explicit text to point "security" bug reporters at
  on how we will handle their bugs.

Cheers,

Mark
  

Comments

Frank Ch. Eigler April 6, 2023, 5:34 p.m. UTC | #1
Hi -

> - Get rid of ChangeLog files and trivial ChangeLog entries
>   [...]

Yes please!

> - Use patchwork more
>   [...]

This doesn't seem like something for community/contributors
to do - patchwork seems mostly a maintainer/committer tool.

>   It would be nice if it was automated a bit more by have a git
>   commit hook that flagged whether a patch was committed. And if
>   the buildbot try-branch system would flag pass/fail on the patch.

Sounds like a sourceware infrastructure RFE.

> - Don't require "real names" in Signed-off-by lines.
>   [...]
> +The name you use as your identity should not be an anonymous id
> +or false name that misrepresents who you are.

(No strong opinion on this one, except that a declaration that is this
informal would have little weight, should it ever be relied upon in
legal proceedings.)


> - "Security" bug guidance
>   [...]

Yeah, a brief SECURITY file would be nice.


- FChE
  
Mark Wielaard April 6, 2023, 9:57 p.m. UTC | #2
Hi Frank,

On Thu, Apr 06, 2023 at 01:34:20PM -0400, Frank Ch. Eigler via Elfutils-devel wrote:
> > - Get rid of ChangeLog files and trivial ChangeLog entries
> >   [...]
> 
> Yes please!

So sad, on irc people are also enthousiastic about this. O well. :)

> > - Use patchwork more
> >   [...]
> 
> This doesn't seem like something for community/contributors
> to do - patchwork seems mostly a maintainer/committer tool.

But I want more community/contributors to feel like they are
maintainers/committers!

> >   It would be nice if it was automated a bit more by have a git
> >   commit hook that flagged whether a patch was committed. And if
> >   the buildbot try-branch system would flag pass/fail on the patch.
> 
> Sounds like a sourceware infrastructure RFE.

Yes, but if I RFE that then it often just comes back to me to add it
:) So I mention it here in the hope someone says "O, but that is easy,
this is exactly how to do it..."

> > - Don't require "real names" in Signed-off-by lines.
> >   [...]
> > +The name you use as your identity should not be an anonymous id
> > +or false name that misrepresents who you are.
> 
> (No strong opinion on this one, except that a declaration that is this
> informal would have little weight, should it ever be relied upon in
> legal proceedings.)

Do you feel this weakens our Developer's Certificate of Origin
process? My point is that we shouldn't judge what is a "real name" or
not. But the name shouldn't misrepresent who someone is. What we care
about is that the identity people use to sign the certificate refers
to a real person that can be contacted about their contributions when
needed.

> > - "Security" bug guidance
> >   [...]
> 
> Yeah, a brief SECURITY file would be nice.

Any suggestions about what to put in such a section or file.  My main
concern is that people are filing things we regard as simple bugs as
"security" issues and get CVEs assigned which cause lots of extra work
for some of our downstream users. I think we should be clear that we
want to fix all bugs and don't want to get dragged into embargoed
security theater.
https://daniel.haxx.se/blog/2023/03/29/pre-notification-dilemmas/

Cheers,

Mark
  
Frank Ch. Eigler April 7, 2023, 12:56 a.m. UTC | #3
Hi -


> > > - Get rid of ChangeLog files and trivial ChangeLog entries
> > >   [...]
> > 
> > Yes please!
> 
> So sad, on irc people are also enthousiastic about this. O well. :)

I hereby call for elfutils contributors to periodically send Mark some
gift ChangeLog messages (in private email) to cheer him up! :-)


> > Sounds like a sourceware infrastructure RFE.
> 
> Yes, but if I RFE that then it often just comes back to me to add it
> :) So I mention it here in the hope someone says "O, but that is easy,
> this is exactly how to do it..."

Yeah.  This sounds like someone(tm) ought to have done before, letting
patchwork monitor git traffic, and likely not a quick/small hack.


> > (No strong opinion on this one, except that a declaration that is this
> > informal would have little weight, should it ever be relied upon in
> > legal proceedings.)
> 
> Do you feel this weakens our Developer's Certificate of Origin
> process? 

(I'm not a lawyer, but ISTM the process is mostly perfunctory anyway.)

> My point is that we shouldn't judge what is a "real name" or
> not. But the name shouldn't misrepresent who someone is. 

Understood.  Operationally, we have no way of verifying anything
beyond an email address, if even that.


> > > - "Security" bug guidance
> > >   [...]
> > 
> > Yeah, a brief SECURITY file would be nice.
> 
> Any suggestions about what to put in such a section or file.  My main
> concern is that people are filing things we regard as simple bugs as
> "security" issues [...]


Yeah, that's a pain.  How about:

"""

The elfutils library and utilities aim to be generally robust and
reliable.  However, elfutils routinely processes complex binary
structured data.  This makes the code intricate and sometimes brittle.
While elfutils developers use a variety of static and dynamic checker
software (valgrind, sanitizers) in testing, bugs may remain.  Some of
these bugs may have security-related implications.


While many errors are cleanly detected at runtime, it is possible that
vulnerabilities exist that could be exploitable.  These may arise from
crafted / fuzzed / erroneous inputs, or perhaps even from valid inputs
with unforseen characteristics.  Therefore, to minimize risks, users
of elfutils tools and libraries should consider measures such as:

- avoiding running complex elfutils analysis on untrustworthy inputs
- avoiding running elfutils tools as privileged processes
- applying common platform level protection mechanisms such as
  selinux, syscall filtering, hardened compilation, etc.

Since most elfutils tools are run in short-lived, local, interactive,
development context rather than remotely "in production", we generally
treat malfunctions as ordinary bugs rather than security vulnerabilities.


Elfutils includes one network client/server: debuginfod.  The
debuginfod man page contains a SECURITY section outlining the general
risks.  tl;dr: many classes of server problems are delegated to
front-end proxies and curated elf/dwarf archives of the operator;
others to careful configuration of the debuginfod client.  These are
not generally reportable as security vulnerabilities.  However, we are
likely to accept security vulnerability reports related to:

- availability: e.g., remotely exploitable server crash, but not
  routine resource exhaustion or overload; client crash due to
  unexpected valid traffic from trusted server

- confidentiality: e.g., allowing the server to expose one client's
  traffic to another client

- integrity: e.g., causing the server to send erroneous
  elf/dwarf/source data across the webapi; causing the client to
  corrupt its cache to lose file integrity

We welcome reports that are tangential to any of these subjects.

Please report bugs via any of:
- email to <elfutils-devel@sourceware.org>
- https://sourceware.org/bugzilla/enter_bug.cgi?product=elfutils

After considering the above exclusions, please report suspected
security vulnerabilities confidentially via any of:

- email to <mark@klomp.org>
- email to <fche@elastic.org>
- email to <secalert@redhat.com>

"""


- FChE
  
Mark Wielaard April 7, 2023, 8:26 p.m. UTC | #4
Hi Frank,

On Thu, Apr 06, 2023 at 08:56:00PM -0400, Frank Ch. Eigler via Elfutils-devel wrote:
> > > > - "Security" bug guidance
> > > >   [...]
> > > 
> > > Yeah, a brief SECURITY file would be nice.
> > 
> > Any suggestions about what to put in such a section or file.  My main
> > concern is that people are filing things we regard as simple bugs as
> > "security" issues [...]
> 
> 
> Yeah, that's a pain.  How about:
> 
> """
> 
> The elfutils library and utilities aim to be generally robust and
> reliable.  However, elfutils routinely processes complex binary
> structured data.  This makes the code intricate and sometimes brittle.
> While elfutils developers use a variety of static and dynamic checker
> software (valgrind, sanitizers) in testing, bugs may remain.  Some of
> these bugs may have security-related implications.
> 
> 
> While many errors are cleanly detected at runtime, it is possible that
> vulnerabilities exist that could be exploitable.  These may arise from
> crafted / fuzzed / erroneous inputs, or perhaps even from valid inputs
> with unforseen characteristics.  Therefore, to minimize risks, users
> of elfutils tools and libraries should consider measures such as:
> 
> - avoiding running complex elfutils analysis on untrustworthy inputs
> - avoiding running elfutils tools as privileged processes
> - applying common platform level protection mechanisms such as
>   selinux, syscall filtering, hardened compilation, etc.
> 
> Since most elfutils tools are run in short-lived, local, interactive,
> development context rather than remotely "in production", we generally
> treat malfunctions as ordinary bugs rather than security vulnerabilities.
> 
> 
> Elfutils includes one network client/server: debuginfod.  The
> debuginfod man page contains a SECURITY section outlining the general
> risks.  tl;dr: many classes of server problems are delegated to
> front-end proxies and curated elf/dwarf archives of the operator;
> others to careful configuration of the debuginfod client.  These are
> not generally reportable as security vulnerabilities.  However, we are
> likely to accept security vulnerability reports related to:
> 
> - availability: e.g., remotely exploitable server crash, but not
>   routine resource exhaustion or overload; client crash due to
>   unexpected valid traffic from trusted server
> 
> - confidentiality: e.g., allowing the server to expose one client's
>   traffic to another client
> 
> - integrity: e.g., causing the server to send erroneous
>   elf/dwarf/source data across the webapi; causing the client to
>   corrupt its cache to lose file integrity
> 
> We welcome reports that are tangential to any of these subjects.

Very nice. I would not have been able to describe things so
nicely. Thanks.

> Please report bugs via any of:
> - email to <elfutils-devel@sourceware.org>
> - https://sourceware.org/bugzilla/enter_bug.cgi?product=elfutils
> 
> After considering the above exclusions, please report suspected
> security vulnerabilities confidentially via any of:
> 
> - email to <mark@klomp.org>
> - email to <fche@elastic.org>
> - email to <secalert@redhat.com>

I am happy to receive such reports. And I trust the secalert team at
Red Hat. But I would like to hear from other distro maintainers what
they think of this policy. I would like to not give one distro or
corporate entity preferred early notification of suspected security
issues.

Also I think we should explicitly add something like:

  We are happy to keep the security implication of a bug, or a proof
  of concept exploit private. But unless the bug is critical we'll
  resolve the underlying issue through the normal project development
  processes, without mentioning the security vulnerability itself.

To make clear we aren't going to play security embargo theater.

Cheers,

Mark
  
Frank Ch. Eigler April 7, 2023, 8:44 p.m. UTC | #5
Hi -

> Very nice. I would not have been able to describe things so
> nicely. Thanks.

My pleasure!

> > - email to <mark@klomp.org>
> > - email to <fche@elastic.org>
> > - email to <secalert@redhat.com>
> 
> I am happy to receive such reports. And I trust the secalert team at
> Red Hat. But I would like to hear from other distro maintainers what
> they think of this policy. I would like to not give one distro or
> corporate entity preferred early notification of suspected security
> issues.

I believe there is an inter-company mailing list for high profile
security issues.  We'd eventually hear back from the RH reps on that
list.  Heck, our own RH reps could/would forward things there for high
severity things that come initially to you or me.


> Also I think we should explicitly add something like:
> 
>   We are happy to keep the security implication of a bug, or a proof
>   of concept exploit private. But unless the bug is critical we'll
>   resolve the underlying issue through the normal project development
>   processes, without mentioning the security vulnerability itself.

I can't think of much harm to mentioning security vulnerabilities by
name, should they rise to the levels to justify issuance of them at
all.  That is, we need not hide genuine security issues that happen to
be low severity.  We just wouldn't treat them with embargo or
emergency haste, but that's okay.


> To make clear we aren't going to play security embargo theater.

Not sure that's fair -- in my experience, security embargos serve a
useful purpose: in coordinating the release of a fix among distros.
That minimizes the window of vulnerability of the entire user base
between the problem becoming public and distributing its fix.  They
are thankfully rare.


- FChE
  
Mark Wielaard Oct. 19, 2023, 4:13 p.m. UTC | #6
Hi hackers,

Coming back to this, because we did start doing some of these, but
didn't actually document all of these suggestions.

On Thu, 2023-04-06 at 18:30 +0200, Mark Wielaard wrote:
> In general it feels like the elfutils community is working well, there
> are regular releases with bug fixes and new features. Most patches are
> reviewed fairly quickly (although there are some exceptions where
> patches have been pending too long). So I don't want to change too
> much. But here are some small suggestions for changes to out processes
> that might be helpful:
> 
> - Get rid of ChangeLog files and trivial ChangeLog entries
>   I personally love ChangeLog entries. Writing them helps me
>   double check I actually intended to make the changes. And
>   it is a great help reviewing patches. It helps having to
>   guess if some specific change was an accident or intended.
> 
>   But patches that have changes against the ChangeLog files are
>   sometimes hard to rebase or move between branches. The gnulib
>   git-merge-changelog driver is awesome, but is not always able
>   to help. Also some commit messages for smaller changes are
>   already fine describing what changed.
> 
>   So I propose to drop ChangeLog files completely and only add
>   a ChangeLog entry to the commit message for larger changes
>   to help the review process.

Some, but not all contributors have now switched to this style of
commits. The attached patch formally documents it.

> - Use patchwork more
>   All patches sent to the mailing list are tracked at
>   https://patchwork.sourceware.org/project/elfutils/list/
>   It has helped me a lot keeping track of patches that
>   have been pending for some time. Also git-pw has been
>   really nice for cherry-picking patches.
>   https://patchwork.readthedocs.io/projects/git-pw/en/latest/
>   
>   Please let me know if you would like to help maintain the
>   pending patch list and I'll add your account as maintainer
>   for the elfutils project.
> 
>   For using it with git-pw use these .git/config settings:
>   [pw]
>     server = https://patchwork.sourceware.org/api/1.2/
>     project = elfutils
>     token = <hex-token>
>     states = committed,accepted,superseded,deferred,rejected,under-review
> 
>   It would be nice if it was automated a bit more by have a git
>   commit hook that flagged whether a patch was committed. And if
>   the buildbot try-branch system would flag pass/fail on the patch.

The automation is still not there. But I am using it happily as todo
list: https://patchwork.sourceware.org/project/elfutils/list/
Currently it lists 42 active patches, so we could use some help with
reviewing. If anybody want to become a elfutils patchwork maintainer
please let me know. Also documented in CONTRIBUTING in the attached
patch.

> - Don't require "real names" in Signed-off-by lines.
>   Our current CONTRIBUTING guide say that you have to use your 
>   your real name for the Signed-off-by line. This is sometimes
>   problematic for people for who their real (legal) name is not
>   how they identify themselves to others. I suggest to change
>   the requirement as follows (this mimics what the linux kernel
>   project did recently):
> 
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> index bb48975b..1a1c443f 100644
> --- a/CONTRIBUTING
> +++ b/CONTRIBUTING
> @@ -45,7 +45,9 @@ then you just add a line saying
>  
>  Signed-off-by: Random J Developer <random@developer.example.org>
>  
> -using your real name (sorry, no pseudonyms or anonymous
> contributions.)
> +using a known identity (sorry, no anonymous contributions.)
> +The name you use as your identity should not be an anonymous id
> +or false name that misrepresents who you are.
>  
>  git commit --signoff will add such a Signed-off-by line at the end of
>  the commit log message for you.

I have now committed the above change.

commit b770e1c4def3532c7b59c4d2e4cd3cee26d4548b
Author: Mark Wielaard <mark@klomp.org>
Date:   Thu Oct 19 17:47:28 2023 +0200

    CONTRIBUTING: Switch from real name policy to know identity policy
    
    Signed-off-by: Mark Wielaard <mark@klomp.org>

> - "Security" bug guidance
>   Here I don't have good guidance, but I have the feeling some of
>   the bugs reported (especially by some fuzzers) are sometimes
>   unnecessarily marked as security issues. Which causes lots of
>   unnecessary work for downstream users of our code. Especially
>   if someone starts assigning CVEs to them. It would be good to
>   have some explicit text to point "security" bug reporters at
>   on how we will handle their bugs.

Thanks to Frank we now have this! With the following note in our
README: See the SECURITY file for defining and handling security-
sensitive bugs.

There are two more changes I like to make, but not right now.

As part of the release (just before, or right after) later this month I
like to switch the main branch from 'master' to 'main'. It is the last
use of some harmful language in our project.
https://inclusivenaming.org/
It will need a few updates to the documentation and buildbot setup. But
we can leave an alias so nothing breaks.

Finally we do have a somewhat informal code of conduct, see the end of
our CONTRIBUTING document:

   committers/maintainers who repeatedly ignore the above guidelines,    
   are hostile or offensive towards other committers or contributors,    
   and don't correct their behavior after being asked by other committers
   will be removed as maintainer/committer.                              

It would imho be good to extend this a little to all project
contributors and adopt a formal code of conduct like the Contributor
Covenant https://www.contributor-covenant.org/
That page also has some good references on reaching agreement on
adopting such a more formal code of conduct.

Please let me know if you would like to help adopting a more formal
code of conduct and/or be part of a code of conduct committee for
elfutils.

Thanks,

Mark
  
Mark Wielaard Oct. 25, 2023, 1:10 p.m. UTC | #7
Hi,

On Thu, 2023-10-19 at 18:13 +0200, Mark Wielaard wrote:
> > - Get rid of ChangeLog files and trivial ChangeLog entries
> >   I personally love ChangeLog entries. Writing them helps me
> >   double check I actually intended to make the changes. And
> >   it is a great help reviewing patches. It helps having to
> >   guess if some specific change was an accident or intended.
> > 
> >   But patches that have changes against the ChangeLog files are
> >   sometimes hard to rebase or move between branches. The gnulib
> >   git-merge-changelog driver is awesome, but is not always able
> >   to help. Also some commit messages for smaller changes are
> >   already fine describing what changed.
> > 
> >   So I propose to drop ChangeLog files completely and only add
> >   a ChangeLog entry to the commit message for larger changes
> >   to help the review process.
> 
> Some, but not all contributors have now switched to this style of
> commits. The attached patch formally documents it.
> 
> > - Use patchwork more
> >   All patches sent to the mailing list are tracked at
> >   https://patchwork.sourceware.org/project/elfutils/list/
> >   It has helped me a lot keeping track of patches that
> >   have been pending for some time. Also git-pw has been
> >   really nice for cherry-picking patches.
> >   https://patchwork.readthedocs.io/projects/git-pw/en/latest/
> >   
> >   Please let me know if you would like to help maintain the
> >   pending patch list and I'll add your account as maintainer
> >   for the elfutils project.
> > 
> >   For using it with git-pw use these .git/config settings:
> >   [pw]
> >     server = https://patchwork.sourceware.org/api/1.2/
> >     project = elfutils
> >     token = <hex-token>
> >     states = committed,accepted,superseded,deferred,rejected,under-review
> > 
> >   It would be nice if it was automated a bit more by have a git
> >   commit hook that flagged whether a patch was committed. And if
> >   the buildbot try-branch system would flag pass/fail on the patch.
> 
> The automation is still not there. But I am using it happily as todo
> list: https://patchwork.sourceware.org/project/elfutils/list/
> Currently it lists 42 active patches, so we could use some help with
> reviewing. If anybody want to become a elfutils patchwork maintainer
> please let me know. Also documented in CONTRIBUTING in the attached
> patch.

Given nobody complained (and people seem happy with the above changes)
I have committed those changes to CONTRIBUTING now.

Cheers,

Mark

> There are two more changes I like to make, but not right now.
> 
> As part of the release (just before, or right after) later this month I
> like to switch the main branch from 'master' to 'main'. It is the last
> use of some harmful language in our project.
> https://inclusivenaming.org/
> It will need a few updates to the documentation and buildbot setup. But
> we can leave an alias so nothing breaks.
> 
> Finally we do have a somewhat informal code of conduct, see the end of
> our CONTRIBUTING document:
> 
>    committers/maintainers who repeatedly ignore the above guidelines,    
>    are hostile or offensive towards other committers or contributors,    
>    and don't correct their behavior after being asked by other committers
>    will be removed as maintainer/committer.                              
> 
> It would imho be good to extend this a little to all project
> contributors and adopt a formal code of conduct like the Contributor
> Covenant https://www.contributor-covenant.org/
> That page also has some good references on reaching agreement on
> adopting such a more formal code of conduct.
> 
> Please let me know if you would like to help adopting a more formal
> code of conduct and/or be part of a code of conduct committee for
> elfutils.
> 
> Thanks,
> 
> Mark
  

Patch

diff --git a/CONTRIBUTING b/CONTRIBUTING
index bb48975b..1a1c443f 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -45,7 +45,9 @@  then you just add a line saying
 
 Signed-off-by: Random J Developer <random@developer.example.org>
 
-using your real name (sorry, no pseudonyms or anonymous
contributions.)
+using a known identity (sorry, no anonymous contributions.)
+The name you use as your identity should not be an anonymous id
+or false name that misrepresents who you are.
 
 git commit --signoff will add such a Signed-off-by line at the end of