-
-
Notifications
You must be signed in to change notification settings - Fork 666
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
proposal: remove 1.14.3 as duplicate of or merge to 14.2.1 #1405
Comments
I think it is a good idea to merge them.
Checking in the build pipeline is not always desired. This can block PRs because some unrelated package became outdated.
This is pretty vague.
I don't think you necessarily need to run the latest version of software, especially if multiple major versions are supported. It's OK to be on Debian stable even though it has older software than Debian testing. |
So this raises a can of worms. I agree in principle that these should be merged but I also noted the following comment in chapter V14:
I think this comment makes a good point regarding the testability of L1 so I would be inclined to take a slightly more subtle approach to this change and hopefully render the comment unnecessary.
|
Yes, if we keep L1 testable is makes sense to split this.
How should we handle components that are not the latest version, but also do not contain known vulnerabilities? The existing requirements require that components are "up to date". I think it's not required to use the latest software, as long as it's possible to patch vulnerabilities in a reasonable time. That means using supported software that receives security updates, or having controls in place to perform those updates yourself. Should we put this in a requirement?
This is not really clear to me. Do you mean that the application needs to be scanned while running? Is this really required? Would scanning the package-lock.json also be acceptable instead?
This seems a bit too vague. Can you give some examples of which risks you are talking about? |
I think "up to date" is vague and not strictly a security requirement so I am happy ignoring this.
I think the details are a little to subtle here although see "dependency risks"
Yes but the point is that at L1 we don't necessarily have access to that file but I could modify the wording.
I will create an updated suggestion
@Sjord what do you think of this update and my responses above? |
Proposed 14.2.1
I would not use phrase "using a runtime or code level dependency scan" here, or if to use it then just as an example but not "the only way to do it". I think ASVS should say what must be achieved as end-results for an application, not that much say how you need to check it. Proposed 14.2.8
Second part of the requirement feels a bit overlapping with other requirements (and maybe those can be merged):
|
Ok so let's add this to list to rework because I think this needs to be considered in the context of the rest of this chapter. |
Do we have agreement, that 1.14.3 can be deleted anyway and the rest of the discussion here is (a bit offtopic) how to improve 14.2.1? |
yeah ok @elarlang |
I want to make one small suggestion on these requirements. I suggest we stop using the term "dependency checker", which is non-standard, and instead use the term I also believe it's important to mention this tool category, since it's the defact way to address third party library security. We already address SBOM's and they are only so useful. What's useful is scanning and updating libraries. |
@jmanico, just to throw a curve ball, sometimes people also confuse SCA with "static code analyzer" 🙃 which makes these acronyms confusing. But I agree with you, we have to use the "standard term" or most common term in our industry. Do we have this somewhere in our glossary of terms? |
Hey @csfreak92 I am with you. SCA used to refer to what we now call SAST. Yea this whole topic of the right acronyms is confusing at best. And yes we have a glossary here. https://github.com/OWASP/ASVS/blob/master/5.0/en/0x90-Appendix-A_Glossary.md |
And @csfreak92 would you mind submitting a PR for SCA? |
No prob, I'll create a PR for it. |
Seems that this issues has some side-jumps, initial topic is solved but there another discussion that is not solved yet: #1405 (comment) + #1405 (comment) ping @tghosth |
OK, I think the big question here is whether 3rd party library risk is definitely in scope for ASVS v5. We generally consider SSDLC activities and CI/CD processes to be out of scope. Should the requirements discussed in this issue therefore be out of scope @elarlang @meghanjacquot ? |
If we are removing SSDLC as part of scope, then I think we should rethink all of 14.2. It seems like that if those are out of scope, analyzing 3rd party risk could potentially be as well therefore making all of 14.2 out of scope. Also, removal of 14.1.1 due to the CI/CD emphasis and 14.1.3 due to 3rd party references. Happy for some other ideas and thoughts @elarlang @tghosth @jmanico |
My only suggestion is that we have one clear requirement for ensuring that we keep libraries up to date from a security perspective. I like the language "Verify that application components do not contain known vulnerabilities" a lot. But I only think we need one (prominent) requirement for this at L1. |
So, sorry for the flip-flopping but I am thinking more about that this. It feels like 3rd party libraries are an inseparable part of writing the application which makes me think that we cannot ignore them entirely. I do however think that maybe we want the 14.2.x requirements to be as simple as possible. We previously considered splitting client-side and server-side dependency requirements because of testability but I no longer think that is worth two requirements. I also think that we should not be too prescriptive about the specific solution. On 14.1.1, I think that is more process than code so I would be incline to drop it, I have opened a separate issue: #2084 I actually think 14.1.3 should stay because 3rd party components might be an inseparable part of the app. |
It is not that much analyzing 3rd party component risk, but using known vulnerability is definitely something to keep and hilight. "Vulnerable and Outdated Components" also climbed to position 6 in the Top 10 project. It does not mean that the section does not have place for improvement, for sure there is, but please no "let's delete all and problem solved". |
Yeah hence my change of mind. Which requirements from 14.2.x do you think should stay/go? |
Can we please focus on one thing in one issue? If the 14.2.1 needs an update, then let's update, but let's chat about everything in the issue is just a nightmare to work through again and again, I waste days worth of hours on this every month. |
Ultimately, I think I no longer agree with the points above about splitting the requirement and I don't agree we should split on testability but rather we should keep it simplified. |
I think @tghosth need to work through the issue again and collect points to discuss, I don't clearly understand, what is the latest direction with 14.2.1 or some added new requirements. |
So I think the original point was that 1.14.3 was a duplicate and that was resolved long ago. I also think the approach I described here and the discussion about testability is overkill for this section. As such, I agree with @meghanjacquot that was can keep the more general 14.2.x discussion in #2088 and close this. |
Related requirements:
CWE:
Proposal:
The text was updated successfully, but these errors were encountered: