Skip to content

Commit 4a7c4b8

Browse files
Krishcalinclaude
andcommitted
MITIG-001/002: say what a mitigating control actually suppressed
Scoped as "build the mitigating-control lifecycle - approver, source, mitigated-until, revalidation". docs/SOD_REFERENCE.md section 6 says the useful build is a different one, and it was right: A SAP "mitigating control" is a row; a "compensating control" is an audit conclusion. Nothing makes the former produce the latter. This module behaved as though something did. A conflict every one of whose holders carried a mitigation row returned early and emitted NOTHING - it left the report entirely on the strength of a line in a CSV, and no other output mentioned that it had. Adding fields to the row would have made a richer version of the same silence. Suppression is still honoured, because organisations rely on it. What changed is that it can no longer be silent. MITIG-001 every suppression, with what it is worth. PCAOB AS 2201 .68 requires a compensating control to operate at a level of precision that would prevent or detect a material misstatement; a row asserts nothing about precision, about whether anybody performs the control, or about whether its evidence would survive examination. MITIG-002 rows that cannot support a conclusion at all: a blanket `*` that removes EVERY conflict for a user - the rubber stamp SAP ships as configuration, reproduced in data - a missing approver, a missing control id, and a missing expiry, which means nothing will ever make the row lapse so nobody will ever revisit it. The remediation carries two facts from section 6 that most tooling misses. SAP defaults the Invalid Mitigation Monitors option OFF, so the standard reports show a green control id over a monitor that is expired, deleted or locked. And a control whose only evidence is an SAP report inherits that report's own reliability - an ITGC failure invalidates both at once. Precise rather than noisy on the sample estate: one LOW finding, MITIG-002 silent because those rows are well formed. THREE GUARDS CAUGHT REAL GAPS MITIG-* routed to `unassigned`, so both findings would have appeared on nobody's worklist. That is the second time this month that guard has caught a check going nowhere. Routed to authorizations: the people who own the SoD ruleset own the rows that suppress its output. ECC parity moved 14 -> 13. The ECC fixture and the S/4 sample carry different mitigation rows, so the two systems now receive different - correct - answers about what was hidden from their reports. Recorded in docs/ECC_COVERAGE.md alongside two earlier moves of the same shape: the module left the identical set by saying more. The firing reference needed regenerating for the catalogue moving 791 -> 793. 706 of 793 proven to fire. Full suite green: 4474 passed. Smoke run 410 findings. Eleven new tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 6fc62ad commit 4a7c4b8

10 files changed

Lines changed: 366 additions & 28 deletions

