Our own implementation works fine, however we should use OpenSSL's one since we already link to the library.
Base64Decode() and Base64Encode() return the required buffer size when "dst" is NULL.
This allows to efficiently allocate a buffer, without wasting memory or risking an overflow.
Base64FromBin() and Base64ToBin() perform all steps, returning a heap-allocated buffer with the data in it.
Turns out %S refers to ANSI/UTF-8 and %s to UTF-16.
This commit fixes a buffer overflow reported by AddressSanitizer and removes an unnecessary conversion to UTF-16.
The open-source project began with version 1.00, build 9022.
With the exception of an informative message fallback for builds older than 9428 (2014), all checks were for closed-source builds.
- <pthread.h> included for the "pthread_t" type definition.
- <net/ethernet.h> include removed as the header doesn't exist.
- AI_ALL and AI_V4MAPPED defined to 0 as the options don't exist.
52 bytes in 2 blocks are definitely lost in loss record 5 of 13
at 0x483877F: malloc (vg_replace_malloc.c:307)
by 0x4ABB1BB: UnixMemoryAlloc (Unix.c:2033)
by 0x4A7FABF: InternalMalloc (Memory.c:3819)
by 0x4A7B769: MallocEx (Memory.c:3650)
by 0x4A7B769: Malloc (Memory.c:3641)
by 0x4AA71A9: CopyStr (Str.c:1884)
by 0x4A61A9C: DnsCacheReverseUpdate (DNS.c:257)
by 0x4A62123: DnsResolveReverse (DNS.c:506)
by 0x4A93EB3: GetHostName (Network.c:15023)
by 0x4A93EB3: AcceptInitEx (Network.c:12589)
by 0x4934659: TCPAcceptedThread (Listener.c:172)
by 0x4A76469: ThreadPoolProc (Kernel.c:872)
by 0x4ABD159: UnixDefaultThreadProc (Unix.c:1589)
by 0x51C2EA6: start_thread (pthread_create.c:477)
2,280 (684 direct, 1,596 indirect) bytes in 9 blocks are definitely lost in loss record 11 of 13
at 0x483877F: malloc (vg_replace_malloc.c:307)
by 0x4C65AC5: gaih_inet.constprop.0 (getaddrinfo.c:1058)
by 0x4C67224: getaddrinfo (getaddrinfo.c:2256)
by 0x4A61E06: DnsResolver (DNS.c:404)
by 0x4A76469: ThreadPoolProc (Kernel.c:872)
by 0x4ABD159: UnixDefaultThreadProc (Unix.c:1589)
by 0x51C2EA6: start_thread (pthread_create.c:477)
by 0x4C7CDEE: clone (clone.S:95)
Also, the default timeout value is set to 30000 (milliseconds) instead of 10000.
The change is made because it was reported that some routers failed to connect in time.
This commit also fixes a bug which caused the server to initialize all boolean options to false.
It was caused by SiLoadProtoCfg() not checking whether the item exists in the configuration file.
CfgGetBool() always returns false if the item doesn't exist.
From a functional point of view, the main improvement is that GetIP() now always prioritizes IPv6 over IPv4.
The previous implementation always returned an IPv4 address, unless not available: in such case it failed.
This means that now connections to hostnames should be established via IPv6 if available.
From a programmer point of view, getting rid of the insane wrappers is enough to justify a complete rewrite.
As an extra, several unrelated unused global variables are removed.
Before this commit, the IP address reported by the NAT-T server was immediately discarded.
That's because the peer should be accessible via the IP address used to establish the TCP connection.
User "domosekai" (https://www.domosekai.com) pointed out that the NAT-T IP address should be taken into account.
In his case it's required due to his broadband carrier's NAT causing TCP and UDP to have different external IPs.
Co-authored-by: domosekai <54519668+domosekai@users.noreply.github.com>
This greatly improves performance and reduces the binary's size (~0.2 MB vs ~5 MB).
All recent Windows versions are supported, starting with Vista.
No dialogs are created, aside from error/warning ones in case of failure.
The only dependency (aside from Windows libraries) is libhamcore.
The bug caused ProtoOptionsGet and ProtoOptionsSet not to work anymore after c90617e0e86dedf78e0e3c8a71263a80eec29caa.
The functions were introduced in aa65327e73, but the issue went unnoticed because bool was the same as UINT.
BOOL was just an alias for bool, this commit replaces all instances of it for consistency.
For some reason bool was defined as a 4-byte integer instead of a 1-byte one, presumably to match WinAPI's definition: https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types
Nothing should break now that bool is 1-byte, as no protocol code appears to be relying on the size of the data type.
PACK, for example, explicitly stores boolean values as 4-byte integers.
This commit can be seen as a follow-up to 61ccaed4f6.
WgkAdd command - Add a WireGuard key
Help for command "WgkAdd"
Purpose:
Add a WireGuard key
Description:
This command can be used to add a WireGuard key to the allowed key list.
To execute this command, you must have VPN Server administrator privileges.
Usage:
WgkAdd [key] [/HUB:hub] [/USER:user]
Parameters:
key - WireGuard key. Make sure it is the public one!
/HUB - Hub the key will be associated to.
/USER - User the key will be associated to, in the specified hub.
================================================================================
WgkDelete command - Delete a WireGuard key
Help for command "WgkDelete"
Purpose:
Delete a WireGuard key
Description:
This command can be used to delete a WireGuard key from the allowed key list.
To execute this command, you must have VPN Server administrator privileges.
Usage:
WgkDelete [key]
Parameters:
key - WireGuard key.
================================================================================
WgkEnum command - List the WireGuard keys
Help for command "WgkEnum"
Purpose:
List the WireGuard keys
Description:
This command retrieves the WireGuard keys that are allowed to connect to the server, along with the associated Virtual Hub and user.
You can add a key with the WgkAdd command.
You can delete a key with the WgkDelete command.
To execute this command, you must have VPN Server administrator privileges.
Usage:
WgkEnum
Please note that the implementation is not 100% conformant to the protocol whitepaper (https://www.wireguard.com/papers/wireguard.pdf).
More specifically: all peers are expected to send a handshake initiation once the current keypair is about to expire or is expired.
I decided not to do that because our implementation is meant to act as a server only. A true WireGuard peer acts, instead, as both a client and a server.
Once the keypair is expired, we immediately delete the session.
The cookie mechanism can be implemented in future.
As for authentication: unfortunately using the already existing methods is not possible due to the protocol not providing a way to send strings to a peer.
That's because WireGuard doesn't have a concept of "users": it identifies a peer through the public key, which is determined using the source address.
As a solution, this commit adds a special authentication method: once we receive the handshake initiation message and decrypt the peer's public key, we check whether it's in the allowed key list.
If it is, we retrieve the associated Virtual Hub and user; if the hub exists and the user is in it, the authentication is successful.
The allowed key list is stored in the configuration file like this:
declare WireGuardKeyList
{
declare 96oA7iMvjn7oXiG3ghBDPaSUytT75uXceLV+Fx3XMlM=
{
string Hub DEFAULT
string User user
}
}
WireGuard does not provide any configuration messages, meaning that we cannot push the IP address we receive from the DHCP server to the client.
In order to overcome the limitation we don't perform any DHCP operations and instead just extract the source IP address from the first IPv4 packet we receive in the tunnel.
The gateway address and the subnet mask can be set using the new "SetStaticNetwork" command. The values can be retrieved using "OptionsGet".
In future we will add a "allowed source IP addresses" function, similar to what the original WireGuard implementation provides.
================================================================================
SetStaticNetwork command - Set Virtual Hub static IPv4 network parameters
Help for command "SetStaticNetwork"
Purpose:
Set Virtual Hub static IPv4 network parameters
Description:
Set the static IPv4 network parameters for the Virtual Hub. They are used when DHCP is not available (e.g. WireGuard sessions).
You can get the current settings by using the OptionsGet command.
Usage:
SetStaticNetwork [/GATEWAY:gateway] [/SUBNET:subnet]
Parameters:
/GATEWAY - Specify the IP address of the gateway that will be used for internet communication.
/SUBNET - Specify the subnet mask, required to determine the size of the local VPN network.
The WireGuard implementation will have two options that should not have a fixed default value, because they represent two keys (one is preshared, the other is private).
Instead of handling these two options differently in ProtoNewContainer(), this commit adds a new function to PROTO_IMPL: ProtoOptionString().
ProtoOptionString() takes the option's name as argument and returns a heap-allocated string that will be used as value. The function returns NULL when the option doesn't need a randomized value.
This new implementation can be easily compiled and executed without the need for other components to be present.
It relies on standard C functions, aside from stat() which is part of POSIX but available on Windows as well.
There's only one third-party dependency, which is tinydir: a single-file header-only library for traversing directories.
2575 // Address
at_least: At condition size < 1U, the value of size must be at least 1.
cannot_single: At condition size < 1U, the value of size cannot be equal to 0.
dead_error_condition: The condition size < 1U cannot be true.
2576 if (size < 1)
2577 {
CID 287533 (#1 of 1): Logically dead code (DEADCODE)dead_error_line: Execution cannot reach this statement: goto LABEL_ERROR;.
2578 goto LABEL_ERROR;
2579 }
CID 355460 (#1 of 1): Dereference before null check (REVERSE_INULL)check_after_deref: Null-checking p suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
737 if (p == NULL)
738 {
739 return false;
740 }
CID 287561 (#1 of 1): Array compared against 0 (NO_EFFECT)array_null: Comparing an array to null is not useful: src == NULL, since the test will always evaluate as true.
Was src formerly declared as a pointer?
3748 if (cedar == NULL || src == NULL || dst == NULL)
3749 {
3750 return false;
3751 }
(gdb) bt
0 0x00007f43857a5e14 in __GI___pthread_mutex_init (mutex=0x0, mutexattr=0x0) at pthread_mutex_init.c:89
1 0x00007f4385eaaf1b in UnixNewLock () at SoftEtherVPN/src/Mayaqua/Unix.c:1845
2 0x00007f4385e92331 in NewLockMain () at SoftEtherVPN/src/Mayaqua/Object.c:89
3 0x00007f4385e92359 in NewLock () at SoftEtherVPN/src/Mayaqua/Object.c:101
4 0x00007f4385e92765 in NewCounter () at SoftEtherVPN/src/Mayaqua/Object.c:171
5 0x00007f4385e92e76 in NewRef () at SoftEtherVPN/src/Mayaqua/Object.c:339
6 0x00007f4385e76939 in NewSkEx (no_compact=0) at SoftEtherVPN/src/Mayaqua/Memory.c:863
7 0x00007f4385e68c95 in NormalizePathW (
dst=0x7ffe65932940 L"\xd6ff2ffb\xfbf14ce5\xad8669ca\x41998a9c\x5107d62d\x8d2ab3f2\x37ceaad2\xffc947ec\xad8ed8d8\x33e9f2f7\xc05723a9\x843263e3\x5516beb3\x12571e2a\xd81405f3\xf92194fe\xd807aa98\x12835b01\x243185be\x550c7dc3\xfd74170d\x12835b01\x553185be\x550c7dc3\x72be5d74\x80deb1fe\x9bdc06a7\xc19bf1f4\x72be5d74\x80deb1fe\x9bdc06a7\xc19bf174\x894d4018\xc54302b8\x145dc92\x143b3917\x62aa4fb8\x915764b1\xd5e11bef\x9d5fbc5\xb956c25b\x59f111f1\x923f82a4\xab1c5ed5\x3956c25b\x59f111f1\x923f82a4\xab1c5ed5\xbaeb40", size=2048, src=<optimized out>)
at SoftEtherVPN/src/Mayaqua/FileIO.c:1960
8 0x00007f4385e69188 in ConbinePathW (
dst=0x7ffe65932940 L"\xd6ff2ffb\xfbf14ce5\xad8669ca\x41998a9c\x5107d62d\x8d2ab3f2\x37ceaad2\xffc947ec\xad8ed8d8\x33e9f2f7\xc05723a9\x843263e3\x5516beb3\x12571e2a\xd81405f3\xf92194fe\xd807aa98\x12835b01\x243185be\x550c7dc3\xfd74170d\x12835b01\x553185be\x550c7dc3\x72be5d74\x80deb1fe\x9bdc06a7\xc19bf1f4\x72be5d74\x80deb1fe\x9bdc06a7\xc19bf174\x894d4018\xc54302b8\x145dc92\x143b3917\x62aa4fb8\x915764b1\xd5e11bef\x9d5fbc5\xb956c25b\x59f111f1\x923f82a4\xab1c5ed5\x3956c25b\x59f111f1\x923f82a4\xab1c5ed5\xbaeb40", size=2048,
dirname=0xbace10 L"/root/.local/bin", filename=0x7ffe65932100 L"SoftEtherVPN/build/vpntest") at SoftEtherVPN/src/Mayaqua/FileIO.c:1686
9 0x00007f4385e6af48 in UnixGetExeNameW (name=0x7f4385ede820 <exe_file_name_w> L"/tmp/a.out", size=2048, arg=0xbb5050 L"./vpntest") at SoftEtherVPN/src/Mayaqua/FileIO.c:1401
10 0x00007f4385e6b04b in InitGetExeName (arg=<optimized out>) at SoftEtherVPN/src/Mayaqua/FileIO.c:1367
11 0x00007f4385e7470a in InitMayaqua (memcheck=memcheck@entry=0, debug=debug@entry=1, argc=argc@entry=3, argv=argv@entry=0x7ffe659340e8)
at SoftEtherVPN/src/Mayaqua/Mayaqua.c:456
12 0x0000000000401282 in main (argc=3, argv=0x7ffe659340e8) at SoftEtherVPN/src/vpntest/vpntest.c:259
"2050 LA_DEL_CRL" - this entry appear in logfile when you delete cert from Certificate Revocation List. Thats why need to change it.
"2051 LA_SET_CRL" - this entry must appear in logfile when you edit cert in Certificate Revocation List, but it doesn't happen (perhaps it's a bug)
Since 35200a29ea we build complete installers using CMake, meaning that there's no need for BuildUtil anymore.
MSBuild projects that are not migrated to CMake yet are kept for reference.
This commit also updates BUILD_WINDOWS.md so that it mentions Visual Studio 2019 instead of 2017.
To solve the problem that the escape condition of the loop that tries name resolution in UDP mode was reversed in the keep-alive function of the Internet connection, so the name resolution retry is set to 250 msec interval instead of the normal 60 second interval.
This works for all VPN protocols.
In SessionMain(): for DHCPDISCOVER and DHCPREQUEST frames, write the static IP address (which is retrieved from the user notes) in the SIADDR field of DHCPHEADER.
In VirtualDhcpServer(): for DHCPDISCOVER and DHCPREQUEST frames, read the static IP address from the SIADDR field of DHCPHEADER and assign it to the client.
- When building on Windows XP using Visual Studio 2008, I encountered the following issue.
- I did a fresh install of Windows XP SP3 32-bit, then applied updates including .NET 3.5. Next I installed MS Visual Studio 2008, then updated with sp1. All of this according to the documentation in your readme for building on Windows.
- In file src/BuildUtils/VpnBuilder.cs, there are two "if" statements testing the same thing, which is to determine if it is a 32-bit or 64-bit machine/compiler. But the then and else clauses are reversed, so clearly, one of them is wrong. The result I saw is that the SDK path being used to run RC.exe is left as the NULL string and so it fails to run the RC.exe program.
- This happens early in the build process, building the build utils. The two "if" statements are used to set paths for the Visual Studio VC and SDK directories. Depending on the integer pointer size, it uses different paths in the registry.
- When I looked in the registry on my Windows XP machine, there is no key HKLM\SOFTWARE\Wow6432Node, I have only seen that on 64-bit machines.
- For the fix, I consolidated the two "if" statements into one, the existing statement on line 380 would only set a value for Paths.VisualStudioVCDir (which got set correctly). Now I moved the code for also setting Paths.MicrosoftSDKDir, while reversing the values from the incorrectly coded "if" statement.
- I can understand that under certain circumstances, this issue would not be encountered, but should be easily reproducible when installing a clean system.
error: cannot initialize a variable of type 'wchar_t *' with an lvalue of type 'const wchar_t [4]'
wchar_t *protocol_str = (udp ? L"UDP" : L"TCP");
^ ~~~~~~~~~~~~~~~~~~~~~~~
The "session created" and "session deleted" messages were useful when a single OPENVPN_SERVER object handled multiple UDP sessions.
Now that each session has its own OPENVPN_SERVER object and session creations/deletions are logged by PROTO, the messages are redundant.
In future we will change the OpenVPN implementation so that the multi-session handling code is deleted.
The messages were like this:
OpenVPN Module: The OpenVPN Server Module is starting.
OpenVPN Session 1 (192.168.122.211:47390 -> 0.0.0.0:1194): A new session is created. Protocol: UDP
OpenVPN Session 1 (192.168.122.211:47390 -> 0.0.0.0:1194): Deleting the session.
OpenVPN Module: The OpenVPN Server Module is stopped.
ProtoHandleDatagrams() takes care of deleting a session if marked as halted.
However, the check is performed when a packet for that session is received; that never happens if the remote host doesn't send at least a packet.
This commit fixes the issue by moving the check into the loop that iterates through all sessions.
ProtoOptionsGet command - Lists the options for the specified protocol
Help for command "ProtoOptionsGet"
Purpose:
Lists the options for the specified protocol
Description:
This command can be used to retrieve the options for a specific protocol.
Detailed info (e.g. value type) will be shown.
You can change an option's value with the ProtoOptionsSet command.
Usage:
ProtoOptionsGet [protocol]
Parameters:
protocol - Protocol name.
ProtoOptionsSet command - Sets an option's value for the specified protocol
Help for command "ProtoOptionsSet"
Purpose:
Sets an option's value for the specified protocol
Description:
This command can be used to change an option's value for a specific protocol.
You can retrieve the options using the ProtoOptionsGet command.
To execute this command, you must have VPN Server administrator privileges.
Usage:
ProtoOptionsSet [protocol] [/NAME:option_name] [/VALUE:string/true/false]
Parameters:
protocol - Protocol name.
/NAME - Option name.
/VALUE - Option value. Make sure to write a value that is accepted by the specified protocol!
PROTO_OPTION is a structure that describes an option (who would've guessed?).
It's designed in a way that allows it to occupy as low memory as possible, while providing great flexibility.
The idea is similar to the one implemented in LIST for trivial types, with the difference that PROTO_OPTION doesn't require casting due to the use of union.
The reason why we don't build these two targets is that they're not used.
More specifically: they require proper configuration to work correctly, which is currently missing.
While vpninstall may be worth salvaging, vpnweb is definitely a relict of the past because it relies on ActiveX.
vpndrvinst is the name of the target and thus the default output name, let's use it.
`vpndrvinst.exe" also sounds less fishy than "driver_installer.exe"...
BuildUtil compiles the project as 32 bit and 64 bit, before building the installer package.
64 bit binaries have the "_x64" suffix and are added to the package alongside the 32 bit ones (that have no suffix).
The CMake project compiles the binaries for a single architecture and they have no suffix.
We decided that providing two separate installers is the best solution.
As for the binaries with the "_ia64" suffix: they never existed during the this repository's lifespan.
The MSBuild project built the binary into "src/bin/hamcore", causing it to be added to "hamcore.se2".
As hinted by the name of the file ("vpnsetup_nosign.exe"), it is not signed by BuildUtil, possibly to save time (the setup package is signed).
The CMake project builds the binary in the same directory as the other ones, allowing the setup to install them without the need to build a package.
Previously, the file needed to be present in order for the setup to work.
This commit removes the requirement so that the setup can be ran from the build directory without the need to copy the file (which is now removed from the repository).
The inclusion of the headers is probably a very old leftover, from when OpenSSL was not encapsulated into Mayaqua yet.
In fact, there was a "HAM_C" (defined in vpndrvinst.c) definition check in Mayaqua/Encrypt.h preventing the redefinition of OpenSSL types.
When "VPN_EXE" is defined, Mayaqua.h defines WinMain(), which handles arguments in a special way.
This commit passes "WIN32" to add_executable(), so that WinMain() is used as entry point instead of main().
The use of main() instead of WinMain() was causing service mode not to work due to the "/service" argument being discarded.
Our CMake project used to forcefully create and use two different build directories: "build" and "tmp".
This commit changes the behavior so that only the build directory CMake is ran in is used.
The "configure" script now runs CMake in "build" by default, instead of "tmp".
strtok() and wcstok() are considered unsafe functions.
A segmentation fault caused by the use of strtok() was recently reported.
Co-authored-by: Takuho NAKANO <takotakot@users.noreply.github.com>
The SSTP implementation doesn't provide packet identification, because it's not required: the protocol is identified by the HTTP header it sends to the server.
"ClientOption", as the name implies, is only used in a client context.
The issue was introduced in 235bd07e67. Before that, an unrelated check prevented UnixVLanSetState() from being called in a server context.
SoftEther VPN originally created the NIC in the UP state and never changed it, even when the the client was not connected.
The behavior was changed in 59e1483dbf, which also added the NicDownOnDisconnect option
The option was disabled by default for backwards compatibility with scripts that don't check whether the NIC is down, but it's not ideal.
This commit forces the correct behavior and removes the commands "TUNDownOnDisconnectEnable", "TUNDownOnDisconnectDisable" and "TUNDownOnDisconnectGet".
PortsUDPSet: This command can be used to specify a single or multiple UDP ports the server should listen on. "0" can be specified to disable the UDP listener.
Administrator privileges are required to execute the command.
PortsUDPGet: This command can be used to retrieve the UDP ports the server is listening on.
The two commands replace the functionality that was previously provided by OpenVpnEnable and OpenVpnGet, respectively.
Originally, StrToPortList() returned NULL when it encountered a number equal to 0 or higher than 65535.
This commit adds a new parameter to the function called "limit_range":
- When its value is true, the function retains the original behavior.
- When its value is false, the function doesn't check whether the number is in the network port number range (1-65535).
The change is required because the command to set the UDP ports will allow to remove all ports by specifying "0" as the port number.
Now that Proto supports UDP, the server can handle multiple protocols on each UDP port.
The UDP ports are specified by the "OpenVPN_UdpPortList" configuration setting, because:
- OpenVPN is currently the only UDP protocol supported by SoftEther VPN to allow a custom port number.
- Before Proto was introduced, a unified interface for the protocols didn't exist; each protocol implementation had to create its own listener.
In preparation for the upcoming WireGuard implementation, this commit renames "OpenVPN_UdpPortList" to "PortsUDP", which should clarify that the setting is global.
The change is reflected in the code. Also, the ports are now stored in a LIST rather than a string. The conversion between string and LIST only happens when loading/saving the configuration.
The default UDP ports are now the same as the TCP ones (443, 992, 1194, 5555).
*** CID 358434: Null pointer dereferences (REVERSE_INULL)
/src/Cedar/Proto.c: 451 in ProtoHandleDatagrams()
445 void ProtoHandleDatagrams(UDPLISTENER *listener, LIST *datagrams)
446 {
447 UINT i;
448 HASH_LIST *sessions;
449 PROTO *proto = listener->Param;
450
>>> CID 358434: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "listener" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
451 if (proto == NULL || listener == NULL || datagrams == NULL)
452 {
453 return;
454 }
455
456 sessions = proto->Sessions;
