Skip to content
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

OQS security response process #124

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

OQS security response process #124

wants to merge 17 commits into from

Conversation

SWilson4
Copy link
Member

@SWilson4 SWilson4 commented Jan 3, 2025

This PR proposes a security response process for OQS. Please read it over and provide feedback. I expect it will undergo a fair bit of revision, but hopefully this gives us a starting point.

I have left a number of comments requesting feedback on specific points throughout the document. These are clearly marked by COMMENT. I'll keep the PR in draft until all of these are removed.

Closes #60.

SWilson4 and others added 2 commits January 3, 2025 16:12
Co-authored-by: Douglas Stebila <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
Signed-off-by: Spencer Wilson <[email protected]>
@dstebila
Copy link
Member

dstebila commented Feb 3, 2025

I think this can be marked as "Ready for review" and proceed with getting final reviews towards merging.

* Add security announcement mailing list

Signed-off-by: Spencer Wilson <[email protected]>
@SWilson4 SWilson4 marked this pull request as ready for review February 4, 2025 13:28
Copy link

@ashman-p ashman-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be addressed how members get added to the VMT? And is there a succession plan?

@baentsch
Copy link
Member

baentsch commented Feb 6, 2025

@baentsch Do recent updates resolve some of the discussion threads for you?

Some, Yes (marked resolved). Some not (please check for further feedback). Thanks for keeping this moving @SWilson4 .

@SWilson4
Copy link
Member Author

SWilson4 commented Feb 6, 2025

Should it be addressed how members get added to the VMT? And is there a succession plan?

Good point. In the latest commit I've proposed that members are added/removed via TSC vote, with provisions for voluntary self-removal or absence up to one year.

Signed-off-by: Spencer Wilson <[email protected]>
@SWilson4 SWilson4 requested a review from baentsch February 21, 2025 15:19
@SWilson4
Copy link
Member Author

@open-quantum-safe/tsc @open-quantum-safe/security-managers

I think that all discussions have now been resolved satisfactorily. Please review the latest version so we can get this merged!

This email is an alias for the members of the VMT.
2. GitHub security advisories.
These are submitted similarly to GitHub issues but are private.
3. Encrypted email to [email protected].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to phase this out to eliminate a single point of failure (not indicating you'd be one, @dstebila , but to remove an undue onus) --> Worthwhile considering setting up a secure messaging key somewhere for "[email protected]" to retain the possibility for encrypted communication?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern with this approach would be key management. How would we share such a key, and would we have to rotate it every time somebody leaves the VMT? I'll leave this one up to @dstebila.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't received any such reports yet, so the need for this seems relatively small, and I not worried at the moment about the burden of receiving these. I understand the single point of failure argument, however. We could also just drop this option entirely if we do not think there will be disclosures of such high sensitivity that would be inappropriate for the other two mechanisms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd then vote to drop option 3. GH submissions are arguably pretty secure, no? Only drawback with that would be if no-one gets notified -- but we shall check/verify that that works.

Copy link
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See additional single comments and apologies for not wrapping them into one coherent review - I simply didn't assume there'd still be so many items in need of clarification :-(

Signed-off-by: Spencer Wilson <[email protected]>
@SWilson4
Copy link
Member Author

See additional single comments and apologies for not wrapping them into one coherent review - I simply didn't assume there'd still be so many items in need of clarification :-(

The one-liners are addressed in the latest commit; others are addressed in the relevant threads.


The Vulnerability Management Team (VMT) is responsible for responding to reports of security vulnerabilities in OQS software.

### Members

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a required minimum number of people in the VMT in order to maintain the security-supported status? I'm typically an optimistic person, but just thinking of the glass half-empty scenario if too many folks are removed.

Also, thoughts on just referring to the VMT security-managers alias rather than needing to keep this list of people in sync with that source of truth?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just thinking of the glass half-empty scenario if too many folks are removed.

Normally, I am the personified "glass-half-empty" type of person, so I'm sad you bet me to that :-) More seriously, the term "folks are removed" gives me pause: What if (too many) "VMT people" are not only actually removed/formally leaving but simply don't show up any more/"don't feel like it"/are in an extended leave/whatever? And indeed, if only for this reason, referencing a single file with a source of truth would be better.


### Coverage

Non-absent members of the VMT take it in turns to acknowledge and triage reports, rotating at each OQS Technical Steering Committee meeting, in the order listed here (which also happens to be alphabetical by last name).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest using an active/inactive term instead of "non-absent".

This email is an alias for the members of the VMT.
2. GitHub security advisories.
These are submitted similarly to GitHub issues but are private.
3. Encrypted email to [email protected].

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

These are submitted similarly to GitHub issues but are private.
3. Encrypted email to [email protected].

When a report is received via one of the first two methods, the "on call" member of the VMT is responsible for acknowledging it promptly (the OSSF recommends within 1-2 days).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the current wording of reporting method #3, is the implication that dsteblia would be responsible for acknowledging if it comes in through that email account?

No assessment of the report is necessary at this time.
The purpose of the acknowledgement is to let the reporter(s) and the rest of the VMT know that the issue is being looked at.

The responding VMT member assumes the role of Quarterback for the issue.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a duplicate of line 60?

@dstebila
Copy link
Member

I think most members of the proposed Vulnerability Management Team has commented on this, but tagging those who haven't yet commented just in case: @bhess, @praveksharma

Copy link
Member

@bhess bhess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this work @SWilson4! See the single comment inline.


OQS packages cryptographic algorithm implementations from a variety of upstream sources.
These vary widely in level of support and software development expertise.
OQS assumes no obligation to fix vulnerabilities in upstream code, but may choose to do so on a best-effort basis.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If OQS decides to fix upstream code on a best-effort basis (e.g., with a local patch) but the upstream remains unfixed, I think the process should outline the need for a warning about this in the security advisory text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decide security (issue) report handling team and procedure
6 participants