Update config.yaml

[baentsch]

That file represents the state of the access as configured in GitHub at the time I created it. If it does not represent the community, or the decisions made therein, please file an issue to document the changes required.

Originally posted by @ryjones in #6 (comment)

As requested: Ensure all (sub-)project roles of contributor- and maintainership are properly represented.

At the least implement #7 (comment)

Then I think @thomwiggers no longer maintains liboqs-rust, right?
@vsoftco in turn should be maintainer of all language wrappers
@baentsch is maintainer of oqs-provider and liboqs; @thb-sb: Would you want to commit being maintainer in either or just one project, too?
@dstebila is no maintainer of oqs-provider but surely of liboqs.
All other projects don't have maintainers, AFAIK (which is a problem and should be tackled -- as per #2

Anyone else, please chime in if I forgot about you(r commitments).

[ghost]

@baentsch is maintainer of oqs-provider and liboqs; @thb-sb: Would you want to commit being maintainer in either or just one project, too?

I am definitely committed to being a maintainer of oqs-provider. Regarding liboqs, I feel like I haven't done enough work on it to be a maintainer, but I would want to be a committer for now, yes.

Thank you btw @baentsch for updating this file.!

[thomwiggers]

Then I think @thomwiggers no longer maintains liboqs-rust, right?

Yep. I'm currently basically just holding the fort.

[baentsch]

Thank you btw @baentsch for updating this file.!

