Update WireGuard COM interface to pass params instead of config

[bsclifton]

Fixes brave/brave-browser#37960

Submitter Checklist:

• I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
• There is a ticket for my issue
• Used GitHub auto-closing keywords in the PR description above
• Wrote a good PR/commit description
• Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
• Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
• Checked the PR locally:
  • npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
  • npm run presubmit wiki, npm run gn_check, npm run tslint
• Ran git rebase master (if needed)

Reviewer Checklist:

• A security review is not needed, or a link to one is included in the PR description
• New files have MPL-2.0 license header
• Adequate test coverage exists to prevent regressions
• Major classes, functions and non-trivial code blocks are well-commented
• Changes in component dependencies are properly reflected in gn
• Code follows the style guide
• Test plan is specified in PR before merging

After-merge Checklist:

• The associated issue milestone is set to the smallest version that the
  changes has landed on
• All relevant documentation has been updated, for instance:
  • https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • https://github.com/brave/brave-browser/wiki/Fingerprinting-Protections
  • https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • https://github.com/brave/brave-browser/wiki/Web-Compatibility-Exceptions-in-Brave
  • https://github.com/brave/brave-browser/wiki/QA-Guide
  • https://github.com/brave/brave-browser/wiki/P3A

Test Plan:

See brave/brave-browser#37960

[diracdeltas]

see comment here: https://hackerone.com/reports/2481145#activity-28112578

[bridiver]

do we expect this to happen under some circumstances or are we trying to handle an unexpected condition here? If it's the latter I think we should just fail here because we might be unintentionally opening another attack vector

[bsclifton]

This is the current behavior - I thought I made comments in this PR explaining... but basically the tray program is passing empty string for the arguments. This triggers the "reconnect using the last known config" logic.

We could rework the tray program - but that's a bigger change of course.

[thypon]

I think, best is to harden this part with the follow-up in brave/brave-browser#39229

[bsclifton]

Issue solved - build works locally 🎉

Investigating the COM interface defined here:

brave-core/components/brave_vpn/common/wireguard/win/brave_wireguard_manager_idl.idl

Line 7 in 9f8f2e5

uuid(053057AB-CF06-4E6C-BBAD-F8DA6436D933),

There is already a separate CLSID per-channel (Release, Beta, Nightly, etc):

brave-core/browser/brave_vpn/win/service_details.cc

Lines 29 to 34 in 9f8f2e5

	// 053057AB-CF06-4E6C-BBAD-F8DA6436D933
	constexpr IID kBraveWireguardServiceIID = {
	0x053057ab,
	0xcf06,
	0x4e6c,
	{0xbb, 0xad, 0xf8, 0xda, 0x64, 0x36, 0xd9, 0x33}};

The next step I'm looking at now is making a per-channel IID (interface ID). We could then update the code to return the correct IID per channel here:

brave-core/browser/brave_vpn/win/service_details.cc

Lines 102 to 104 in 9f8f2e5

	const IID& GetBraveVpnWireguardServiceIid() {
	return kBraveWireguardServiceIID;
	}

And then the IDL can be rebuilt. Basically, if this gets installed, the IID registration (where # of arguments changes) will break other channels trying to use VPN on the machine. If this was merged and installed for Nightly, Release channel VPN would stop working properly for Windows folks because the COM interface changed.

[bsclifton]

OK made some changes and verified it works with latest PR builder.
Rebased and squashed those changes.
Also removed the SKIP labels so this can run on all platforms

[bsclifton]

Tested and this works great 😄

[thypon]

https://bravesoftware.slack.com/archives/C079KC463FH/p1719482602829669

[github-actions]

[puLL-Merge] - brave/brave-core@24261

Description

This PR updates the Brave VPN WireGuard service implementation, particularly focusing on improving security and input validation. The changes include modifying the interface for enabling VPN connections, adding input validation for WireGuard configuration parameters, and updating the COM interface identifier.

Possible Issues

1. The change in the COM interface identifier (from 053057AB-CF06-4E6C-BBAD-F8DA6436D933 to 6D319801-690B-441E-8C94-5C18D8E7E9D7) may cause compatibility issues with existing clients that use the old interface.

2. The new input validation logic might potentially reject some previously valid inputs, which could lead to connection failures for some users until they update their configuration.

Security Hotspots

1. The EnableVpn function now takes individual parameters instead of a single config string, which allows for better input validation. However, ensure that all callers of this function are updated to provide the correct parameters.

2. The new validation functions (ValidateKey, ValidateAddress, ValidateEndpoint) improve security by checking inputs, but their implementation should be carefully reviewed to ensure they don't introduce new vulnerabilities.

3. The WireguardGenerateKeypair function now uses SysAllocString to allocate memory for keys. Ensure that this memory is properly freed to prevent memory leaks.

Changes

Changes

1. brave_wireguard_manager.cc:

   • Updated EnableVpn to take separate parameters instead of a single config string.
   • Added input validation for public key, private key, address, and endpoint.
   • Improved error handling and logging.
   • Updated GenerateKeypair to use SysAllocString for key allocation.

2. brave_wireguard_manager.h:

   • Updated EnableVpn method signature to match the new implementation.

3. wireguard_service_runner.cc:

   • Added denial of access for guest accounts in the COM security settings.

4. status_tray_runner.cc:

   • Updated ConnectVPN to use empty strings for reconnection using last known good config.

5. service_details.cc:

   • Updated the COM interface identifier.

6. wireguard_connection_api_impl_win.cc:

   • Updated PlatformConnectImpl to use the new EnableVpn interface.

7. wireguard_utils_win.cc and wireguard_utils_win.h:

   • Updated EnableBraveVpnWireguardService to take separate parameters.
   • Improved error handling and logging.

8. wireguard_utils.cc and wireguard_utils.h:

   • Added new validation functions: ValidateKey, ValidateAddress, and ValidateEndpoint.
   • Updated GenerateNewX25519Keypair implementation.

9. Added new unit tests in wireguard_utils_unittest.cc for the new validation functions.

10. Updated various MIDL-generated files to reflect the changes in the COM interface.

These changes significantly improve the security and robustness of the Brave VPN WireGuard implementation by adding thorough input validation and improving error handling.