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.
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 }
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.
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.
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 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;
As a side effect, the DH parameter is now applied to the TCP server as well.
Previously, the default value was always used, ignoring the one from the configuration.
When a datagram is received, the matching session is looked up in a hash list; if it's not found, a new session is created.
This method allows to use a single UDP port for multiple protocols, as we do with TCP.
Also, each session has its own dedicated thread, used to process the received datagrams and generate the ones that are then sent through the UDP listener.
In addition to guaranteeing constant performance, separate threads also prevent a single one from blocking all sessions.
- An additional parameter is added to IsPacketForMe(), used to specify the protocol type (currently either TCP or UDP).
- SupportedModes() is dropped because it's now redundant.
- IsOk() and EstablishedSessions() are dropped because error checking should be handled by the implementation.
- ProtoImplDetect() now takes a buffer and its size rather than a SOCK, so that it can be used to detect UDP protocols.
- The OpenVPN toggle check is moved to ProtoImplDetect(), so that we don't have to duplicate it once UDP support is implemented.
The PROTO structure is now used to identify the system as a whole, rather than a single protocol. It's stored and initialized in Server.
ProtoCompare(), ProtoAdd() and ProtoDetected() are renamed to make the difference between PROTO and PROTO_IMPL more clear.
ProtoGet() and ProtoNum() are removed because the related list can now be accessed directly by Server.
From https://community.openvpn.net/openvpn/wiki/Openvpn23ManPage:
--block-outside-dns
Block DNS servers on other network adapters to prevent DNS leaks.
This option prevents any application from accessing TCP or UDP port 53 except one inside the tunnel.
It uses Windows Filtering Platform (WFP) and works on Windows Vista or later.
This option is considered unknown on non-Windows platforms and unsupported on Windows XP, resulting in fatal error.
You may want to use --setenv opt or --ignore-unknown-option (not suitable for Windows XP) to ignore said error.
Note that pushing unknown options from server does not trigger fatal errors.
The function has been greatly improved, here are some of the changes:
- The required SESSION (c->Session) parameter is checked correctly: the function returns immediately in case it's NULL. Previously, the function didn't return in case the parameter was NULL; multiple checks were in place, but not in all instances where the parameter was dereferenced.
- The resolved IP address is cached with all proxy types.
- The "RestoreServerNameAndPort" variable is documented.
- The Debug() messages have been improved.
This commit moves the generic (not related to our protocol) proxy stuff from Cedar to Mayaqua, in dedicated files.
The functions are refactored so that they all have the same arguments and follow the same logic.
Dedicated error codes are added, in order to indicate clearly why the function(s) failed.
Coverity Scan detected an out-of-bounds access issue: OvsProcessData() checked whether the payload size was bigger than the size of the buffer, instead of checking whether the entire packet size (payload size + 2 bytes) was, resulting in an out-of-bounds access in case the payload size is bigger than 1998.
This commit also improves the variable names, the comments and adds two Debug() lines.
OvsDecrypt() returns 0 when it fails, resulting in "size" rolling over with an end result of 4294967292.
This commit fixes the issue by checking whether "size" is greater than sizeof(UINT) before performing the subtraction.
The bug was caused by a typo in the StrCpy() call: the source buffer was the same as the destination one, meaning that the function didn't do anything.
- Fixed the RADIUS PEAP client to use the standard TLS versioning.
- Implementation of a function to fix the MAC address of L3 VPN protocol by entering e.g. "MAC: 112233445566" in the "Notes" field of the user information.
- Implementation of a function to fix the virtual MAC address to be assigned to the L3 VPN client as a string attribute from RADIUS server when authentication.
Hardcoded paths are used in log file enumeration such as LogFileList
command or GenerateEraseFileList function to delete old log files when
disk free space is lacking.
Fixes: SoftEtherVPN/SoftEtherVPN#972
If SecureNAT is enabled and the hostname of the server
is longer than 16characters, every NETBIOS name resolution
query triggers the buffer overflow. If the server was built
with stack protection, the process will be killed.
This commit adds a protocol interface to the server, its purpose is to manage TCP connections and the various third-party protocols.
More specifically, ProtoHandleConnection() takes care of exchanging the packets between the local and remote endpoint; the protocol implementation only has to parse them and act accordingly.
The interface knows which protocol is the connection for by calling IsPacketForMe(), a function implemented for each protocol.
A race condition in the DHCP server caused it to offer the same IP address to multiple clients when they connected at the same time, because an offered IP address was considered free until the final step (DHCP_ACK).
This commit introduces a list to keep track of the pending leases created during DHCP_OFFER, so that an IP address is guaranteed to be offered to a single client.
StrCat() appends a string to an already existing string. In order to know where the existing string ends, it uses StrLen() which in turn uses strlen(), a function considered unsafe because it doesn't stop until it finds the null character.
Since the string was allocated but not initialized, StrCat() was either:
- Working correctly.
- Copying only a part of the string.
- Making the program crash via strlen().
The fix consists in using StrCpy(), which starts writing at the beginning of the string.
found by cppcheck
[src/Cedar/DDNS.c:656]: (style) Condition 'ret==NULL' is always true
[src/Cedar/DDNS.c:515] -> [src/Cedar/DDNS.c:640]: (style) The expression 'use_https == false' is always true because 'use_https' and 'false' represent the same value.
[src/Cedar/DDNS.c:516] -> [src/Cedar/DDNS.c:648]: (style) The expression 'no_cert_verify == false' is always true because 'no_cert_verify' and 'false' represent the same value.
[src/Cedar/DDNS.c:816] -> [src/Cedar/DDNS.c:860]: (style) The expression 'no_cert_verify == false' is always true because 'no_cert_verify' and 'false' represent the same value.
[src/Cedar/DDNS.c:530]: (style) Variable 'use_vgs' is assigned a value that is never used.
[src/Cedar/DDNS.c:497]: (style) The function 'DCUpdateNow' is never used.
found by cppcheck
[src/Cedar/Account.c:523]: (style) The function 'GetUserPolicy' is never used.
[src/Cedar/Account.c:163]: (style) The function 'NormalizePolicyName' is never used.
found by cppcheck
[src/Cedar/Command.c:23220] -> [src/Cedar/Command.c:23232]: (style) Variable 'len' is reassigned a value before the old one has been used.
found by cppcheck
[src/Cedar/CM.c:4509]: (style) Variable 'easy' is assigned a value that is never used.
[src/Cedar/CM.c:4547]: (style) Variable 'hub_name' is assigned a value that is never used.
[src/Cedar/CM.c:4609]: (style) Variable 'is_account' is assigned a value that is never used.
[src/Cedar/CM.c:8545]: (style) The function 'CmLoadK' is never used.
To fix the bug of OpenVPN 2.4.6 and particular version of kernel mode TAP driver on Linux, the TAP device must be up after the OpenVPN client is connected. However there is no direct push instruction to do so to OpenVPN client. Therefore we push the dummy IPv4 address (RFC7600) to the OpenVPN client to enforce the TAP driver UP state.
This allows an OpenVPN client to bypass a firewall which is aware of the protocol and is able to block it.
The XOR mask set on the server has to be the same on the client, otherwise it will not be able to connect with certain obfuscation modes.
A special OpenVPN client built with the "XOR patch" is required in order to use this function, because it has never been merged in the official OpenVPN repository.
Two parameters are added to the server configuration: "OpenVPNObfuscationMethod" and "OpenVPNObfuscationMask".
Their value can be retrieved with "OpenVpnObfuscationGet" and set with "OpenVpnObfuscationEnable" in the VPN Command Line Management Utility.
resolve possible null pointer dereference
found by cppcheck
[src/Cedar/Protocol.c:3138] -> [src/Cedar/Protocol.c:3071]: (warning) Either the condition 's!=NULL' is redundant or there is possible null pointer dereference: s.
[src/Cedar/Protocol.c:916]: (style) Variable 'save' is assigned a value that is never used.
[src/Cedar/Protocol.c:6242]: (style) Variable 'size' is assigned a value that is never used.
[src/Cedar/Protocol.c:778]: (style) Variable 'old_disable' is assigned a value that is never used.
[src/Cedar/Protocol.c:1021]: (style) Variable 'save' is assigned a value that is never used.
[src/Cedar/Protocol.c:3708]: (style) Variable 'is_vgc' is assigned a value that is never used.
[src/Cedar/Protocol.c:5785]: (style) Variable 's' is assigned a value that is never used.
[src/Cedar/Protocol.c:6164]: (style) The function 'SocksConnectEx' is never used.
[src/Cedar/Protocol.c:907]: (style) The function 'CompareNodeInfo' is never used.
[src/Cedar/Protocol.c:6968]: (style) The function 'ProxyConnect' is never used.
[src/Cedar/Protocol.c:3986]: (style) The function 'SecureDelete' is never used.
[src/Cedar/Protocol.c:4042]: (style) The function 'SecureEnum' is never used.
[src/Cedar/Protocol.c:4127]: (style) The function 'SecureWrite' is never used.
[src/Cedar/Protocol.c:6463]: (style) The function 'SocksConnect' is never used.
[src/Cedar/Protocol.c:7185]: (style) The function 'TcpConnectEx2' is never used.
[src/Cedar/Protocol.c:7206]: (style) The function 'TcpIpConnect' is never used.
1.0.0.1 is a real IP address, owned by CloudFlare and used for their DNS service.
This commit changes the IP address we push to 192.0.0.8, which is defined in RFC7600 as dummy IPv4 address.
found by cppcheck
[src/Cedar/Proto_IkePacket.c:958]: (style) The function 'IkeNewCertPayload' is never used.
[src/Cedar/Proto_IkePacket.c:942]: (style) The function 'IkeNewCertRequestPayload' is never used.
[src/Cedar/Proto_IkePacket.c:875]: (style) The function 'IkeNewNoticeErrorInvalidExchangeTypePayload' is never used.
[src/Cedar/Proto_IkePacket.c:2542]: (style) The function 'IkeNewSpi' is never used.
[src/Cedar/Proto_IkePacket.c:142]: (style) The function 'IkePhase1CryptIdToKeySize' is never used.
[src/Cedar/Proto_IkePacket.c:157]: (style) The function 'IkePhase2CryptIdToKeySize' is never used.
[src/Cedar/Proto_IkePacket.c:172]: (style) The function 'IkeStrToPhase1CryptId' is never used.
[src/Cedar/Proto_IkePacket.c:187]: (style) The function 'IkeStrToPhase1HashId' is never used.
[src/Cedar/Proto_IkePacket.c:196]: (style) The function 'IkeStrToPhase2CryptId' is never used.
[src/Cedar/Proto_IkePacket.c:211]: (style) The function 'IkeStrToPhase2HashId' is never used.
[src/Cedar/Proto_IkePacket.c:2168]: (style) Condition 'b==NULL' is always true
1. ifdef DEBUG -> defined(_DEBUG) || defined(DEBUG)
In VC++ compilers, the macro is "_DEBUG", not "DEBUG".
2. If set memcheck = true, the program will be vitally slow since it will log all malloc() / realloc() / free() calls to find the cause of memory leak.
For normal debug we set memcheck = false.
Please set memcheck = true if you want to test the cause of memory leaks.