I definitely will not update the file. This is an LF tool (I don't feel sufficiently motivated or paid to learn :-) and am looking at LF/@ryjones to implement updates as per discussions/decisions documented in this issue.

[dstebila]

That file represents the state of the access as configured in GitHub at the time I created it. If it does not represent the community, or the decisions made therein, please file an issue to document the changes required.

Originally posted by @ryjones in #6 (comment)

As requested: Ensure all (sub-)project roles of contributor- and maintainership are properly represented.

At the least implement #7 (comment)

Then I think @thomwiggers no longer maintains liboqs-rust, right? @vsoftco in turn should be maintainer of all language wrappers @baentsch is maintainer of oqs-provider and liboqs; @thb-sb: Would you want to commit being maintainer in either or just one project, too? @dstebila is no maintainer of oqs-provider but surely of liboqs. All other projects don't have maintainers, AFAIK (which is a problem and should be tackled -- as per #2

Anyone else, please chime in if I forgot about you(r commitments).

This list of changes looks correct to me.

[baentsch]

@ryjones are you willing to implement these changes as part of your contributions to OQS or do you expect someone else to do this? If the latter, whom and how (documentation)?

[ryjones]

@baentsch the way to do it would be to file a PR against the config to make it reflect what the community wants. Here is an example.

[baentsch]

@dstebila I know you have other priorities than this project particularly as the term starts and most likely don't (have time to) read my longish arguments, but in this case, please make an exception. I promise I (already started) to not tag you anymore on other or concrete technical matters if not really necessary.

This issue didn't make progress over the past few months and seems to be at the core of different problems. As much decision-making seems to have moved away from GH and on to meetings (which I still think is wrong, slows down the project substantially, leads to sub-optimal decisions and systematically disadvantages non-native speakers --maybe another topic of its own?) this is to request we put this on the agenda for the next meeting.

To be agreed should be things on two levels:

Basic principles:

This is to propose we agree that OQS project access rights control, i.e., config.yml maintenance, adheres to these principles:

• Two approvals by TSC members are required to merge a change to this file; one approval is sufficient for other changes to the TSC GH repo
• GOVERNANCE.md files' contents in sub projects (where present) shall never be overruled in config.yml
• The logic in config.yml shall be the same across sub projects (see next level) -- if not possible, exceptions should be documented in the file so everyone can understand the rationale later on
• LF admin overrules to these principles must be openly documented as to the reason of the overrule

Second, concrete implementation:

As per (slightly modified) original proposal by @SWilson4 this proposal is to agree that for every sub project the same rules apply:

• sub project maintainers have admin rights on the sub projects
• OQS and sub project release managers have maintainer rights on the sub projects
• sub project committers have write rights on all branches of the sub projects
• sub project contributors have write rights on all branches except main on those sub projects
• OQS and sub project triage actors have triage rights on all branches of the sub projects
• OQS maintainers and LF admins have admin rights on the organization (e.g., org-wide secret management) as well as maintenance rights on the team configurations

The rationale for all this is to

• enhance project security and accountability (e.g., less risk of further least privilege violations)
• improve openness & understanding (returns control to each sub project community)
• get clarity on roles and responsibilities
• reduce (also key person) risk
• support smooth releases
• ease maintenance

The only truly new concept this proposal introduces is the notion ("team") of release managers. IMO those should be the OQS maintainers (you and me) as well as 100% committed (e.g., employed) core team members (Spencer and Pravek). This would allow us to return to the more efficient method of doing releases without introducing undue risks, again, incl. key person risk.

Should anyone other on or off the @open-quantum-safe/tsc read this, please weigh in on this even before the meeting with comments/change suggestions, etc: I firmly believe that open discussion outside of meetings gets better results than snap decisions e.g., because one "has to run to the next meeting" or misunderstands a verbal cue.

Particularly also tagging @ryjones for input as you (unlike me) know the syntax and semantics underlying this file as well as the interaction with CODEOWNER files and piece-part access controls required, e.g., during releases.

[SWilson4]

Second, concrete implementation:

As per (slightly modified) original proposal by @SWilson4 this proposal is to agree that for every sub project the same rules apply:

* sub project maintainers have admin rights on the sub projects

* OQS and sub project release managers have maintainer rights on the sub projects

* sub project committers have write rights on all branches of the sub projects

* sub project contributors have write rights on all branches except main on those sub projects

* OQS and sub project triage actors have triage rights on all branches of the sub projects

* OQS maintainers and LF admins have admin rights on the organization (e.g., org-wide secret management) as well as maintenance rights on the team configurations

Thanks for this proposal, Michael. I think the configuration largely makes sense, though I'd like to highlight a couple of points for discussion:

• Personally, I prefer to not be able to push directly to main as it's far too easy to do accidentally from the command line (branch protection rules have saved me multiple times in this project and previous jobs). Even if there is a fallback or warning that requires a git push -f to override, I do a lot of rebases and hence a lot of force pushes, so it wouldn't help me much. If I'm the only committer with this concern, then fine by me to proceed; I'll just add a hook to my git setup to make sure I don't push to main by mistake.
• I recently discovered that anybody with write permissions has the power to add and overwrite CI environment variables and secrets via the GitHub REST API (notably, via the GitHub CLI). This was very handy for me recently, but it does mean that people with write perms have a fair bit of power to screw things up more or less silently. Do we want contributors to have this power? It seems to be a package deal along with being able to create branches on the base repo (which makes sense as the latter basically gives you the ability to run whatever you like in CI). I don't have a solution for this, but I thought it was worth bringing up.

[baentsch]

prefer to not be able to push directly to main

Good point. (These mistakes happening) also applies to me. Then strike that part. Push-to-main disallowed for all except in release situations -- except during releases: Release managers should be able to manually enable that for/before release and re-activate the restriction after release. Question remaining: What then is the conceptual difference (in rights required) for committers and contributors?

I recently discovered that anybody with write permissions has the power to add and overwrite CI environment variables and secrets via the GitHub REST API (notably, via the GitHub CLI).

Yikes -- that indeed sounds pretty strange. Is there maybe some limitation/check as to how the credentials are created that are required for using the GH REST API (which in turn could be properly controlled)? But then again, what damage can be done this way? Someone can run GH jobs without proper authorization. Doesn't sound really bad. Anything else? What about other, non GH-related secrets, e.g., docker hub credentials, etc.? Are they also accessible via GH CLI?

[ryjones]

as far as branch protection, we can roll out default branch protection (no force pushes), but it is also settable on a per-repo basis.

[SWilson4]

Yikes -- that indeed sounds pretty strange. Is there maybe some limitation/check as to how the credentials are created that are required for using the GH REST API (which in turn could be properly controlled)?

I'm not sure if/how we can control credential creation. I would guess that it's out of our hands for the most part.

But then again, what damage can be done this way? Someone can run GH jobs without proper authorization. Doesn't sound really bad. Anything else? What about other, non GH-related secrets, e.g., docker hub credentials, etc.? Are they also accessible via GH CLI?

Any secret that we store in a GitHub environment or repo variable can be overwritten (right now, this includes CircleCI and Docker Hub tokens for our "bot" accounts). Note that this is write-only access: the secret value itself can't actually be fetched. The same doesn't apply for organization-level secrets; those require admin access to manage.

The worst-case scenario that I can think of (off the top of my head) for write-only access is denial of service. Somebody overwrites a token, preventing us from deploying Docker code or breaking our CI. Someone with push access can already do quite a bit more than this: they can run arbitrary code (via workflow files) with access to environment secrets. This would allow somebody, to, say, push an unauthorized Docker image to an official account.

[baentsch]

I'm not sure if/how we can control credential creation. I would guess that it's out of our hands for the most part.

What do you mean by that? Reading token creation documentation this seems to be a controlled (or at least controllable) process:

If you selected an organization as the resource owner and the organization requires approval for fine-grained personal access tokens, then your token will be marked as pending until it is reviewed by an organization administrator. Your token will only be able to read public resources until it is approved.

--> So, what is the org setting for OQS? What should it be?

The worst-case scenario that I can think of (off the top of my head) for write-only access is denial of service.

That doesn't sound overly bad -- particularly considering this can always be tracked to an individual responsible. If there's nobody seeing different and more negative attack potential, I'd suggest to accept the risk of your second bullet point, @SWilson4 . What would be good to confirm, though, is that indeed secrets cannot be read (for later use): Anyone (incl. @ryjones ) aware of such assertion? Regarding writes/changing secret values: Is there a record (or even better, notification) of such change?

as far as branch protection, we can roll out default branch protection (no force pushes), but it is also settable on a per-repo basis.

AFAIK we already have (at least had at the time when I was trusted administrator) that protection set at least on "sensitive" repos. Admins (and ideally also release managers) can (and should be able to) set/configure that. That brings back the old question: Which privilege (level) is required to change the rule set/enable/disable branch protection? Does the proposal above make this possible? https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches seems to indicate this privilege to be admin only. In such case, what about enabling auto merge for specifically tagged PRs (release ones, created by the specifically nominated release managers as per the above proposal)? @SWilson4 : This would alleviate your concern to "accidentally" push to "main", right? It also would dovetail nicely with your new CI-task trigger logic.

[SWilson4]

What do you mean by that? Reading token creation documentation this seems to be a controlled (or at least controllable) process:

I was thinking of classic PATs as being a problem. But I missed this line in the docs:

Organization owners can set a policy to restrict the access of personal access tokens (classic) to their organization. For more information, see "Setting a personal access token policy for your organization."

--> So, what is the org setting for OQS? What should it be?

Currently, it seems to be very permissive: I've created and used both classic PATs and fine-grained PATs with no issues. I'd say that this seems to be a good reason to disable classic PATs and require approval for fine-grained PATs (possibly with policy limiting them to committers?)

In such case, what about enabling auto merge for specifically tagged PRs (release ones, created by the specifically nominated release managers as per the above proposal)? @SWilson4 : This would alleviate your concern to "accidentally" push to "main", right? It also would dovetail nicely with your new CI-task trigger logic.

Makes sense to me.

[ryjones]

@baentsch secrets are hard to exfil.

Here is an extract of the audit log

[baentsch]

I'd say that this seems to be a good reason to disable classic PATs and require approval for fine-grained PATs (possibly with policy limiting them to committers?)

Reasonable proposal. Finally something setting committers and contributors apart :-) Doable, @ryjones ?

secrets are hard to exfil.

Sorry, I don't understand this sentence. Can you please elaborate (maybe providing a pointer as to why "exfil"(?) is hard), @ryjones ?

[ryjones]

It used to be easy to exfiltrate secrets by doing a MIME encoding or similar. GitHub has patched all of those paths I'm aware of.

[dstebila]

@dstebila I know you have other priorities than this project particularly as the term starts and most likely don't (have time to) read my longish arguments, but in this case, please make an exception. I promise I (already started) to not tag you anymore on other or concrete technical matters if not really necessary.

Thanks for such a detailed proposal around this, Michael. We have been rather inconsistent and ad hoc with this over the past few months, and you're right that we should be more systematic about it. I think by and large your proposal makes sense. In terms of operationalizing it, it does sound like there will be a lot of conditions to keep track of on what approvals are required for a certain type of access change, and that might change depending on the subproject. Do you have some ideas on how to make that easier for people to evaluate? For example, maybe some pull request templates that record the various situations?

[baentsch]

Do you have some ideas on how to make that easier for people to evaluate? For example, maybe some pull request templates that record the various situations?

Such PR template (documenting the agreed-upon proposal for TSC members to check off when reviewing) would be a sensible step, I agree. Some PR-checking automation could also be deployed (e.g., evaluating MAINTAINER and COMMITTER files -- as and if established). But that's all icing on the cake: Let's first agree on the baseline proposal, @dstebila