relation between ASVS-4.0-7.2.{1,2} and 7.1.{3,4}

[timhemel]

Reading 7.1 and 7.2 it seemed to me that there may be some duplication. Perhaps I have missed something.

It looks to me as if 7.2.1 and 7.2.2 are specific cases of 7.1.3 and 7.1.4 combined, and if so, they would be redundant. The descriptions in the sections don't really help me to understand the difference. One section seems to be about the content, the other about the format. Could someone explain this?

| 7.1.3 | Verify that the application logs security relevant events including successful and failed authentication events, access control failures, deserialization failures and input validation failures. (C5, C7) | | ✓ | ✓ | 778 | | 7.1.4 | Verify that each log event includes necessary information that would allow for a detailed investigation of the timeline when an event happens. (C9) | | ✓ | ✓ | 778 | | 7.2.1 | Verify that all authentication decisions are logged, without storing sensitive session tokens or passwords. This should include requests with relevant metadata needed for security investigations. | | ✓ | ✓ | 778 | | 7.2.2 | Verify that all access control decisions can be logged and all failed decisions are logged. This should include requests with relevant metadata needed for security investigations. | | ✓ | ✓ | 285 |

If I explain logging to developers, I tell them they need to decide which events to log (i.e. all relevant security events, failed and successful) and what data to log for that event (enough for a security investigation, but as few sensitive data as possible). 7.1 nicely summarizes this.

[elarlang]

another one from my todo list :)

My current improvement idea on that field: 7.1 should be clearly "what must every log line contain and what it should not contain", "Log Content" is ok category name here. I'm ok with this part. 7.2 is the confusing one here. It seems to be list of security events. Category name "Log Processing" is confusing here. I think I have discussed it in some issue already, but did not find it at the moment. And my current setup, I agree, both 7.2.1 and 7.2.2 are covered by 7.1.3+7.1.4.

Good material was recently published under CheatSheet project: https://cheatsheetseries.owasp.org/cheatsheets/Application_Logging_Vocabulary_Cheat_Sheet.html

Now it's good question, how to list all expected security events - at the moment there is only 2: authentication and authorization but this gives a false signal - like logging only those is already good enough.

[timhemel]

Sorry, I did not know it was on your list already :) . I tried to my best to search if the issue was reported already but could not find it.

Do I understand correctly that you intend 7.1 to be about the 'what' and 7.2 about the 'when'? Currently both are in 7.1 and that is fine, i don't see any reason to split it, unless you want to be more detailed about the specific events.

It is going to be hard to be complete with respect to expected security events, as some events may be very domain specific. Therefore I would use the system's threat model as inspiration. Good candidates would be:

• any functionality that requires non-repudiation
• any important status of a mitigation in progress, such as the result of a security check or a security decision.

Of course the ASVS could suggest some common ones, like authentication, authorization, validation, DoS-protection, security relevant errors, attack detection. That list in the cheatsheet is a good source of inspiration, it could grow into a standard of its own!

So, I would give the reader the general guidelines and some examples, rather than give the false impression of completeness.

[elarlang]

Sorry, I did not know it was on your list already :) . I tried to my best to search if the issue was reported already but could not find it.

No reason to say sorry. I like that someone else notices this kind stuff as well. And I like your fact-based arguments, keep going!

It is going to be hard to be complete with respect to expected security events, as some events may be very domain spec

One way to go is to have "architecture" requirement like (however to say it in English correctly) "Verify, that all expected security events for logs are documented." Like requirement https://github.com/OWASP/ASVS/issues/892

This could be required input for current 7.1.3. In this case 7.2.* requirements are not necessary, but on the other hand, it's nice to point out "minimum list of events what must be logged".

[drwetter]

