Conversation
66180d1 to
3f6ca32
Compare
|
|
||
| Given the adversarial models above, the following are examples of issues considered security-relevant and should be reported in accordance with [Reporting Security Issues](#reporting-security-issues): | ||
|
|
||
| * Implementation defects that compromise confidentiality, integrity, or availability (e.g. memory safety bugs, undefined behavior) |
There was a problem hiding this comment.
are examples of issues considered security-relevant
undefined behavior
I think that we need to qualify this. E.g. s2n-tls was adding 0 to a null pointer in a few places. That was technically undefined behavior, but I do not consider that security relevant.
There was a problem hiding this comment.
I think I'm OK leaving this unqualified. The "adding 0 to NULL is UB" seems to be fixed more recently (llvm/llvm-project#113062), and I kind of doubt there are many more false-positive UB findings out there such that we can't handle them on a case by case basis (ie I'm fine treating them as security-relevant until proven otherwise)
SECURITY.md
Outdated
|
|
||
| * Implementation defects that compromise confidentiality, integrity, or availability (e.g. memory safety bugs, undefined behavior) | ||
| * Logic bugs that lead to incorrect TLS negotiation, handshake errors, or authentication bypass | ||
| * Negotiation of weak or non-policy-approved cryptographic algorithms |
There was a problem hiding this comment.
Once again, tricky. Customers can allow themselves to negotiate RC4/SSLv3. I would bet that we also allow RSA1024
There was a problem hiding this comment.
Yeah I was thinking about this one after the fact. If a took out "weak" would that be sufficient you think?
"Negotiation of non-policy-approved cryptographic algorithms"
or I could be more specific like:
"Negotiation of cryptographic algorithms not specified in the configured security policy"
SECURITY.md
Outdated
| @@ -0,0 +1,78 @@ | |||
| # Security Policy | |||
There was a problem hiding this comment.
I think there is a missing qualification here where s2n-tls supports a wide variety of behaviors that are not following best practice, but may be acceptable for some specific threat models.
- non-PQ key exchange
- TLS 1.2 session resumption
- RSA key exchange
- 1024 bit RSA certs
- RC4/3DES
- SSLv3
But I think the security policy as written would immediately make these security vulnerabilities.
- Obtain long-term secrets (e.g. private keys) of any client or server after a session is complete
This makes RSA key exchange and TLS 1.2 resumption a security issue
- Record encrypted traffic now for future decryption once a cryptographically relevant quantum computer is available (harvest now, decrypt later)
This makes non-pq key exchange a security issue.
There was a problem hiding this comment.
Good point, I've added some content to address this
SECURITY.md
Outdated
| @@ -0,0 +1,78 @@ | |||
| # Security Policy | |||
There was a problem hiding this comment.
What about Security Posture/threat model instead of Policy? I'd rather not overload our use of "Security Policy".
There was a problem hiding this comment.
I went with "Security Reporting Policy"
SECURITY.md
Outdated
|
|
||
| ### Out of Scope Attacks | ||
|
|
||
| Applications integrating with s2n-tls are responsible for the security of the host on which the process loading the s2n-tls library runs. This means the following attacks are considered out of scope: |
There was a problem hiding this comment.
I don't know if I agree with this. Yes, it's outside of our threat model, but I don't want people to stop reporting this stuff to us.
I want to encourage users to report stuff to us, rather than discouraging it. We should be biased towards fixing issues even if there is no practical exploit.
There was a problem hiding this comment.
I also would like us to still accept reports of related issues, and clearly say we reserve the right to decide if mitigations in s2n-tls make sense.
I think physical observations likely remain out of scope, but thinks like fault injections where the advantage is due to easily improvement implementation with minimal impact to performance or maintainability, would be something we could accept as an improvement.
There was a problem hiding this comment.
Thanks for the feedback, I've incorporated these suggestions
|
|
||
| If you are unsure whether an issue is security-relevant, please err on the side of caution and report it through the [Reporting Security Issues](#reporting-security-issues) process. | ||
|
|
||
| ## Prenotification Policy |
There was a problem hiding this comment.
Is this intended to be a duplicate of the README.md policy that offers pre-notification of releases (not specifically security related issue/releases?)
There was a problem hiding this comment.
Actually I see this policy is duplicated but both are in the Security issues area. I am okay with over-communicating in this instance because someone could get here without reading to the "Security issue notifications" section of the README.md
There was a problem hiding this comment.
yeah, a few things are duplicated in various places but I had the same thought, that people will arrive at these different docs in different ways, so better to just include the information everywhere
Goal
Adds a SECURITY.md containing guidance around the s2n-tls threat model and other policies related to security
Why
Provide guidance to contributors on s2n-tls considers threats and how and when to report them
How
Providing guidance on out of scope threats, adversarial models, and examples of in and out of scope vulnerabilities.
Callouts
The Security Policy linked at https://github.com/aws/s2n-tls/security/policy is an AWS-wide policy that will remain in place, but I've linked this s2n-tls specific policy in the README.
Testing
Documentation-only change, no code modified.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.