diff --git a/CHANGELOG.md b/CHANGELOG.md index 047933d..bcf6c8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -134,6 +134,25 @@ Nothing has been released yet. Everything below is what the tool does today. ### Fixed +- **Reading signatures no longer contacts anybody. It had been doing so since the beginning.** + This tool promises it never talks to the internet, and `bws list --signatures`, + `bws snapshot create` and the window all broke that promise: while checking who signed each + binary, Windows went out and fetched certificates it did not already hold. Measured three + runs out of three, with real connections to a certificate distribution point on the public + internet. It uploaded nothing - no list of your services went anywhere - but saying "nothing + was sent" would be too generous: a connection carries your address, and **which certificate + is asked for says whose software is installed on that machine**. On a tool that runs with + administrator rights on somebody else's server, that is the whole point of the promise. It now + checks signatures against what your machine already has, and `--follow-network` - the switch + that already decided whether a launch path on somebody else's share may be opened - is what + lets it reach out again. On the machine this was found on, all 790 signed entries read + exactly the same either way, and the listing got faster. + + **Nothing is called untrusted because this tool would not look it up.** If a signature cannot + be settled without reaching the network, that entry's signature is reported as unread, with + the system's own number and sentence - never as a verdict about the certificate. A file your + machine can verify on its own is unaffected. `--follow-network` gives the full answer back. + - **A forced stop whose polite step gave up and whose kill worked no longer reports failure.** The sheet said "the entry is not where you asked" and the command line returned exit code 3 over an entry standing exactly where every step wanted it, because the verdict counted steps diff --git a/site/i18n/en.json b/site/i18n/en.json index 4c79123..4cc4297 100644 --- a/site/i18n/en.json +++ b/site/i18n/en.json @@ -44,7 +44,7 @@ "switch.signatures": "Read who signed each binary and whether Windows trusts it. Several seconds over a whole machine, so it is off unless asked - and a query about signatures turns it on by itself.", "switch.memory": "Read what each running entry's process is using. Off by default because it is a measurement rather than a setting: it is different a second later.", "switch.required-by": "Read which entries break if one is stopped, asked of Windows directly rather than worked out from what everything declares. A call per entry, so off unless asked. show and snapshot create read it every time.", - "switch.follow-network": "Let the tool look at a launch path that lives on another machine. Off by default for safety: one unreachable share costs twenty one seconds, and the connection authenticates as whoever ran it.", + "switch.follow-network": "Let the tool reach off this machine at all - a launch path living on somebody else's share, and a certificate this machine does not already hold. Off by default for safety: one unreachable share costs twenty one seconds and authenticates as whoever ran it, and fetching a certificate is a connection to a third party in the middle of an ordinary listing. Without it a signature is checked against what is here, and a result that might have been caused by not looking is reported as unread rather than as a verdict.", "switch.force": "On kill, end the process straight away without asking politely - the preview then shows one step instead of two. On snapshot create, write over a file that is already there.", "switch.restart": "On kill, bring the entry back once the process is gone, along with everything that shared it.", "switch.full": "On show, print the fields that are genuinely empty as well. A field nobody could read is printed either way.", diff --git a/site/i18n/pl.json b/site/i18n/pl.json index 78c2909..7e2cb2a 100644 --- a/site/i18n/pl.json +++ b/site/i18n/pl.json @@ -44,7 +44,7 @@ "switch.signatures": "Czyta, kto podpisał każdy plik i czy Windows temu podpisowi ufa. Kilka sekund na całej maszynie, więc jest wyłączone, dopóki nie poprosisz - a zapytanie o podpisy włącza to samo.", "switch.memory": "Czyta, ile pamięci zajmuje proces każdego działającego wpisu. Domyślnie wyłączone, bo to pomiar, a nie ustawienie: sekundę później jest inny.", "switch.required-by": "Czyta, które wpisy przestaną działać po zatrzymaniu tego - pytając Windows wprost, zamiast wyliczać to z deklaracji. Jedno wywołanie na wpis, więc wyłączone, dopóki nie poprosisz. show i snapshot create czytają to zawsze.", - "switch.follow-network": "Pozwala zajrzeć pod ścieżkę uruchomienia leżącą na innej maszynie. Domyślnie wyłączone dla bezpieczeństwa: jeden nieosiągalny udział kosztuje dwadzieścia jeden sekund, a połączenie uwierzytelnia się jako ten, kto uruchomił program.", + "switch.follow-network": "Pozwala programowi sięgnąć poza tę maszynę - pod ścieżkę uruchomienia leżącą na udziale innego komputera oraz po certyfikat, którego ta maszyna jeszcze nie ma. Domyślnie wyłączone dla bezpieczeństwa: jeden nieosiągalny udział kosztuje dwadzieścia jeden sekund i uwierzytelnia się jako ten, kto uruchomił program, a pobranie certyfikatu to połączenie do obcego serwera w trakcie zwykłego listowania. Bez tego podpis jest sprawdzany wobec tego, co jest na miejscu, a wynik, który mógł wziąć się z niezajrzenia, jest zgłaszany jako nieodczytany, a nie jako werdykt.", "switch.force": "Przy kill kończy proces od razu, bez uprzejmego pytania - podgląd pokazuje wtedy jeden krok zamiast dwóch. Przy snapshot create nadpisuje istniejący plik.", "switch.restart": "Przy kill podnosi wpis z powrotem, gdy proces zniknie, razem ze wszystkim, co ten proces dzieliło.", "switch.full": "Przy show drukuje także pola, które są naprawdę puste. Pole, którego nie dało się odczytać, drukuje się i tak.", diff --git a/src/Bws.Cli/Resources/cli.en.json b/src/Bws.Cli/Resources/cli.en.json index 6f19219..8b67546 100644 --- a/src/Bws.Cli/Resources/cli.en.json +++ b/src/Bws.Cli/Resources/cli.en.json @@ -10,7 +10,7 @@ "cli.unknownCommand": "There is no command {0}. There is: {1}.", "cli.unknownCommandDidYouMean": "There is no command {0}. Did you mean {1}?", - "cli.usage": "Examples:\n bws list --query \"start:auto !status:running\" what should be up and is not\n bws show Spooler everything known about one entry\n bws stop Spooler --dry-run --dependents what stopping it would take down\n bws start-type Spooler manual --dry-run what taking it off automatic would do\n bws kill Spooler --dry-run what ending its process would take with it\n bws snapshot create before.json freeze the machine before a change\n\nUsage:\n bws [-h|--help] [--version]\n bws list [--query TEXT] [--signatures] [--memory] [--required-by] [--follow-network]\n [--json] [--timing]\n bws show NAME [--full] [--follow-network] [--json] [--timing]\n bws stop|start|restart NAME [--dry-run] [--dependents] [--timeout SECONDS] [--json] [--timing]\n bws kill NAME [--force] [--restart] [--dry-run] [--dependents] [--timeout SECONDS] [--json] [--timing]\n bws start-type NAME automatic|manual|disabled [--dry-run] [--json] [--timing]\n bws snapshot create [FILE] [--note TEXT] [--follow-network] [--force] [--json] [--timing]\n bws snapshot diff EARLIER LATER [--exit-code] [--json] [--timing]\n bws snapshot diff EARLIER --live [--exit-code] [--json] [--timing]\n\n --signatures reads who signed each binary. Several seconds, so it is off unless asked.\n --memory reads what each running entry's process is using. Fast, and off by default\n because it is a measurement rather than a setting: it is different a second later.\n A query about either turns that one on by itself.\n --required-by reads which entries break if one is stopped. Windows is asked directly\n rather than the answer being worked out from what everything declares, so entries grouped\n by a load order name are counted too. It costs a call per entry, so it is off unless asked.\n show and snapshot create read it every time, without a switch.\n --follow-network lets the tool look at a launch path that lives on another machine.\n Off by default, and that default is about safety rather than speed: one unreachable\n share costs twenty one seconds, and the connection authenticates as whoever ran it.\n Without it the disk question for such an entry is reported as not read, never as\n missing, and the path itself is still shown.\n --timeout is how long the tool waits for one step to reach the state it asked for,\n counted from the moment the manager accepts the request. Sixty seconds unless you say\n otherwise. Running out of it is not a failure, it is the end of watching: the report\n says where the entry was left, and an entry left stopping usually arrives by itself.\n It is not a cap on how long the command takes. The manager answers in its own time,\n and for a service that never reports itself that answer takes tens of seconds - the\n report says so when it happens.\n\n kill is for a service that will not stop. It asks politely first and ends the process\n behind the entry only if that does not work, so an entry which stops on its own is never\n ended - the preview shows both steps and the second one says it is conditional.\n Ending a process is the one thing this tool does that nobody can refuse on the machine's\n behalf, and it takes every other service living in that process with it whether or not\n they stopped first. The preview names them, and names the process by number.\n --force skips asking politely and ends the process straight away. It changes the plan\n rather than the running of it, so the preview shows one step instead of several and the\n difference is visible before anything happens.\n --restart brings the entry back once the process is gone, along with anything that shared\n it. Without it the machine is left with those services stopped, and the report says how to\n start them again.\n Windows spells this idea --force on Stop-Service and means something else by it - there it\n means \"even if something depends on it\", which is what --dependents does here. That is why\n this is a verb of its own rather than a switch on stop.\n\n show prints everything this tool knows about one entry, including the parts a listing\n leaves out unless asked: the signature, the privileges, the security descriptor and the\n memory. It reads all of them every time, because over one entry that costs about sixty\n milliseconds where over the whole machine it costs a second.\n Fields that are genuinely empty are left out. --full prints those as well. A field\n nobody could read is printed either way, because leaving one out would look like an\n answer.\n --json gives the same document bws list --json gives for that entry, on its own rather\n than inside an array of one.\n\n start-type says what the manager will do with an entry at the next boot. It changes a\n setting and moves nothing: an entry that is running keeps running, and one that is\n stopped stays stopped. Disabled is the one worth pausing over, because it stops the\n manager starting the entry at all - including on demand, for something else that needs\n it. Neither --dependents nor --timeout applies here: nothing comes down with a setting\n and there is no state to wait for.\n An automatic entry can also be marked to start late, and this tool has no word for that\n state: automatic here does not say whether the entry starts at boot or after it. So\n after changing the start type of such an entry, the report offers no way back rather\n than a line whose effect depends on a flag it does not name.\n\n snapshot create always reads signatures and hashes, because a snapshot is kept and\n compared later, and one without them would compare against one with them as though the\n machine had changed. Without a file name it writes into the current directory.\n A snapshot describes the whole machine and is worth filing accordingly: every launch\n path and file hash, the account each entry runs as, its privileges and its security\n descriptor, and the name of this machine and of the account that took it. It is written\n with whatever permissions its directory already gives it, and this tool narrows nothing -\n so a directory other people can read is one they can read all of that in.\n --note says what the snapshot was taken for.\n --force here writes over a file that is already there. Without it, an existing file is left\n alone and the command ends without writing - a snapshot is kept for months, and this\n will not replace one, or anything else, unless told to.\n\n snapshot diff says what changed going from the first file to the second. Configuration\n differences and running state are reported apart, because two snapshots taken a day\n apart differ in what was running and almost none of it is drift.\n --exit-code ends with code 5 when anything differs. Off by default, so a script that\n only wants the differences printed is not tripped by finding some.\n --live compares the file against this machine as it is now. It reads signatures and\n hashes, like snapshot create, because the file it is compared against has them.\n\n Every command, switch and exit code, laid out to be read: https://betterwindowsservices.donislawdev.com/cli-reference/", + "cli.usage": "Examples:\n bws list --query \"start:auto !status:running\" what should be up and is not\n bws show Spooler everything known about one entry\n bws stop Spooler --dry-run --dependents what stopping it would take down\n bws start-type Spooler manual --dry-run what taking it off automatic would do\n bws kill Spooler --dry-run what ending its process would take with it\n bws snapshot create before.json freeze the machine before a change\n\nUsage:\n bws [-h|--help] [--version]\n bws list [--query TEXT] [--signatures] [--memory] [--required-by] [--follow-network]\n [--json] [--timing]\n bws show NAME [--full] [--follow-network] [--json] [--timing]\n bws stop|start|restart NAME [--dry-run] [--dependents] [--timeout SECONDS] [--json] [--timing]\n bws kill NAME [--force] [--restart] [--dry-run] [--dependents] [--timeout SECONDS] [--json] [--timing]\n bws start-type NAME automatic|manual|disabled [--dry-run] [--json] [--timing]\n bws snapshot create [FILE] [--note TEXT] [--follow-network] [--force] [--json] [--timing]\n bws snapshot diff EARLIER LATER [--exit-code] [--json] [--timing]\n bws snapshot diff EARLIER --live [--exit-code] [--json] [--timing]\n\n --signatures reads who signed each binary. Several seconds, so it is off unless asked.\n --memory reads what each running entry's process is using. Fast, and off by default\n because it is a measurement rather than a setting: it is different a second later.\n A query about either turns that one on by itself.\n --required-by reads which entries break if one is stopped. Windows is asked directly\n rather than the answer being worked out from what everything declares, so entries grouped\n by a load order name are counted too. It costs a call per entry, so it is off unless asked.\n show and snapshot create read it every time, without a switch.\n --follow-network lets the tool reach off this machine at all. Two things need that,\n and both are off unless you ask.\n One is a launch path that lives on somebody else's share. One unreachable share costs\n twenty one seconds, and the connection authenticates as whoever ran it. Without the\n switch the disk question for such an entry is reported as not read, never as missing,\n and the path itself is still shown.\n The other is checking a signature whose certificate chain this machine does not already\n hold: left to itself Windows goes and fetches the missing certificate, which is a\n connection to a third party in the middle of an ordinary listing. Without the switch\n the check uses only what is here, and any result that might have been caused by not\n looking is reported as unread rather than as a verdict about the certificate - so this\n never calls a certificate bad on the strength of not having looked it up. A file this\n machine can verify on its own reads the same either way.\n --timeout is how long the tool waits for one step to reach the state it asked for,\n counted from the moment the manager accepts the request. Sixty seconds unless you say\n otherwise. Running out of it is not a failure, it is the end of watching: the report\n says where the entry was left, and an entry left stopping usually arrives by itself.\n It is not a cap on how long the command takes. The manager answers in its own time,\n and for a service that never reports itself that answer takes tens of seconds - the\n report says so when it happens.\n\n kill is for a service that will not stop. It asks politely first and ends the process\n behind the entry only if that does not work, so an entry which stops on its own is never\n ended - the preview shows both steps and the second one says it is conditional.\n Ending a process is the one thing this tool does that nobody can refuse on the machine's\n behalf, and it takes every other service living in that process with it whether or not\n they stopped first. The preview names them, and names the process by number.\n --force skips asking politely and ends the process straight away. It changes the plan\n rather than the running of it, so the preview shows one step instead of several and the\n difference is visible before anything happens.\n --restart brings the entry back once the process is gone, along with anything that shared\n it. Without it the machine is left with those services stopped, and the report says how to\n start them again.\n Windows spells this idea --force on Stop-Service and means something else by it - there it\n means \"even if something depends on it\", which is what --dependents does here. That is why\n this is a verb of its own rather than a switch on stop.\n\n show prints everything this tool knows about one entry, including the parts a listing\n leaves out unless asked: the signature, the privileges, the security descriptor and the\n memory. It reads all of them every time, because over one entry that costs about sixty\n milliseconds where over the whole machine it costs a second.\n Fields that are genuinely empty are left out. --full prints those as well. A field\n nobody could read is printed either way, because leaving one out would look like an\n answer.\n --json gives the same document bws list --json gives for that entry, on its own rather\n than inside an array of one.\n\n start-type says what the manager will do with an entry at the next boot. It changes a\n setting and moves nothing: an entry that is running keeps running, and one that is\n stopped stays stopped. Disabled is the one worth pausing over, because it stops the\n manager starting the entry at all - including on demand, for something else that needs\n it. Neither --dependents nor --timeout applies here: nothing comes down with a setting\n and there is no state to wait for.\n An automatic entry can also be marked to start late, and this tool has no word for that\n state: automatic here does not say whether the entry starts at boot or after it. So\n after changing the start type of such an entry, the report offers no way back rather\n than a line whose effect depends on a flag it does not name.\n\n snapshot create always reads signatures and hashes, because a snapshot is kept and\n compared later, and one without them would compare against one with them as though the\n machine had changed. Without a file name it writes into the current directory.\n A snapshot describes the whole machine and is worth filing accordingly: every launch\n path and file hash, the account each entry runs as, its privileges and its security\n descriptor, and the name of this machine and of the account that took it. It is written\n with whatever permissions its directory already gives it, and this tool narrows nothing -\n so a directory other people can read is one they can read all of that in.\n --note says what the snapshot was taken for.\n --force here writes over a file that is already there. Without it, an existing file is left\n alone and the command ends without writing - a snapshot is kept for months, and this\n will not replace one, or anything else, unless told to.\n\n snapshot diff says what changed going from the first file to the second. Configuration\n differences and running state are reported apart, because two snapshots taken a day\n apart differ in what was running and almost none of it is drift.\n --exit-code ends with code 5 when anything differs. Off by default, so a script that\n only wants the differences printed is not tripped by finding some.\n --live compares the file against this machine as it is now. It reads signatures and\n hashes, like snapshot create, because the file it is compared against has them.\n\n Every command, switch and exit code, laid out to be read: https://betterwindowsservices.donislawdev.com/cli-reference/", "cli.unknownOption": "Unknown option: {0}", "cli.wordsNotTaken": "{0} takes {1}. Nothing here can use: {2}.", "cli.takes.oneName": "one name", diff --git a/src/Bws.Core/NetworkPath.cs b/src/Bws.Core/NetworkPath.cs index f51324b..cc314e1 100644 --- a/src/Bws.Core/NetworkPath.cs +++ b/src/Bws.Core/NetworkPath.cs @@ -1,7 +1,16 @@ namespace Bws.Core; /// -/// Whether this tool may look at a file that lives on another machine. +/// Whether this tool may reach off this machine at all. +/// +/// THE NAME OF THIS TYPE IS NARROWER THAN WHAT IT NOW DECIDES, AND THAT IS SAID HERE RATHER +/// THAN QUIETLY LIVED WITH. Since 2026-09-22 it also decides whether Windows may go and +/// fetch a certificate while verifying a signature - measured that day, reading signatures +/// opened HTTP connections to certificates.intel.com on a listing nobody had told to use the +/// network. Both are the same decision for whoever runs this: one promise, one control, and +/// a second switch would have meant finding two of them before the tool was actually quiet. +/// Renaming the type is a sweep of its own and is a backlog row rather than a change made on +/// the way past. /// /// It exists because of a measurement rather than a principle. A service can register a /// launch path on a share, and asking whether that file is there is an ordinary diff --git a/src/Bws.Core/WindowsBinaryInspector.Verdicts.cs b/src/Bws.Core/WindowsBinaryInspector.Verdicts.cs new file mode 100644 index 0000000..9efa437 --- /dev/null +++ b/src/Bws.Core/WindowsBinaryInspector.Verdicts.cs @@ -0,0 +1,118 @@ +using System.Runtime.InteropServices; + +namespace Bws.Core; + +/// +/// The half of that decides what an answer MEANS, kept +/// apart from the half that goes and gets one. +/// +/// The seam is not the line count that forced it. Everything in the other file is the +/// interop dance - a fixed buffer, a handle, a call, a second call to let go of what the first +/// one left behind. Everything here is policy, and policy is what the tests point at: whether +/// a number becomes a verdict or an admission that there is none, and which of those survives +/// a run that was told not to touch the network. Those are the questions somebody reads this +/// file to answer, and none of them needs a single line of the marshalling next door. +/// +public sealed partial class WindowsBinaryInspector +{ + /// + /// The system's number, given a name. + /// + /// Only the results that have a distinct meaning for somebody looking at a service list + /// are named. Everything else stays and keeps its + /// number, because a value folded into a near-enough neighbour is worse than one that + /// admits it has no name: the trigger kinds already taught that the unnamed case can + /// turn out to be the second most common one on the machine. + /// + /// Only Trusted and NotSigned have been observed on a real machine. The rest are mapped + /// from documented results and are NOT OBSERVED. + /// + private static SignatureStatus Classify(int result) => result switch + { + 0 => SignatureStatus.Trusted, + NoSignature => SignatureStatus.NotSigned, + unchecked((int)0x800B0109) => SignatureStatus.UntrustedRoot, + unchecked((int)0x800B0101) => SignatureStatus.Expired, + unchecked((int)0x800B010C) => SignatureStatus.Revoked, + unchecked((int)0x80096010) => SignatureStatus.Tampered, + _ => SignatureStatus.Unknown + }; + + /// + /// A verification result, turned into an answer or into an admission that there is none. + /// + /// THIS EXISTS BECAUSE THE CHEAP FIX FOR THE FETCH WOULD HAVE BOUGHT SILENCE WITH A + /// LIE, AND THE OWNER SAW IT BEFORE IT WAS WRITTEN. Once the chain engine may not go + /// and get a certificate it does not hold, a file that IS properly signed can come back + /// with a non-zero result - and would then turn our own refusal + /// into a sentence about somebody's certificate. A tool whose subject is trust must not + /// say "this root is not trusted" when what happened is "I would not look it up". + /// + /// The direction that CANNOT happen, and it is worth stating because it is the half + /// that would be unforgivable: is produced only + /// by a result of zero, and zero requires a chain that was built and validated. Refusing + /// a fetch can prevent a zero. It cannot manufacture one. So nothing here can turn a file + /// the system distrusts into one it trusts - the risk runs the other way only. + /// + /// WHICH CODE WINDOWS ACTUALLY RETURNS IN THAT CASE IS NOT SPRAWDZONE AND THIS DESIGN + /// DELIBERATELY DOES NOT DEPEND ON IT. The documentation separates CERT_E_CHAINING + /// (0x800B010A, no chain could be built) from CERT_E_UNTRUSTEDROOT (0x800B0109, a chain + /// was built and ends somewhere untrusted), and on that reading only the first would + /// arrive from a refused fetch. It could not be confirmed here: with both caches cleared, + /// not one of the 790 signed files on this machine needed a fetch to reach its verdict, + /// so the failing case does not exist to be observed. Resting on the documented split + /// would be a bet on behaviour nobody in this project has seen, which is exactly the + /// shape rule 9 warns about. + /// + /// So the line is drawn by a rule instead: a verdict survives the quiet mode only if + /// it is a fact about the FILE or about its own certificate, never about the chain to a + /// root. Trusted survives because it needs full validation. NotSigned, Tampered and + /// Expired survive because a missing signature, a hash mismatch and a date are all + /// readable without leaving the machine. Everything else - UntrustedRoot, anything + /// Unknown - becomes a refusal carrying the system's own number and sentence, which is + /// the shape binaryOnDisk already takes for a path on somebody else's share. + /// + /// The cost, said rather than buried: in the quiet mode this gives up the ability + /// to report a genuinely untrusted root as one. That is a lost signal, and a lost signal + /// wearing a label is a different thing from a false accusation - the number and the + /// system's sentence both travel, and --follow-network turns the full answer back + /// on. On this machine the cost is zero entries out of 790. + /// + internal static Reading Settle( + int result, NetworkPaths networkPaths, Func publisher) + { + if (networkPaths == NetworkPaths.Skip && !SurvivesWithoutTheNetwork(result)) + { + // The publisher is deliberately not read. It would open the file a second time to + // decorate an answer we are declining to give, and a refusal carries no value. + // + // THE FALLBACK IS NOT DECORATION. Marshal.GetExceptionForHR answers null for any + // non-negative HRESULT - S_FALSE is 1 and reaches this line - and the first version + // of this method handed an empty string to a refusal when that happened. A refusal + // with no sentence is the thing rule 8 forbids wearing the shape of the thing it + // requires. ManagerTerms.Describe is what every other refusal in this project uses + // and it always says something. + return Reading.Denied( + result, + Marshal.GetExceptionForHR(result)?.Message ?? ManagerTerms.Describe(result)); + } + + return Reading.Present(new BinarySignature(Classify(result), result, publisher())); + } + + /// + /// Whether this result would have been the same had the machine been unplugged. + /// + /// Revoked is absent on purpose rather than by oversight: revocation checking is off two + /// fields above, so that code cannot arrive, and listing it here would suggest somebody + /// had thought about how it behaves offline when there is nothing to think about. + /// + private static bool SurvivesWithoutTheNetwork(int result) => result switch + { + 0 => true, + NoSignature => true, + unchecked((int)0x800B0101) => true, + unchecked((int)0x80096010) => true, + _ => false + }; +} diff --git a/src/Bws.Core/WindowsBinaryInspector.cs b/src/Bws.Core/WindowsBinaryInspector.cs index 8c00938..3c699b6 100644 --- a/src/Bws.Core/WindowsBinaryInspector.cs +++ b/src/Bws.Core/WindowsBinaryInspector.cs @@ -118,8 +118,7 @@ public Reading ReadSignature(string file) { // Read rather than remembered. This file is asked about once in a run, so a // cache would be a way to be wrong later and never a way to be quicker. - return Reading.Present( - new BinarySignature(Classify(embedded), embedded, ReadPublisher(file))); + return Settle(embedded, networkPaths, () => ReadPublisher(file)); } return ThroughCatalogue(file); @@ -226,7 +225,7 @@ public Reading ReadHash(string file) /// second leaks for the life of the process, which on a run touching several hundred /// files is not a rounding error. /// - private static unsafe int Verify(string file) + private unsafe int Verify(string file) { fixed (char* path = file) { @@ -379,8 +378,7 @@ private unsafe Reading VerifyAgainst( // The signer of a catalogue-signed file is whoever signed the catalogue. That // is the same answer Explorer gives, and reading it from the catalogue file // keeps four more functions out of the interop list. - return Reading.Present( - new BinarySignature(Classify(result), result, CataloguePublisher(cataloguePath))); + return Settle(result, networkPaths, () => CataloguePublisher(cataloguePath)); } } finally @@ -393,7 +391,7 @@ private unsafe Reading VerifyAgainst( } /// The parts of the request that never differ, whichever way the file is signed. - private static unsafe WINTRUST_DATA Request(WINTRUST_DATA_UNION_CHOICE choice) => new() + private unsafe WINTRUST_DATA Request(WINTRUST_DATA_UNION_CHOICE choice) => new() { cbStruct = (uint)sizeof(WINTRUST_DATA), @@ -403,11 +401,33 @@ private unsafe Reading VerifyAgainst( // Revocation checking is deliberately off. It reaches the network, which ADR-19 // forbids outright, and it would make the answer depend on whether a certificate // authority happens to be reachable from this machine right now. + // + // THAT SENTENCE WAS TRUE AND INCOMPLETE FOR THIRTEEN MONTHS, AND THE LINE BELOW IS + // WHAT IT WAS MISSING. Turning revocation off does not stop the chain engine going + // out to FETCH a certificate it does not hold. Measured 2026-09-22, three runs out of + // three, by tools/outbound-probe/outbound.ps1: `bws list --signatures` loads + // WINHTTP.dll, WS2_32.dll and DNSAPI.dll and opens HTTP connections to + // certificates.intel.com. Plain `bws list` does none of it, so signature reading is + // the whole of the difference. THREE GUARDS AND EVERY REVIEW OF THIS FILE HAD PASSED + // OVER IT, because none of them can see a module the chain engine loads at run time. fdwRevocationChecks = WINTRUST_DATA_REVOCATION_CHECKS.WTD_REVOKE_NONE, dwUnionChoice = choice, dwStateAction = WINTRUST_DATA_STATE_ACTION.WTD_STATEACTION_VERIFY, - dwProvFlags = WINTRUST_DATA_PROVIDER_FLAGS.WTD_SAFER_FLAG + + // WTD_CACHE_ONLY_URL_RETRIEVAL confines the chain engine to what this machine already + // holds. It is on unless the caller has asked for the network, which is the same + // switch that decides whether a launch path on somebody else's share may be opened - + // one promise, one control, and nobody gets more network than they had before. + // + // Measured on this machine with the certificate URL cache and the DNS cache both + // cleared: 797 entries, 790 Trusted, 3 NotSigned and 790 publishers WITH the flag and + // WITHOUT it, identical. So the fetch that was happening changed no answer here. It + // is still a fetch, and it still went to a third party. + dwProvFlags = networkPaths == NetworkPaths.Follow + ? WINTRUST_DATA_PROVIDER_FLAGS.WTD_SAFER_FLAG + : WINTRUST_DATA_PROVIDER_FLAGS.WTD_SAFER_FLAG + | WINTRUST_DATA_PROVIDER_FLAGS.WTD_CACHE_ONLY_URL_RETRIEVAL }; private static unsafe int Ask(ref WINTRUST_DATA data) @@ -424,27 +444,4 @@ private static unsafe int Ask(ref WINTRUST_DATA data) return result; } } - - /// - /// The system's number, given a name. - /// - /// Only the results that have a distinct meaning for somebody looking at a service list - /// are named. Everything else stays and keeps its - /// number, because a value folded into a near-enough neighbour is worse than one that - /// admits it has no name: the trigger kinds already taught that the unnamed case can - /// turn out to be the second most common one on the machine. - /// - /// Only Trusted and NotSigned have been observed on a real machine. The rest are mapped - /// from documented results and are NOT OBSERVED. - /// - private static SignatureStatus Classify(int result) => result switch - { - 0 => SignatureStatus.Trusted, - NoSignature => SignatureStatus.NotSigned, - unchecked((int)0x800B0109) => SignatureStatus.UntrustedRoot, - unchecked((int)0x800B0101) => SignatureStatus.Expired, - unchecked((int)0x800B010C) => SignatureStatus.Revoked, - unchecked((int)0x80096010) => SignatureStatus.Tampered, - _ => SignatureStatus.Unknown - }; } diff --git a/tests/Bws.Architecture.Tests/AssemblyFacts.cs b/tests/Bws.Architecture.Tests/AssemblyFacts.cs index 086f8ec..d0cef88 100644 --- a/tests/Bws.Architecture.Tests/AssemblyFacts.cs +++ b/tests/Bws.Architecture.Tests/AssemblyFacts.cs @@ -6,13 +6,22 @@ namespace Bws.Architecture.Tests; /// /// Reads what an assembly refers to, straight from its metadata. /// -/// Two different questions need two different answers: +/// Three different questions need three different answers: /// - assembly references say which other assemblies this one links against, -/// - type references say which types it actually names. +/// - type references say which types it actually names, +/// - native modules say which operating system libraries its P/Invokes bind. /// /// Only the second one catches a call into something that lives in the base class /// library. System.Console is the case that matters here: every assembly links /// against the runtime anyway, so an assembly-level check would never see it. +/// +/// And only the third catches a way out of this machine that is not managed at all. +/// Added 2026-09-22 for : a call to WinHTTP through the Win32 +/// generator produces a binding to winhttp.dll and a type reference to Windows.Win32.PInvoke, +/// so a check reading the first two sees an ordinary native call and says nothing. It lives +/// here rather than in a reader of its own for the reason Sources.cs gives about the +/// file list it holds - a second copy of "open the assembly and read its metadata" is a second +/// chance for one of them to quietly stop reading the right thing. /// internal sealed class AssemblyFacts { @@ -20,12 +29,14 @@ private AssemblyFacts( string path, string name, IReadOnlySet assemblyReferences, - IReadOnlySet typeReferences) + IReadOnlySet typeReferences, + IReadOnlySet nativeModules) { Path = path; Name = name; AssemblyReferences = assemblyReferences; TypeReferences = typeReferences; + NativeModules = nativeModules; } internal string Path { get; } @@ -44,6 +55,18 @@ private AssemblyFacts( /// Fully qualified names, such as "System.Console". internal IReadOnlySet TypeReferences { get; } + /// + /// The operating system libraries this assembly's P/Invokes bind, as the linker wrote + /// them - "ADVAPI32.dll", "dwmapi.dll". Compared without case, because the case comes from + /// whoever declared the import and differs between our generator and other people's code. + /// + /// What this cannot see, named rather than left to be assumed covered: a module + /// loaded by name at run time, through NativeLibrary.Load or LoadLibrary. That is a string, + /// and no reader of metadata will ever see it. tools/outbound-probe/outbound.ps1 is + /// what answers that question, by watching what the running process actually loaded. + /// + internal IReadOnlySet NativeModules { get; } + internal static AssemblyFacts Read(string path) { using var file = File.OpenRead(path); @@ -68,7 +91,56 @@ internal static AssemblyFacts Read(string path) types.Add(string.IsNullOrEmpty(declaringNamespace) ? name : $"{declaringNamespace}.{name}"); } - return new AssemblyFacts(path, ownName, assemblies, types); + // Walked through the method table rather than through the ModuleRef table, and the + // reason is availability rather than preference: MetadataReader does not expose a row + // count for that table to anything outside itself in this runtime. Measured 2026-09-22 + // to give the same answer on all four assemblies that were compared - what it reports + // is every module a P/Invoke actually binds, which is the narrower and more honest of + // the two questions anyway. + var modules = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var handle in metadata.MethodDefinitions) + { + var import = metadata.GetMethodDefinition(handle).GetImport(); + + if (!import.Module.IsNil) + { + modules.Add(metadata.GetString(metadata.GetModuleReference(import.Module).Name)); + } + } + + return new AssemblyFacts(path, ownName, assemblies, types, modules); + } + + /// + /// The same read, for a file that might not be a managed assembly at all. + /// + /// It exists because one guard walks a directory rather than naming a file. A build + /// output folder holds whatever the restore put there, and asking a native library for its + /// managed metadata throws. Returning false is the answer for "this is not the kind of file + /// I read" - it is never used to swallow a failure on a file that IS one, because such a + /// file failing to open is a broken build and should say so. + /// + internal static bool TryRead(string path, out AssemblyFacts facts) + { + facts = null!; + + using var file = File.OpenRead(path); + using var portableExecutable = new PEReader(file); + + if (!portableExecutable.HasMetadata) + { + return false; + } + + var metadata = portableExecutable.GetMetadataReader(); + + if (!metadata.IsAssembly) + { + return false; + } + + facts = Read(path); + return true; } internal static AssemblyFacts Of(string projectName) => Read(GuardedAssemblies.PathOf(projectName)); diff --git a/tests/Bws.Architecture.Tests/OutboundGuards.cs b/tests/Bws.Architecture.Tests/OutboundGuards.cs new file mode 100644 index 0000000..5e82a48 --- /dev/null +++ b/tests/Bws.Architecture.Tests/OutboundGuards.cs @@ -0,0 +1,401 @@ +// Explicit for the same reason SourceTree.cs says so at the top of itself: these guards read +// files off disk, and the implicit using set is not something to depend on across projects. +using System.IO; + +namespace Bws.Architecture.Tests; + +/// +/// The three ways out of this machine that cannot see. +/// +/// What was already held, so that this class is read as the addition it is. +/// LayeringGuards.No_shipped_assembly_reaches_for_the_network refuses any reference to +/// System.Net.* in the three assemblies that ship, read out of their compiled metadata +/// rather than out of their source. That covers the whole managed surface - an +/// HttpClient, a TcpClient, a name lookup - and it covers it in the one place +/// that cannot be argued with by formatting or by a comment. +/// +/// Three ways past it, and all three were measured on 2026-09-22 rather than imagined. +/// +/// 1. A P/Invoke. Adding WinHttpOpen to NativeMethods.txt and calling it emits a +/// binding to winhttp.dll and a type reference to Windows.Win32.PInvoke. +/// Not one character of System.Net appears, and that guard stays green. So does a +/// hand-written [DllImport("ws2_32.dll")] that never touches the inventory file at +/// all. +/// 2. Somebody else's assembly. Four managed libraries ship beside the window and the +/// terminal, and the guard above reads none of them. A package is precisely how a network +/// client arrives without us naming one - the guard says so about itself, in its own +/// comment, and calls itself PARTIAL for exactly this reason. +/// 3. A fourth project. The list of assemblies those guards read is written out by +/// hand in . It agrees with src today. A +/// project added to src tomorrow is read by nothing and reddens nothing. +/// +/// WHAT THIS IS NOT, AND THE PROJECT'S OWN HISTORY IS THE ARGUMENT. ADR-19 has been +/// broken twice here and NEITHER time by anything resembling a network client: once by +/// //server/share being parsed as a local path because Windows takes a forward slash as +/// a separator, and once by a UNC path walking into File.Exists - which blocks for +/// 21 053 ms on an unreachable host against 1.23 ms locally, and which authenticates to +/// somebody else's share with the elevated token of whoever ran the tool. Both arrived through +/// System.IO. A register of network names would have caught zero of the two, and the +/// thing that holds that surface is and its mutation +/// entries, not this file. This one locks the surface nobody has breached yet, which is worth +/// doing and is a smaller claim than "the tool cannot reach the network". +/// +/// And it says nothing about what happens at run time. A module loaded by name through +/// NativeLibrary.Load or LoadLibrary is a string, and a string is invisible to +/// every check here. So is a certificate chain reaching for an AIA URL inside WinVerifyTrust, +/// which is the one call in this product that could go outbound without our code saying so - +/// revocation checking is off deliberately (WindowsBinaryInspector.Request, with ADR-19 +/// named in its comment), and chain building is not the same switch. The instrument for all of +/// that is tools/outbound-probe/outbound.ps1, which runs the product and watches the +/// modules it actually loads and the sockets it actually holds. This class is what runs on +/// every push. +/// +/// TWO REGISTERS, DELIBERATELY DIFFERENT SHAPES. Our own assemblies get an ALLOW list: +/// every native module they bind is named here with its reason, and a new one reddens the +/// build until somebody writes down why. We control that list, so it does not churn, and +/// "nobody adds a way out by accident" is the whole point. Somebody else's assemblies get a +/// DENY list instead: we do not choose what WPF-UI binds, so an allow list there would ask a +/// question on every dependency bump and the question would not be ours to answer. The cost of +/// that choice is stated rather than hidden - a deny list can only refuse the names it knows, +/// so a networking module under a name not listed there would pass. The managed half of the +/// same check has no such hole, because "no type whose namespace begins System.Net" needs no +/// register at all. +/// +/// Both registers, and the two pure functions the canary points at, are in +/// . They were in this file until it passed the length +/// ceiling on the day it was written, and the split went along the seam this project already +/// uses everywhere else - what the guards read lives apart from what the guards assert. +/// +public sealed class OutboundGuards +{ + /// + /// Every native module the three shipped assemblies bind is one somebody wrote down. + /// + [Theory] + [InlineData("Bws.Core")] + [InlineData("Bws.Cli")] + [InlineData("Bws.Gui")] + public void Every_native_module_a_shipped_assembly_binds_is_registered(string projectName) + { + var assembly = AssemblyFacts.Of(projectName); + + var strangers = assembly.NativeModules + .Where(module => !OutboundRegisters.RegisteredNativeModules.ContainsKey(module)) + .OrderBy(module => module, StringComparer.OrdinalIgnoreCase) + .ToArray(); + + Assert.True( + strangers.Length == 0, + $"{projectName} binds a native module nobody registered: " + + $"[{string.Join(", ", strangers)}]. A P/Invoke is the way out of this machine that " + + "the System.Net guard cannot see - winhttp.dll and ws2_32.dll carry no managed type " + + "reference at all. If the module is legitimate, it takes a line in " + + "OutboundRegisters.RegisteredNativeModules saying which call needs it and why (ADR-19)."); + } + + /// + /// None of those modules is one of the refused ones, which is the same claim from the other + /// side. + /// + /// Not redundant with the theory above, and the difference is the failure mode. That + /// one fails when the register is out of date, whichever direction the change went. This one + /// fails when the register itself has been edited to admit a way out - which is the edit a + /// reader of a red build is most tempted to make, because it turns the build green. + /// + [Theory] + [InlineData("Bws.Core")] + [InlineData("Bws.Cli")] + [InlineData("Bws.Gui")] + public void No_shipped_assembly_binds_a_networking_module(string projectName) + { + var assembly = AssemblyFacts.Of(projectName); + + var found = assembly.NativeModules + .Where(OutboundRegisters.NetworkingModules.ContainsKey) + .Select(module => $"{module} ({OutboundRegisters.NetworkingModules[module]})") + .OrderBy(text => text, StringComparer.Ordinal) + .ToArray(); + + Assert.True( + found.Length == 0, + $"{projectName} binds a module whose purpose is to speak to a network: " + + $"[{string.Join(", ", found)}]. Zero outbound connections is a decision, not an " + + "aspiration (ADR-19), and registering the module in this file does not make it one."); + } + + /// + /// The reader is looking at something. A scan that read no modules would satisfy both + /// theories above by finding nothing, and would look exactly like a clean product. + /// + [Fact] + public void The_module_reader_is_reading_something() + { + var core = AssemblyFacts.Of("Bws.Core"); + + // NAMES RATHER THAN A COUNT, and the review of the pull request that brought this file + // is why. A count of three passes when the reader finds three modules nobody expected, + // which is exactly the state this canary exists to refuse: it would read as a working + // scan over a product whose real imports were never seen. + string[] mustBeThere = ["ADVAPI32.dll", "KERNEL32.dll", "PSAPI.dll", "WINTRUST.dll"]; + + var missing = mustBeThere.Where(module => !core.NativeModules.Contains(module)).ToArray(); + + Assert.True( + missing.Length == 0, + $"the module reader did not find [{string.Join(", ", missing)}] in Bws.Core, which " + + "talks to the service control manager, to processes and to WinVerifyTrust. It read " + + $"[{string.Join(", ", core.NativeModules.OrderBy(m => m, StringComparer.OrdinalIgnoreCase))}]. " + + "Either it is reading the wrong thing - and every other assertion in this file is " + + "then passing for that reason rather than because the product is clean - or the " + + "product genuinely stopped making one of those calls, which is worth the same look."); + } + + /// + /// Nothing shipping beside us names the managed network, and this one needs no register. + /// + [Fact] + public void Nothing_that_ships_beside_us_names_the_managed_network() + { + var offenders = new List(); + + foreach (var assembly in OutboundRegisters.ThirdPartyAssemblies()) + { + var types = assembly.TypeReferences + .Where(type => type.StartsWith("System.Net.", StringComparison.Ordinal)); + + var assemblies = assembly.AssemblyReferences + .Where(name => name.StartsWith("System.Net.", StringComparison.OrdinalIgnoreCase)); + + foreach (var name in types.Concat(assemblies).OrderBy(n => n, StringComparer.Ordinal)) + { + offenders.Add($"{assembly.Name} names {name}"); + } + } + + Assert.True( + offenders.Count == 0, + "a library that ships beside this product names the network: " + + $"[{string.Join(", ", offenders)}]. Measured clean on 2026-09-22 across all four - " + + "Wpf.Ui, Wpf.Ui.Abstractions, WinRT.Runtime and Microsoft.Windows.SDK.NET. A " + + "dependency is how a network client arrives without anybody here naming one, so " + + "this is a conversation with the owner rather than an entry in a register (ADR-19)."); + } + + /// + /// And none of them binds a networking module either, which is the native half of it. + /// + [Fact] + public void Nothing_that_ships_beside_us_binds_a_networking_module() + { + var offenders = new List(); + + foreach (var assembly in OutboundRegisters.ThirdPartyAssemblies()) + { + foreach (var module in assembly.NativeModules.Where(OutboundRegisters.NetworkingModules.ContainsKey)) + { + offenders.Add($"{assembly.Name} binds {module} ({OutboundRegisters.NetworkingModules[module]})"); + } + } + + Assert.True( + offenders.Count == 0, + "a library that ships beside this product binds a networking module: " + + $"[{string.Join(", ", offenders)}] (ADR-19)."); + } + + /// + /// The same canary as above, for the half of the scan that walks a directory. + /// + [Fact] + public void The_scan_of_what_ships_beside_us_found_the_libraries_it_is_meant_to_read() + { + var names = OutboundRegisters.ThirdPartyAssemblies() + .Select(assembly => assembly.Name) + .OrderBy(name => name, StringComparer.Ordinal) + .ToArray(); + + // The four measured on 2026-09-22, named rather than counted. A count of four is + // satisfied by four libraries nobody expected - and the way this scan would actually + // break is by reading ONE output folder instead of both, which loses Wpf.Ui while + // still finding four things beside the terminal. + string[] mustBeThere = + [ + "Wpf.Ui", "Wpf.Ui.Abstractions", "WinRT.Runtime", "Microsoft.Windows.SDK.NET" + ]; + + var missing = mustBeThere + .Where(name => !names.Contains(name, StringComparer.OrdinalIgnoreCase)) + .ToArray(); + + Assert.True( + missing.Length == 0, + $"the scan did not read [{string.Join(", ", missing)}]. It read " + + $"[{string.Join(", ", names)}]. Either it is looking in the wrong folder - and the " + + "two assertions above are then passing over a set that does not contain what ships " + + "- or a dependency left the tree, which wants a look of its own rather than a " + + "quietly smaller scan."); + } + + /// + /// The Win32 inventory names nothing that reaches off this machine. + /// + [Fact] + public void The_win32_inventory_names_nothing_that_reaches_the_network() + { + var offenders = new List(); + + foreach (var file in OutboundRegisters.InventoryFiles()) + { + var lines = File.ReadAllLines(file); + + for (var index = 0; index < lines.Length; index++) + { + // Comments carry the argument for every name on this list, and one of them + // would have to be able to discuss WinHTTP in order to say the product does + // not use it. The declarations are what this reads. + var line = OutboundRegisters.Declaration(lines[index]); + + if (line.Length == 0) + { + continue; + } + + if (OutboundRegisters.ReachesTheNetwork(line)) + { + offenders.Add( + $"{Path.GetFileName(Path.GetDirectoryName(file))}/NativeMethods.txt:{index + 1} declares {line}"); + } + } + } + + Assert.True( + offenders.Count == 0, + $"the Win32 inventory declares something that reaches the network: [{string.Join(", ", offenders)}]. " + + "That file calls itself the honest inventory of what this tool touches in the " + + "operating system, and zero outbound connections is ADR-19."); + } + + /// + /// The list of shipped projects still matches what is under src. + /// + /// This is the guard for the guards, and it is the cheapest one in the file. Every + /// assembly-reading check in this project - layering, licences, the two theories above - + /// walks a list written out by hand. A fourth project under src would be read by + /// none of them and would redden nothing, which is the failure this project already names + /// as the worst kind: a thing that never ran looks exactly like a thing that found nothing. + /// + [Fact] + public void The_list_of_shipped_projects_still_matches_what_is_under_src() + { + var onDisk = Directory + .EnumerateDirectories(Path.Combine(SourceTree.Root(), "src")) + .Select(path => Path.GetFileName(path)!) + .OrderBy(name => name, StringComparer.Ordinal) + .ToArray(); + + var registered = GuardedAssemblies.Shipped + .OrderBy(name => name, StringComparer.Ordinal) + .ToArray(); + + Assert.True( + onDisk.SequenceEqual(registered, StringComparer.Ordinal), + $"src holds [{string.Join(", ", onDisk)}] and GuardedAssemblies.Shipped says " + + $"[{string.Join(", ", registered)}]. Every guard that reads a built assembly walks " + + "that list, so a project missing from it ships without one of them ever looking at " + + "it - and every one of those guards stays green while that is true."); + } + + /// + /// The registers reject what they exist to reject, and leave alone what they do not. + /// + /// A guard nobody has watched fail is indistinguishable from a guard that reads + /// nothing, and the product has been clean on every one of these since the day it was + /// written - so nothing above has ever been seen to say no. + /// + [Fact] + public void The_registers_catch_every_shape_they_exist_to_catch() + { + string[] mustBeRefusedAsModules = + [ + "ws2_32.dll", "WS2_32.DLL", "winhttp.dll", "wininet.dll", + "urlmon.dll", "dnsapi.dll", "mpr.dll", "netapi32.dll" + ]; + + var missedModules = mustBeRefusedAsModules + .Where(module => !OutboundRegisters.NetworkingModules.ContainsKey(module)) + .ToArray(); + + Assert.True( + missedModules.Length == 0, + $"the networking module register does not know: [{string.Join(", ", missedModules)}]. " + + "The casing cases are in that list on purpose - a module name arrives from the " + + "linker in whatever case the SDK wrote it, and ADVAPI32 is upper while kernel32 in " + + "somebody else's assembly is lower."); + + string[] mustBeLeftAlone = ["ADVAPI32.dll", "KERNEL32.dll", "dwmapi.dll", "UXTHEME.dll"]; + + var wronglyRefused = mustBeLeftAlone.Where(OutboundRegisters.NetworkingModules.ContainsKey).ToArray(); + + Assert.True( + wronglyRefused.Length == 0, + $"the networking module register refuses something ordinary: [{string.Join(", ", wronglyRefused)}]. " + + "A register that refused everything would satisfy the half above and be useless."); + + string[] inventoryLinesThatMustFail = + [ + "WinHttpOpen", "InternetOpenUrl", "URLDownloadToFileW", "WSAStartup", + "DnsQuery_W", "WNetAddConnection2W" + ]; + + var missedNames = inventoryLinesThatMustFail.Where(line => !OutboundRegisters.ReachesTheNetwork(line)).ToArray(); + + Assert.True( + missedNames.Length == 0, + $"the inventory register would not refuse: [{string.Join(", ", missedNames)}]."); + + // Every one of these is on the real list today, and the third is the one that matters. + // SC_MANAGER_CONNECT reddened the first run of this guard, because it contains the word + // "connect" and "connect" is an ordinary word. It is here so that a future widening of + // the register back into substring matching fails on a clean tree rather than sending + // somebody to read the service control manager's own constants. + string[] inventoryLinesThatMustPass = + [ + "OpenSCManager", + "QueryServiceConfig2", + "SC_MANAGER_CONNECT", + "WinVerifyTrust", + "GetProcessMemoryInfo", + "DwmSetWindowAttribute", + "CryptCATAdminEnumCatalogFromHash", + "SERVICE_ENUMERATE_DEPENDENTS" + ]; + + var wronglyRejected = inventoryLinesThatMustPass.Where(OutboundRegisters.ReachesTheNetwork).ToArray(); + + Assert.True( + wronglyRejected.Length == 0, + $"the inventory register refuses a name the product genuinely uses: " + + $"[{string.Join(", ", wronglyRejected)}]. Every one of those is on the real list in " + + "src/Bws.Core/NativeMethods.txt, so this would be a red build on a clean tree."); + + // The comment half, which is the other way this scan could be wrong. Those files argue + // at length for every name they carry, and one of them has to be able to say the words + // in the register in order to explain that the product does not use them. + string[] commentaryThatMustNotCount = + [ + "// WinHttpOpen is deliberately not here - ADR-19 forbids it outright.", + " // socket, and why this product never opens one", + "WinVerifyTrust // not WinHttp, whatever the first four letters suggest" + ]; + + var countedProse = commentaryThatMustNotCount + .Where(line => OutboundRegisters.ReachesTheNetwork(OutboundRegisters.Declaration(line))) + .ToArray(); + + Assert.True( + countedProse.Length == 0, + $"the inventory scan counted a comment as a declaration: [{string.Join(", ", countedProse)}]. " + + "Those files exist to argue about what the product touches, so a scan that cannot " + + "tell an argument from a declaration makes the argument unwritable."); + } +} diff --git a/tests/Bws.Architecture.Tests/OutboundRegisters.cs b/tests/Bws.Architecture.Tests/OutboundRegisters.cs new file mode 100644 index 0000000..fd681de --- /dev/null +++ b/tests/Bws.Architecture.Tests/OutboundRegisters.cs @@ -0,0 +1,200 @@ +// Explicit for the same reason SourceTree.cs says so at the top of itself: these guards read +// files off disk, and the implicit using set is not something to depend on across projects. +using System.IO; + +namespace Bws.Architecture.Tests; + +/// +/// What refuses, and why each entry is on its list. +/// +/// Kept apart from the assertions for the reason Sources.cs gives about the file +/// list it holds. A register is read by a person deciding whether to add a line to it, +/// and that person is not reading the assertions - so the argument for every entry belongs +/// where the entry is. The whole case for these three registers existing at all, including +/// what they cannot do, is at the top of . +/// +internal static class OutboundRegisters +{ + /// + /// Every native module our own code may bind, and why. Measured 2026-09-22 by reading the + /// P/Invoke imports out of the built assemblies: four modules in the core, one in the + /// window, and none at all in the terminal. + /// + /// Adding a line here is the deliberate act. Failing this test is the question being + /// asked, and the answer belongs in this list rather than in a comment beside the call. + /// + internal static readonly Dictionary RegisteredNativeModules = + new(StringComparer.OrdinalIgnoreCase) + { + ["ADVAPI32.dll"] = + "The service control manager itself - opening it, enumerating it, reading and " + + "changing configuration, starting and stopping. Every name in the first half of " + + "src/Bws.Core/NativeMethods.txt lives here.", + + ["KERNEL32.dll"] = + "Process handles and what can be asked of them without opening a process for " + + "reading: OpenProcess, GetProcessTimes, TerminateProcess, and the handle types " + + "underneath all of the above.", + + ["PSAPI.dll"] = + "GetProcessMemoryInfo, which is the one question in this product asked of a " + + "process rather than of the manager - how much memory the thing behind an entry " + + "is using.", + + ["WINTRUST.dll"] = + "Signature verification: WinVerifyTrust and the catalogue calls beside it. This " + + "is the module closest to being a way out, because a certificate chain can " + + "fetch. Revocation checking is off by WTD_REVOKE_NONE and the comment there " + + "names ADR-19 as the reason - what that switch does NOT cover is measured by " + + "the runtime probe rather than argued about here.", + + ["dwmapi.dll"] = + "DwmSetWindowAttribute, and it is the whole of what the window touches in the " + + "operating system: telling the window manager that the title bar above this " + + "window is a dark one." + }; + + /// + /// Modules that exist to speak to a network, refused in anything that ships beside us. + /// + /// A deny list, and the header says why it is one here and an allow list above. It + /// can only refuse what it knows, so each entry is a name a networking library would + /// actually bind rather than a guess at a family. + /// + internal static readonly Dictionary NetworkingModules = + new(StringComparer.OrdinalIgnoreCase) + { + ["ws2_32.dll"] = "Winsock, which is what a socket is on Windows", + ["wsock32.dll"] = "the older Winsock", + ["mswsock.dll"] = "the Winsock service provider", + ["winhttp.dll"] = "WinHTTP, the Windows HTTP stack", + ["wininet.dll"] = "WinINet, the other Windows HTTP stack", + ["urlmon.dll"] = "URLDownloadToFile lives here - one call, one download", + ["httpapi.dll"] = "the HTTP Server API", + ["dnsapi.dll"] = "a name lookup, which is a network round trip", + ["iphlpapi.dll"] = "the IP helper API. It reads tables rather than opening " + + "connections, and it is refused anyway because nothing in this " + + "product has any use for it", + ["netapi32.dll"] = "network management, including enumerating somebody else's shares", + ["mpr.dll"] = "the multiple provider router - WNetAddConnection, which is how a " + + "share gets mounted. The nearest module to the way ADR-19 was " + + "actually broken here", + ["rasapi32.dll"] = "dial-up and VPN", + ["winsta.dll"] = "the session manager, which reaches other sessions and other hosts" + }; + + /// + /// Names refused in the Win32 inventory files, checked at the DECLARATION rather than at + /// the module. + /// + /// This is the weakest of the checks in this class and it is kept for one reason: the + /// error message. Everything it catches, the module register above catches too - and + /// the module register additionally catches a hand-written DllImport that never appears in + /// an inventory file. What this one adds is where it fires. NativeMethods.txt calls itself + /// "the honest inventory of what the tool touches in the operating system", and a guard + /// that fails on the line somebody typed says more than one that fails on a built file. + /// + /// MATCHED ON THE START OF THE DECLARATION, NEVER ON A FRAGMENT OF IT, AND THE FIRST RUN + /// OF THIS GUARD IS WHY. Written with a substring match it refused + /// SC_MANAGER_CONNECT - which is connecting to the service control manager, and is + /// on the real list in src/Bws.Core/NativeMethods.txt. connect and socket are + /// ordinary words, and this repository already carries the same lesson from the other + /// direction, where a mechanical replacement of Status landed in a cell template. + /// The file declares one API name per line, so the beginning of that name is what + /// identifies the family, and SC_MANAGER_CONNECT begins with neither of them. That + /// exact case is in the canary below so that it cannot come back. + /// + internal static readonly string[] NetworkingNamesInTheInventory = + [ + "WinHttp", + "InternetOpen", + "InternetConnect", + "URLDownloadToFile", + "WSAStartup", + "WSASocket", + "socket", + "connect", + "DnsQuery", + "GetAddrInfo", + "gethostby", + "WNetAddConnection", + "NetShareEnum", + "HttpSendRequest", + "HttpOpenRequest" + ]; + + /// + /// The managed libraries that land beside the product when it is built. + /// + /// Both output folders, unioned by assembly name. The window and the terminal do not + /// carry the same set - Wpf.Ui reaches only the first - and reading one folder would leave + /// the other unguarded while looking complete. + /// + internal static IEnumerable ThirdPartyAssemblies() + { + var ours = new HashSet( + GuardedAssemblies.Shipped.Select(GuardedAssemblies.AssemblyNameOf), + StringComparer.OrdinalIgnoreCase); + + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var project in GuardedAssemblies.Shipped) + { + var folder = Path.GetDirectoryName(GuardedAssemblies.PathOf(project)); + + if (folder is null) + { + continue; + } + + // Top level only. A runtimes/ folder below holds native halves that carry no + // managed metadata at all, and walking into it would trade a clear answer for a + // pile of files this reader cannot open. + foreach (var file in Directory.EnumerateFiles(folder, "*.dll", SearchOption.TopDirectoryOnly)) + { + var name = Path.GetFileNameWithoutExtension(file); + + if (ours.Contains(name) || !seen.Add(name)) + { + continue; + } + + if (AssemblyFacts.TryRead(file, out var facts)) + { + yield return facts; + } + } + } + } + + /// + /// Whether one declaration from a Win32 inventory file reaches off this machine. + /// + /// A pure function over one line, so the canary can point it at declarations that are + /// not in this repository at all. A canary that could only run the scan over the real + /// files would prove the scan runs, never that it can say no. + /// + internal static bool ReachesTheNetwork(string declaration) => + Array.Exists( + NetworkingNamesInTheInventory, + name => declaration.StartsWith(name, StringComparison.OrdinalIgnoreCase)); + + /// + /// One line of an inventory file reduced to the name it declares, or empty if it declares + /// none. Comments carry the argument for every name on those lists, and one of them would + /// have to be able to discuss WinHTTP in order to say the product does not use it. + /// + internal static string Declaration(string line) + { + var comment = line.IndexOf("//", StringComparison.Ordinal); + + return (comment >= 0 ? line[..comment] : line).Trim(); + } + + /// The Win32 inventory files, one per project that declares any native call. + internal static IEnumerable InventoryFiles() => + Directory.EnumerateFiles( + Path.Combine(SourceTree.Root(), "src"), + "NativeMethods.txt", + SearchOption.AllDirectories); +} diff --git a/tests/Bws.Core.Tests/SignatureWithoutTheNetworkTests.cs b/tests/Bws.Core.Tests/SignatureWithoutTheNetworkTests.cs new file mode 100644 index 0000000..cdee8a1 --- /dev/null +++ b/tests/Bws.Core.Tests/SignatureWithoutTheNetworkTests.cs @@ -0,0 +1,216 @@ +using Bws.Core; + +namespace Bws.Core.Tests; + +/// +/// That refusing to reach the network never turns into a sentence about somebody's certificate. +/// +/// WHERE THIS CAME FROM. On 2026-09-22 a runtime probe found that reading signatures +/// opened HTTP connections to a third party - measured three runs out of three, with +/// certificates.intel.com named out of the DNS cache. Turning revocation checking off, which +/// this code had done since the beginning and had a comment about, does not stop the chain +/// engine fetching a certificate it does not hold. WTD_CACHE_ONLY_URL_RETRIEVAL does. +/// +/// AND THAT FIX, ON ITS OWN, WOULD HAVE BOUGHT SILENCE WITH A LIE. Once the engine may +/// not go and get an intermediate certificate, a properly signed file can come back non-zero - +/// and the classifier would then have reported OUR refusal as UntrustedRoot, which is a claim +/// about the machine's trust store. The owner named that risk before a line of it was written: +/// a tool whose subject is trust must not say a certificate is bad when what happened is that +/// it would not look it up. +/// +/// The direction that is closed by construction, and it is the half that would be +/// unforgivable. Trusted comes only from a result of zero, and zero needs a chain that was +/// built and validated. A refused fetch can prevent a zero and cannot manufacture one. The last +/// test in this file is that claim, asserted over every code the others use rather than argued +/// for in a comment. +/// +public sealed class SignatureWithoutTheNetworkTests +{ + private const int Trusted = 0; + private const int NoSignature = unchecked((int)0x800B0100); + private const int Expired = unchecked((int)0x800B0101); + private const int UntrustedRoot = unchecked((int)0x800B0109); + private const int NoChain = unchecked((int)0x800B010A); + private const int Revoked = unchecked((int)0x800B010C); + private const int Tampered = unchecked((int)0x80096010); + private const int Nameless = unchecked((int)0x80004005); + + /// + /// A verdict that is a fact about the file or about its own certificate survives the quiet + /// mode, because none of them needed the network to be reached. + /// + [Theory] + [InlineData(Trusted, SignatureStatus.Trusted)] + [InlineData(NoSignature, SignatureStatus.NotSigned)] + [InlineData(Expired, SignatureStatus.Expired)] + [InlineData(Tampered, SignatureStatus.Tampered)] + public void A_verdict_about_the_file_itself_survives_the_quiet_mode(int result, SignatureStatus expected) + { + var reading = WindowsBinaryInspector.Settle(result, NetworkPaths.Skip, () => "Someone"); + + Assert.Equal(ReadOutcome.Present, reading.Outcome); + Assert.Equal(expected, reading.Value!.Status); + } + + /// + /// A verdict about the chain to a root does NOT, because in the quiet mode it cannot be + /// told apart from this tool declining to complete the chain. + /// + [Theory] + [InlineData(UntrustedRoot)] + [InlineData(NoChain)] + [InlineData(Nameless)] + public void A_verdict_about_the_chain_becomes_a_refusal_in_the_quiet_mode(int result) + { + var reading = WindowsBinaryInspector.Settle(result, NetworkPaths.Skip, () => "Someone"); + + Assert.Equal(ReadOutcome.Denied, reading.Outcome); + + // The number travels, exactly as it does for a refused read from the manager. Rule 8 + // forbids a failed read that looks like an answer, and it equally forbids one that + // arrives without the means to diagnose it. + Assert.Equal(result, reading.ErrorCode); + Assert.False(string.IsNullOrWhiteSpace(reading.Reason)); + } + + /// + /// With the network allowed, every one of those is a verdict again. The switch has to work + /// in both directions or it is not a switch, it is a removal. + /// + [Theory] + [InlineData(UntrustedRoot, SignatureStatus.UntrustedRoot)] + [InlineData(NoChain, SignatureStatus.Unknown)] + [InlineData(Revoked, SignatureStatus.Revoked)] + [InlineData(Nameless, SignatureStatus.Unknown)] + public void The_same_result_is_a_verdict_again_once_the_network_is_allowed( + int result, SignatureStatus expected) + { + var reading = WindowsBinaryInspector.Settle(result, NetworkPaths.Follow, () => "Someone"); + + Assert.Equal(ReadOutcome.Present, reading.Outcome); + Assert.Equal(expected, reading.Value!.Status); + Assert.Equal(result, reading.Value.ResultCode); + } + + /// + /// The publisher is not read on the way to a refusal. + /// + /// Not a saving, although it is one - reading it opens the file a second time. It is that a + /// refusal carries no value, so producing one would mean doing work to decorate an answer + /// this code is declining to give. + /// + [Fact] + public void Nothing_is_read_about_a_signature_this_code_is_refusing_to_report() + { + var asked = false; + + var reading = WindowsBinaryInspector.Settle( + UntrustedRoot, + NetworkPaths.Skip, + () => { asked = true; return "Someone"; }); + + Assert.Equal(ReadOutcome.Denied, reading.Outcome); + Assert.False(asked, "the publisher was read in order to be thrown away with the refusal."); + } + + /// + /// And the publisher IS read when there is an answer to attach it to, which is the half + /// that keeps the test above from passing on a function that never reads anything. + /// + [Fact] + public void The_publisher_is_read_when_there_is_a_verdict_to_carry_it() + { + var reading = WindowsBinaryInspector.Settle(Trusted, NetworkPaths.Skip, () => "Microsoft Windows"); + + Assert.Equal(ReadOutcome.Present, reading.Outcome); + Assert.Equal("Microsoft Windows", reading.Value!.Publisher); + } + + /// + /// NOTHING BECOMES TRUSTED THAT WAS NOT ALREADY A ZERO, IN EITHER MODE. + /// + /// The one sentence in this file that would be a disaster to get wrong, asserted rather + /// than reasoned about. Every other test here is about losing information honestly - this + /// one is about never gaining any. + /// + [Fact] + public void No_result_but_zero_is_ever_reported_as_trusted() + { + foreach (var mode in new[] { NetworkPaths.Skip, NetworkPaths.Follow }) + { + // Zero is the one result that MAY be Trusted, so it is the one this loop skips. + // It is in the shared list because the assertion below it - that a refusal always + // carries a sentence - has to walk every shape, including the ones that never + // become refusals at all. + foreach (var code in EveryCode.Where(c => c != Trusted)) + { + var reading = WindowsBinaryInspector.Settle(code, mode, () => "Someone"); + + var claimsTrust = + reading.Outcome == ReadOutcome.Present + && reading.Value!.Status == SignatureStatus.Trusted; + + Assert.False( + claimsTrust, + $"result 0x{code:X8} in mode {mode} was reported as Trusted. Refusing to " + + "reach the network may cost an answer and may never produce one - a tool " + + "that called an unverified binary trusted would be worse than one that " + + "said nothing at all."); + } + } + } + + /// + /// A refusal always says something, whatever number produced it. + /// + /// Raised by the review of the pull request that introduced this file, and it was right: + /// the first version handed string.Empty to the refusal whenever + /// Marshal.GetExceptionForHR answered null, which it does for every non-negative + /// HRESULT. S_FALSE is 1, it reaches that line, and it was already in the list this + /// test walks - the earlier assertion simply did not ask about the sentence. + /// + /// A refusal with no sentence is exactly the shape rule 8 forbids: it looks like a field + /// that was read and came back empty. + /// + [Fact] + public void Every_refusal_carries_both_the_number_and_a_sentence() + { + foreach (var code in EveryCode) + { + var reading = WindowsBinaryInspector.Settle(code, NetworkPaths.Skip, () => "Someone"); + + if (reading.Outcome != ReadOutcome.Denied) + { + continue; + } + + // The number, either as it arrived or as Reading unwraps it. A result in the + // FACILITY_WIN32 family is deliberately reduced to its Win32 code on the way in - + // 0x80070005 becomes 5 - because access denied out of a signature and access denied + // out of the manager were two different numbers for one fact, and that has its own + // mutation entry. Asserting the raw value here would have quietly re-opened it. + var unwrapped = ((uint)code & 0xFFFF0000u) == 0x80070000u + ? (int)((uint)code & 0xFFFFu) + : code; + + Assert.Equal(unwrapped, reading.ErrorCode); + + Assert.False( + string.IsNullOrWhiteSpace(reading.Reason), + $"the refusal produced by result 0x{code:X8} carries no sentence at all. The " + + "number alone reaches the JSON, and a reader meets a field that was not read " + + "and is told nothing about why."); + } + } + + /// + /// Every shape a verification result can take, including the ones Windows would never + /// produce. A policy asserted only over codes somebody expected is a policy with a hole + /// the shape of what they did not. + /// + private static readonly int[] EveryCode = + [ + Trusted, NoSignature, Expired, UntrustedRoot, NoChain, Revoked, Tampered, Nameless, + unchecked((int)0x80070005), 1, -1, int.MinValue, int.MaxValue + ]; +}