Either we take out "including successful and failed authentication events, access control failures," from 7.1.3 and leave this to in 7.2.1 + 7.2.2 . Or better: change the order. To me it is more relevant to log failed login attempts and similar (thus it maybe it better to put important things first but that doesn't seem relevant here). Also please Level 1 for auth events, see #1040.

[elarlang]

Logging and level 1 - by current description (see https://github.com/OWASP/ASVS/blob/master/4.0/en/0x03-Using-ASVS.md), level 1 is not the case for logging related requirements, as those require access to logs.

[drwetter]

Right, forgot that definition (if you mean completely penetration testable and not Level 1 is the bare minimum that all applications should strive for).

OT: It should be either one or the other

[elarlang]

and on this topic there is discussion in https://github.com/OWASP/ASVS/issues/956

[drwetter]

yeah maybe and thx for the hint. But participate in that discussion is too much for me.

My personal stake is that at least ASVS should be more clear and not on one side completely penetration testable and on the other hand not Level 1 is the bare minimum that all applications should strive for . That seems a stretch to me and that needs to be fixed - YMMV

[tghosth]

@elarlang any idea what the next action on this is?

[elarlang]

Starting point here is - how do we cover "all security events what must be logged" without listing them.

[jmanico]

May I suggest we take the OWASP logging vocabulary cheat sheet and build a separate logging standard for logging events?

https://cheatsheetseries.owasp.org/cheatsheets/Logging_Vocabulary_Cheat_Sheet.html

[elarlang]

May I suggest we take the OWASP logging vocabulary cheat sheet and build a separate logging standard for logging events? https://cheatsheetseries.owasp.org/cheatsheets/Logging_Vocabulary_Cheat_Sheet.html

this is something to do, but do not answer to my question at all - how to cover it in ASVS :)

[jmanico]

It does answer your question, let me help.

By suggesting we move the logging requirements to a separate standard, I suggest that we do NOT provide those details in ASVS.

Is that suggestion helpful?

[tghosth]

I think we need to provide high level requirement in ASVS and in the references section link to the relevant cheatsheet. So what is the next action here @elarlang ?

[jmanico]

I'd like to add that when looking at:

7.1.3 and 7.2.1 both (in part) contain the same information.

https://github.com/OWASP/ASVS/blob/master/5.0/en/0x15-V7-Error-Logging.md

[tghosth]

Section 7 needs serious rework deduplication and maybe also ensure that there is no important, high-level content from the cheat sheet which is missing.

@set-reminder 1 week @elarlang to look at this

[octo-reminder[bot]]

⏰ Reminder Monday, December 19, 2022 12:00 AM (GMT+01:00)

@elarlang to look at this

[octo-reminder[bot]]

🔔 @tghosth

@elarlang to look at this

[tghosth]

For full clarity I would add to this:

a) "This should include requests with relevant metadata needed for security investigations." sounds like a different requirement that shouldn't need to appear within 7.2.1 and 7.2.2 but rather should be covered once and separately. b) "Verify that all access control decisions can be logged and all failed decisions are logged" the first half and second half seem to repeat themselves. This seems to be an old requirement from ASVS 2.0 so it is hard to see if there is rationale for this strange wording. I think it needs to be simplified.

[elarlang]

ping @timhemel - I did some reorganizing and deduplications https://github.com/OWASP/ASVS/pull/1794

It all requires more work, but lets start from the first step - do you think it solves the first set of problems addressed when the issue was opened (https://github.com/OWASP/ASVS/issues/997#issue-892711114)?

[elarlang]

The scope for this issue: duplication between requirements 7.1.3 (proposed to move to 7.2.3), 7.1.4, 7.2.1, 7.2.2

For covering different security events, I opened separate issue: https://github.com/OWASP/ASVS/issues/1795 which also should handle potential need to modify or finetune requirements 7.1.3/7.2.3, 7.2.1 and 7.2.2.

[tghosth]

hi @elarlang, in #1794

a) We lose some detail from 7.2.1 and 7.2.2. "This should include requests with relevant metadata needed for security investigations". Are you sure this is covered elsewhere already?

b) Why did you comment out the surrounding text?

[elarlang]

I tried to put self-explaining commit messages: https://github.com/OWASP/ASVS/pull/1794

a) commit https://github.com/OWASP/ASVS/pull/1794/commits/5790450cd3149064988a11c0490fbd21200678c2 says "deduplicate 7.2.1 and 7.2.2 from 7.1.4"

and current 7.1.4 is:

#	Description	L1	L2	L3	CWE
7.1.4	Verify that each log event includes necessary information that would allow for a detailed investigation of the timeline when an event happens. (C9)		✓	✓	778

b) commit https://github.com/OWASP/ASVS/pull/1794/commits/1584d726975fc7c86dc05af04dedc67b0956238a says "hide outdated section descriptions"

Those texts are outdated - if we make changes and want to get feedback, we don't want to get feedback for already known outdated or out of sync intro texts.

[tghosth]

Ah ok, so what do you think about rewording 7.1.4 slightly:

#	Description	L1	L2	L3	CWE
7.1.4	[MODIFIED] Verify that each log entry includes necessary metadata that would allow for a detailed investigation of the timeline when an event happens. (C9)		✓	✓	778

[elarlang]

I don't mind, but I can not see improvement from "information" > "metadata" change. Any reasoning?

[elarlang]

@tghosth - PR updated via https://github.com/elarlang/ASVS/commit/b43f8b75445fc8a0596531905333115505dc7d9c