data/finding_details.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2034,6 +2034,16 @@
20342034
"module": "master_data_changes",
20352035
"risk": "Bank-account changes were found in the change documents and no payment-run export was supplied, so the one test that separates a routine master-data update from a payment diversion — did money then leave into the new account — was not run. This is recorded as a gap rather than left silent for a specific reason: nothing here says those changes were not paid, only that nobody looked, and a report that shows bank changes and then says nothing further reads as a report that checked and found them innocuous. Bank-detail changes are the most direct payment-fraud vector in an ERP, so the follow-up question is the one the whole control rests on. Attack scenario is the one MDC-BANK-001 describes: an account changed shortly before a payment run, genuine invoices paid to the attacker, and the real supplier's enquiry arriving weeks later. Whether that has already happened on this estate is exactly what the missing export would settle, and it is a question with an expiry date — recall windows on a misdirected payment are short, so the cost of not looking rises daily."
20362036
},
2037+
"MITIG-001": {
2038+
"mitigation": "1. For each suppressed risk, confirm the control actually operates and that a named person performs it. A row in mitigating_controls.csv is a claim, not evidence, and it is the claim that removed the conflict from this report.\n2. Test precision, which is what AS 2201 .68 actually asks for: would the control prevent or DETECT the specific misuse the conflict permits, at an amount that would matter? A monthly review that samples ten documents does not detect one fraudulent payment.\n3. Check the monitor as well as the control id. SAP's Invalid Mitigation Monitors option is off by default, so the standard reports show a green control while its monitor is expired, deleted or locked.\n4. Where the control's only evidence is an SAP report, record that it inherits that report's own reliability. A control evidenced by the system it compensates for is not independent of it, and an ITGC failure invalidates both at once.\n5. Where a control cannot be evidenced, remove the mitigation rather than leaving the conflict hidden. An unmitigated conflict on the report is worth more to you than a mitigated one nobody can support.",
2039+
"module": "access_risk_analysis",
2040+
"risk": "A mitigating control row removed one or more segregation conflicts from the results in this report, and where every holder of a risk carried such a row the risk does not appear at all. The suppression is honoured because organisations legitimately rely on it, and it is reported because a conflict that leaves a report on the strength of a table row should not leave it silently. The distinction matters more than the vocabulary suggests. In SAP GRC a mitigating control is a row linked to a risk and an assignment; in audit language a compensating control is a conclusion reached after testing. Nothing makes the first produce the second. PCAOB AS 2201 paragraph .68 requires a compensating control to operate at a level of precision that would prevent or detect a misstatement that could be material, and a row asserts nothing about precision, about whether anybody performs the control, or about whether its evidence would survive examination. Two further facts make silent suppression dangerous. SAP ships the rubber stamp as configuration in the form of Approve Despite Risk, and it defaults the Invalid Mitigation Monitors option off, so the standard reports show a green control identifier while the monitor behind it is expired, deleted or locked. A report that hides conflicts on this basis without saying so is asserting an audit conclusion it has not reached."
2041+
},
2042+
"MITIG-002": {
2043+
"mitigation": "1. Replace every blanket entry with per-risk rows. If a user genuinely needs several mitigations that is several rows and several decisions, each of which somebody owns and can be asked about.\n2. Give every row an approver. A mitigation with no approver records no decision, and there is nobody to ask why it exists.\n3. Give every row an expiry, set to the review cycle you actually run. The row then lapses and the conflict returns to the report if the review stops happening, which is the only mechanism that makes revalidation real.\n4. Give every row a control identifier that resolves to a documented control. A row naming no control cannot be tested by anyone.\n5. Remove rows you cannot evidence rather than renewing them. The conflict reappearing on the report is the correct outcome, not a regression.",
2044+
"module": "access_risk_analysis",
2045+
"risk": "These mitigating control rows suppress segregation conflicts while missing what an auditor would need in order to credit them. The blanket entry is the sharpest case: a row that names no specific risk removes every conflict for that user, whatever the conflict is and whenever it arises, which is the rubber stamp SAP ships as configuration reproduced in data. It cannot have been approved on the merits of each risk, because the risks were not enumerated. A row with no approver records no decision, so there is nobody to ask what was considered. A row with no expiry has never been revalidated and never will be, because nothing will make it lapse: it is a permanent exemption described as a control. A row with no control identifier names nothing that could be tested. In each case the effect is the same and it is not neutral - a conflict that would otherwise appear in this report has been removed from it, and the basis for removing it will not withstand being asked about."
2046+
},
20372047
"NET-001": {
20382048
"mitigation": "1. Inventory the exposure: run report RSRFCCHK (or review SM59 and table RFCDES) to list every destination that carries stored logon data, and record the stored user and target system for each.\n2. Prefer trust over stored passwords: where source and target are your own systems, replace the stored credential with a Trusted RFC relationship defined in SMT1 (trusted/trusting systems) so the caller identity flows and the target performs an S_RFCACL check rather than relying on a stored password.\n3. Where trust is not possible, switch the SM59 destination to certificate/SNC single sign-on on the Logon and Security tab instead of user and password.\n4. If a technical user must remain, create a dedicated account in SU01 of type System or Communication (never Dialog), and scope its role tightly, restricting authorization object S_RFC to only the required function groups; never assign SAP_ALL or use DDIC.\n5. Set a long generated password, store it in a secrets vault, and rotate it on a schedule.\n6. Lock down who can maintain destinations: restrict transaction SM59 and object S_RFC_ADM to the Basis team only.\n7. Enforce inbound RFC authorization on targets by setting profile parameter auth/rfc_authority_check to 6 or 9 so the target validates S_RFC for the called function group.\n8. Optionally deploy Unified Connectivity (transaction UCONCOCKPIT) to allow-list which function modules are callable externally.\n9. Verify by re-running RSRFCCHK and confirming the count drops; test each interface in non-production first and coordinate with interface owners before removing stored credentials. SM59 changes take effect immediately, but SNC or parameter changes may require an instance restart.",
20392049
"module": "network_services",

docs/ARCHITECTURE.html

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ <h3 class="ch" id="ch05"><span class="cnum">Chapter 05</span>What MonitorRisk do
689689
<p class="lede">If you read nothing else, read this. Everything after it is elaboration.</p>
690690

691691
<h4>The idea in three sentences</h4>
692-
<p>Somebody takes a set of photographs of how the SAP system is currently configured — lists of users, roles, settings, connections, and so on — and saves them as ordinary files. MonitorRisk reads those files on a computer that has no connection to SAP at all, and applies <strong>791 checks</strong> written by people who know SAP well. It produces a report that says what is wrong, how bad it is, what to do about it, who is able to do it, and what it is worth in money.</p>
692+
<p>Somebody takes a set of photographs of how the SAP system is currently configured — lists of users, roles, settings, connections, and so on — and saves them as ordinary files. MonitorRisk reads those files on a computer that has no connection to SAP at all, and applies <strong>793 checks</strong> written by people who know SAP well. It produces a report that says what is wrong, how bad it is, what to do about it, who is able to do it, and what it is worth in money.</p>
693693

694694
<figure>
695695
<div class="cap"><span class="fid">FIG. 5.1</span><span class="ftx">The whole idea</span></div>
@@ -711,7 +711,7 @@ <h4>The idea in three sentences</h4>
711711

712712
<rect class="box" x="664" y="34" width="250" height="118" rx="4"/>
713713
<text class="sv-h" x="686" y="74">MonitorRisk</text>
714-
<text class="sv-ms" x="686" y="104">791 checks</text>
714+
<text class="sv-ms" x="686" y="104">793 checks</text>
715715
<text class="sv-ms" x="686" y="128">36 modules</text>
716716

717717
<path class="ln" d="M914 93 H 984" marker-end="url(#ar)"/>
@@ -729,7 +729,7 @@ <h4>The seven questions it answers</h4>
729729

730730
<div class="tw"><table>
731731
<tr><th class="k" style="width:4%"></th><th style="width:26%">Question</th><th>How it is answered</th></tr>
732-
<tr><td class="k">1</td><td><strong>What is wrong?</strong></td><td>791 checks across 36 subject areas produce a list of findings, each naming the specific accounts, roles, settings or connections involved.</td></tr>
732+
<tr><td class="k">1</td><td><strong>What is wrong?</strong></td><td>793 checks across 36 subject areas produce a list of findings, each naming the specific accounts, roles, settings or connections involved.</td></tr>
733733
<tr><td class="k">2</td><td><strong>How bad is each one?</strong></td><td>A severity from Critical to Low, assigned by the check itself according to what an attacker could achieve.</td></tr>
734734
<tr><td class="k">3</td><td><strong>What should we do first?</strong></td><td>A priority from P1 to P4 that combines severity with whether the flaw is known to be exploited, how exposed it is, and how much privilege it confers.</td></tr>
735735
<tr><td class="k">4</td><td><strong>What exactly do we do?</strong></td><td>A numbered remediation procedure naming the specific SAP transaction, parameter or table to change, how to verify it worked, and what to be careful of.</td></tr>
@@ -1175,7 +1175,7 @@ <h4>Route C — a source-code export</h4>
11751175
<p>When the customer supplies one, a further module unpacks it and analyses the code itself, applying <strong>136 rules</strong> for the classic weaknesses described in chapter 3, family 6.</p>
11761176

11771177
<h4>Why convergence matters more than it sounds</h4>
1178-
<p>Because all three routes produce the same shape of information, none of the 791 checks contains any logic about provenance. There is no “if this came from the collector, be more careful” branch anywhere. Adding Route B required no changes to the checking logic at all, and required no new tests of the checks, because the checks could not tell the difference.</p>
1178+
<p>Because all three routes produce the same shape of information, none of the 793 checks contains any logic about provenance. There is no “if this came from the collector, be more careful” branch anywhere. Adding Route B required no changes to the checking logic at all, and required no new tests of the checks, because the checks could not tell the difference.</p>
11791179
<p>This is a general principle worth naming, since it recurs later in the book: <strong>push variation to the edges</strong>. Let the outside of the system deal with the messy diversity of the real world, and let the middle see one clean, uniform thing.</p>
11801180

11811181
<div class="call takeaway">
@@ -1374,7 +1374,7 @@ <h4>Line by line</h4>
13741374
</table></div>
13751375

13761376
<h4>Why 685 of these are hard</h4>
1377-
<p>If one check is fifteen lines, 791 checks are not simply nine thousand lines of the same thing. The difficulty is elsewhere:</p>
1377+
<p>If one check is fifteen lines, 793 checks are not simply nine thousand lines of the same thing. The difficulty is elsewhere:</p>
13781378
<ul>
13791379
<li><strong>Knowing what to check.</strong> Most of the value is the accumulated knowledge of which settings matter and why. A cryptic permission with a wildcard value can mean “may impersonate any user from a trusted system”. Knowing that is decades of SAP experience, not programming.</li>
13801380
<li><strong>Knowing when not to fire.</strong> A check that reports something harmless trains the reader to ignore it. The single most common cause of a security tool being abandoned is false alarms.</li>
@@ -1490,7 +1490,7 @@ <h4>What is deliberately not in the record</h4>
14901490
<p>The long explanation of why something is dangerous, and the step-by-step instructions for fixing it, are not stored in the finding. They live in a separate knowledge base, looked up by check identifier when a report is written (chapter <a href="#ch20">20</a>). Two reasons:</p>
14911491
<ul>
14921492
<li>The same finding may appear five hundred times in one scan — once per affected role. Storing a page of narrative five hundred times would be wasteful and would make the record awkward to move around.</li>
1493-
<li>Improving the guidance for 791 checks becomes an edit to a content file rather than a change to thirty programs, which means it can be done by the person who knows SAP best rather than the person who writes code best.</li>
1493+
<li>Improving the guidance for 793 checks becomes an edit to a content file rather than a change to thirty programs, which means it can be done by the person who knows SAP best rather than the person who writes code best.</li>
14941494
</ul>
14951495

14961496
<div class="call takeaway">
@@ -1620,7 +1620,7 @@ <h4>Relationships, not just values</h4>
16201620
<section class="part" id="part5">
16211621
<div class="pnum">Part five</div>
16221622
<h2 class="parth">Making sense of the results</h2>
1623-
<p class="pintro">791 checks against a real estate can produce thousands of findings. An undifferentiated list of thousands of problems is not information; it is a way of guaranteeing that nothing gets fixed. This part is about turning the list into decisions.</p>
1623+
<p class="pintro">793 checks against a real estate can produce thousands of findings. An undifferentiated list of thousands of problems is not information; it is a way of guaranteeing that nothing gets fixed. This part is about turning the list into decisions.</p>
16241624
<ol class="plist">
16251625
<li><b>19</b><a href="#ch19">What to fix first</a></li>
16261626
<li><b>20</b><a href="#ch20">Telling people what to actually do</a></li>
@@ -2583,7 +2583,7 @@ <h3 class="ch" id="appB"><span class="cnum">Appendix B</span>The thirty modules
25832583

25842584
<div class="call amber">
25852585
<span class="k">One honest note about counting</span>
2586-
<p>The headline “791 checks” needs a footnote. <strong>446</strong> check identifiers are written out individually in the source. A further <strong>345</strong> come from <strong>six families generated at run time</strong> from shipped rule lists — one check per Web Dispatcher rule (14), one per technical parameter (79), one per code rule (136), one per duty-separation risk (99), one per imported code family (10), one per conflicting-duty pair (7). Both numbers are true; stating both is more useful than picking whichever is larger.</p>
2586+
<p>The headline “793 checks” needs a footnote. <strong>448</strong> check identifiers are written out individually in the source. A further <strong>345</strong> come from <strong>six families generated at run time</strong> from shipped rule lists — one check per Web Dispatcher rule (14), one per technical parameter (79), one per code rule (136), one per duty-separation risk (99), one per imported code family (10), one per conflicting-duty pair (7). Both numbers are true; stating both is more useful than picking whichever is larger.</p>
25872587
<p>These figures are derived from the code rather than typed here, and the test suite fails if this page and the source ever disagree.</p>
25882588
</div>
25892589

@@ -2714,7 +2714,7 @@ <h4>E.3 — Refusing to print a number</h4>
27142714

27152715
<p style="font-size:15px;color:var(--ink2);max-width:80ch"><strong>Provenance and caveat.</strong> This book was written from the repository’s documentation and its top-level program, at <a href="https://github.com/Krishcalin/SAP-S4HANA-RISE-Security-Scanner">github.com/Krishcalin/SAP-S4HANA-RISE-Security-Scanner</a>. Module names, the command-line surface, the pipeline order, the corpus-split rule, the gate rules and the deployment-mode behaviour are taken from the source and documentation. Descriptions of the internal structure of individual inspectors, the exact shape of a finding record, and the illustrative code in Appendix E are reconstructions written to be faithful in substance rather than literal in syntax; verify against the source before quoting them. All diagrams are architectural rather than exhaustive.</p>
27162716

2717-
<p style="font-size:15px;color:var(--ink2);max-width:80ch">The counts this document states — 791 checks, 446 written as literals, 345 generated at run time from 6 rule families, 38 modules, 136 custom-code rules, 27 duty-separation risks, 78 profile parameters and 135 logical sources — are derived from the code by <code>tests/test_architecture_doc.py</code>, which fails the build if this page and the source disagree. Edition 1.1 corrected three figures that had drifted from the source.</p>
2717+
<p style="font-size:15px;color:var(--ink2);max-width:80ch">The counts this document states — 793 checks, 448 written as literals, 345 generated at run time from 6 rule families, 38 modules, 136 custom-code rules, 27 duty-separation risks, 78 profile parameters and 135 logical sources — are derived from the code by <code>tests/test_architecture_doc.py</code>, which fails the build if this page and the source disagree. Edition 1.1 corrected three figures that had drifted from the source.</p>
27182718

27192719
<p style="font-size:15px;color:var(--ink2)"><strong>Confidential.</strong> This document is not for publication. It describes the internal design, the current limitations and the forward direction of a commercial product; treat it as you would any other confidential product document.</p>
27202720

0 commit comments

Comments
 (0)