openvpn-devel Mailing List for OpenVPN

For the record https://github.com/microsoft/binskim/issues/508 On Fri, Dec 31, 2021, 8:35 PM Илья Шипицин <chi...@gm...> wrote: > CETCOMPAT is not supported for ARM. > Regarding other arch I do not have particular opinion, I'm fine with > either props or vcxproj approach > > On Fri, Dec 31, 2021, 5:09 PM Lev Stipakov <lst...@gm...> wrote: > >> Hi, >> >> Sorry for the delay. >> >> 1) Was it really necessary to modify .props? I enabled this via >> Linker->Advanced->CET Shadow Stack Compatible and only .vcxproj files >> got modified. >> >> 2) I think we could enable it for all binaries >> (openvpn/openvpnmsica/openvpnserv/tapctl) for ARM64/WIn32/x64 Release >> configurations. >> >> -Lev >> >> ma 27. jouluk. 2021 klo 11.09 Илья Шипицин (chi...@gm...) >> kirjoitti: >> > >> > gentle ping >> > >> > >> > сб, 16 окт. 2021 г. в 19:15, Ilya Shipitsin <chi...@gm...>: >> >> >> >> found by BinSkim, more details: >> >> >> https://docs.microsoft.com/en-us/cpp/build/reference/cetcompat?view=msvc-160 >> >> >> >> Signed-off-by: Ilya Shipitsin <chi...@gm...> >> >> --- >> >> src/compat/Debug.props | 10 ++++++++++ >> >> src/compat/Release.props | 10 ++++++++++ >> >> src/openvpn/openvpn.vcxproj | 4 ++++ >> >> src/openvpnmsica/openvpnmsica-Debug.props | 10 ++++++++++ >> >> src/openvpnmsica/openvpnmsica-Release.props | 10 ++++++++++ >> >> src/openvpnserv/openvpnserv.vcxproj | 4 ++++ >> >> 6 files changed, 48 insertions(+) >> >> >> >> diff --git a/src/compat/Debug.props b/src/compat/Debug.props >> >> index 31bb9d91..14d7a1f7 100644 >> >> --- a/src/compat/Debug.props >> >> +++ b/src/compat/Debug.props >> >> @@ -17,5 +17,15 @@ >> >> <DebugInformationFormat>EditAndContinue</DebugInformationFormat> >> >> </ClCompile> >> >> </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> <ItemGroup /> >> >> </Project> >> >> \ No newline at end of file >> >> diff --git a/src/compat/Release.props b/src/compat/Release.props >> >> index 50eaa8de..df04ddf2 100644 >> >> --- a/src/compat/Release.props >> >> +++ b/src/compat/Release.props >> >> @@ -22,5 +22,15 @@ >> >> <OptimizeReferences>true</OptimizeReferences> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> <ItemGroup /> >> >> </Project> >> >> \ No newline at end of file >> >> diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj >> >> index 65ee6839..38dd22de 100644 >> >> --- a/src/openvpn/openvpn.vcxproj >> >> +++ b/src/openvpn/openvpn.vcxproj >> >> @@ -158,6 +158,7 @@ >> >> >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> >> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> >> @@ -173,6 +174,7 @@ >> >> >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> >> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> >> >> @@ -204,6 +206,7 @@ >> >> >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> >> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> >> @@ -220,6 +223,7 @@ >> >> >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> >> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> >> >> diff --git a/src/openvpnmsica/openvpnmsica-Debug.props >> b/src/openvpnmsica/openvpnmsica-Debug.props >> >> index 43532cfe..c99346af 100644 >> >> --- a/src/openvpnmsica/openvpnmsica-Debug.props >> >> +++ b/src/openvpnmsica/openvpnmsica-Debug.props >> >> @@ -10,5 +10,15 @@ >> >> <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary> >> >> </ClCompile> >> >> </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> <ItemGroup /> >> >> </Project> >> >> \ No newline at end of file >> >> diff --git a/src/openvpnmsica/openvpnmsica-Release.props >> b/src/openvpnmsica/openvpnmsica-Release.props >> >> index 47727b35..70f82713 100644 >> >> --- a/src/openvpnmsica/openvpnmsica-Release.props >> >> +++ b/src/openvpnmsica/openvpnmsica-Release.props >> >> @@ -11,5 +11,15 @@ >> >> <ControlFlowGuard>Guard</ControlFlowGuard> >> >> </ClCompile> >> >> </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> + <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> >> + <Link> >> >> + <CETCompat>true</CETCompat> >> >> + </Link> >> >> + </ItemDefinitionGroup> >> >> <ItemGroup /> >> >> </Project> >> >> \ No newline at end of file >> >> diff --git a/src/openvpnserv/openvpnserv.vcxproj >> b/src/openvpnserv/openvpnserv.vcxproj >> >> index 5fd7d60b..65d03e3b 100644 >> >> --- a/src/openvpnserv/openvpnserv.vcxproj >> >> +++ b/src/openvpnserv/openvpnserv.vcxproj >> >> @@ -130,6 +130,7 @@ >> >> <Link> >> >> >> <AdditionalDependencies>Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> >> @@ -141,6 +142,7 @@ >> >> <Link> >> >> >> <AdditionalDependencies>legacy_stdio_definitions.lib;Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> >> >> @@ -163,6 +165,7 @@ >> >> <Link> >> >> >> <AdditionalDependencies>Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> >> @@ -174,6 +177,7 @@ >> >> <Link> >> >> >> <AdditionalDependencies>legacy_stdio_definitions.lib;Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> >> <SubSystem>Console</SubSystem> >> >> + <CETCompat>true</CETCompat> >> >> </Link> >> >> </ItemDefinitionGroup> >> >> <ItemDefinitionGroup >> Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> >> >> -- >> >> 2.29.2.windows.2 >> >> >> > _______________________________________________ >> > Openvpn-devel mailing list >> > Ope...@li... >> > https://lists.sourceforge.net/lists/listinfo/openvpn-devel >> >> >> >> -- >> -Lev >> > 

CETCOMPAT is not supported for ARM. Regarding other arch I do not have particular opinion, I'm fine with either props or vcxproj approach On Fri, Dec 31, 2021, 5:09 PM Lev Stipakov <lst...@gm...> wrote: > Hi, > > Sorry for the delay. > > 1) Was it really necessary to modify .props? I enabled this via > Linker->Advanced->CET Shadow Stack Compatible and only .vcxproj files > got modified. > > 2) I think we could enable it for all binaries > (openvpn/openvpnmsica/openvpnserv/tapctl) for ARM64/WIn32/x64 Release > configurations. > > -Lev > > ma 27. jouluk. 2021 klo 11.09 Илья Шипицин (chi...@gm...) > kirjoitti: > > > > gentle ping > > > > > > сб, 16 окт. 2021 г. в 19:15, Ilya Shipitsin <chi...@gm...>: > >> > >> found by BinSkim, more details: > >> > https://docs.microsoft.com/en-us/cpp/build/reference/cetcompat?view=msvc-160 > >> > >> Signed-off-by: Ilya Shipitsin <chi...@gm...> > >> --- > >> src/compat/Debug.props | 10 ++++++++++ > >> src/compat/Release.props | 10 ++++++++++ > >> src/openvpn/openvpn.vcxproj | 4 ++++ > >> src/openvpnmsica/openvpnmsica-Debug.props | 10 ++++++++++ > >> src/openvpnmsica/openvpnmsica-Release.props | 10 ++++++++++ > >> src/openvpnserv/openvpnserv.vcxproj | 4 ++++ > >> 6 files changed, 48 insertions(+) > >> > >> diff --git a/src/compat/Debug.props b/src/compat/Debug.props > >> index 31bb9d91..14d7a1f7 100644 > >> --- a/src/compat/Debug.props > >> +++ b/src/compat/Debug.props > >> @@ -17,5 +17,15 @@ > >> <DebugInformationFormat>EditAndContinue</DebugInformationFormat> > >> </ClCompile> > >> </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> <ItemGroup /> > >> </Project> > >> \ No newline at end of file > >> diff --git a/src/compat/Release.props b/src/compat/Release.props > >> index 50eaa8de..df04ddf2 100644 > >> --- a/src/compat/Release.props > >> +++ b/src/compat/Release.props > >> @@ -22,5 +22,15 @@ > >> <OptimizeReferences>true</OptimizeReferences> > >> </Link> > >> </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> <ItemGroup /> > >> </Project> > >> \ No newline at end of file > >> diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj > >> index 65ee6839..38dd22de 100644 > >> --- a/src/openvpn/openvpn.vcxproj > >> +++ b/src/openvpn/openvpn.vcxproj > >> @@ -158,6 +158,7 @@ > >> > <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> > >> > <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> > >> @@ -173,6 +174,7 @@ > >> > <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> > >> > <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> > >> @@ -204,6 +206,7 @@ > >> > <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> > >> > <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> > >> @@ -220,6 +223,7 @@ > >> > <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> > >> > <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> > >> diff --git a/src/openvpnmsica/openvpnmsica-Debug.props > b/src/openvpnmsica/openvpnmsica-Debug.props > >> index 43532cfe..c99346af 100644 > >> --- a/src/openvpnmsica/openvpnmsica-Debug.props > >> +++ b/src/openvpnmsica/openvpnmsica-Debug.props > >> @@ -10,5 +10,15 @@ > >> <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary> > >> </ClCompile> > >> </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> <ItemGroup /> > >> </Project> > >> \ No newline at end of file > >> diff --git a/src/openvpnmsica/openvpnmsica-Release.props > b/src/openvpnmsica/openvpnmsica-Release.props > >> index 47727b35..70f82713 100644 > >> --- a/src/openvpnmsica/openvpnmsica-Release.props > >> +++ b/src/openvpnmsica/openvpnmsica-Release.props > >> @@ -11,5 +11,15 @@ > >> <ControlFlowGuard>Guard</ControlFlowGuard> > >> </ClCompile> > >> </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> + <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> > >> + <Link> > >> + <CETCompat>true</CETCompat> > >> + </Link> > >> + </ItemDefinitionGroup> > >> <ItemGroup /> > >> </Project> > >> \ No newline at end of file > >> diff --git a/src/openvpnserv/openvpnserv.vcxproj > b/src/openvpnserv/openvpnserv.vcxproj > >> index 5fd7d60b..65d03e3b 100644 > >> --- a/src/openvpnserv/openvpnserv.vcxproj > >> +++ b/src/openvpnserv/openvpnserv.vcxproj > >> @@ -130,6 +130,7 @@ > >> <Link> > >> > <AdditionalDependencies>Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> > >> @@ -141,6 +142,7 @@ > >> <Link> > >> > <AdditionalDependencies>legacy_stdio_definitions.lib;Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> > >> @@ -163,6 +165,7 @@ > >> <Link> > >> > <AdditionalDependencies>Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> > >> @@ -174,6 +177,7 @@ > >> <Link> > >> > <AdditionalDependencies>legacy_stdio_definitions.lib;Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> > >> <SubSystem>Console</SubSystem> > >> + <CETCompat>true</CETCompat> > >> </Link> > >> </ItemDefinitionGroup> > >> <ItemDefinitionGroup > Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> > >> -- > >> 2.29.2.windows.2 > >> > > _______________________________________________ > > Openvpn-devel mailing list > > Ope...@li... > > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > > > > -- > -Lev > 

Hi, Sorry for the delay. 1) Was it really necessary to modify .props? I enabled this via Linker->Advanced->CET Shadow Stack Compatible and only .vcxproj files got modified. 2) I think we could enable it for all binaries (openvpn/openvpnmsica/openvpnserv/tapctl) for ARM64/WIn32/x64 Release configurations. -Lev ma 27. jouluk. 2021 klo 11.09 Илья Шипицин (chi...@gm...) kirjoitti: > > gentle ping > > > сб, 16 окт. 2021 г. в 19:15, Ilya Shipitsin <chi...@gm...>: >> >> found by BinSkim, more details: >> https://docs.microsoft.com/en-us/cpp/build/reference/cetcompat?view=msvc-160 >> >> Signed-off-by: Ilya Shipitsin <chi...@gm...> >> --- >> src/compat/Debug.props | 10 ++++++++++ >> src/compat/Release.props | 10 ++++++++++ >> src/openvpn/openvpn.vcxproj | 4 ++++ >> src/openvpnmsica/openvpnmsica-Debug.props | 10 ++++++++++ >> src/openvpnmsica/openvpnmsica-Release.props | 10 ++++++++++ >> src/openvpnserv/openvpnserv.vcxproj | 4 ++++ >> 6 files changed, 48 insertions(+) >> >> diff --git a/src/compat/Debug.props b/src/compat/Debug.props >> index 31bb9d91..14d7a1f7 100644 >> --- a/src/compat/Debug.props >> +++ b/src/compat/Debug.props >> @@ -17,5 +17,15 @@ >> <DebugInformationFormat>EditAndContinue</DebugInformationFormat> >> </ClCompile> >> </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> <ItemGroup /> >> </Project> >> \ No newline at end of file >> diff --git a/src/compat/Release.props b/src/compat/Release.props >> index 50eaa8de..df04ddf2 100644 >> --- a/src/compat/Release.props >> +++ b/src/compat/Release.props >> @@ -22,5 +22,15 @@ >> <OptimizeReferences>true</OptimizeReferences> >> </Link> >> </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> <ItemGroup /> >> </Project> >> \ No newline at end of file >> diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj >> index 65ee6839..38dd22de 100644 >> --- a/src/openvpn/openvpn.vcxproj >> +++ b/src/openvpn/openvpn.vcxproj >> @@ -158,6 +158,7 @@ >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> @@ -173,6 +174,7 @@ >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> >> @@ -204,6 +206,7 @@ >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> @@ -220,6 +223,7 @@ >> <AdditionalDependencies>Ncrypt.lib;gdi32.lib;ws2_32.lib;wininet.lib;crypt32.lib;iphlpapi.lib;winmm.lib;Fwpuclnt.lib;Rpcrt4.lib;setupapi.lib;Advapi32.lib</AdditionalDependencies> >> <AdditionalLibraryDirectories>$(OPENSSL_HOME)/lib;$(LZO_HOME)/lib;$(PKCS11H_HOME)/lib;%(AdditionalLibraryDirectories)</AdditionalLibraryDirectories> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> >> diff --git a/src/openvpnmsica/openvpnmsica-Debug.props b/src/openvpnmsica/openvpnmsica-Debug.props >> index 43532cfe..c99346af 100644 >> --- a/src/openvpnmsica/openvpnmsica-Debug.props >> +++ b/src/openvpnmsica/openvpnmsica-Debug.props >> @@ -10,5 +10,15 @@ >> <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary> >> </ClCompile> >> </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> <ItemGroup /> >> </Project> >> \ No newline at end of file >> diff --git a/src/openvpnmsica/openvpnmsica-Release.props b/src/openvpnmsica/openvpnmsica-Release.props >> index 47727b35..70f82713 100644 >> --- a/src/openvpnmsica/openvpnmsica-Release.props >> +++ b/src/openvpnmsica/openvpnmsica-Release.props >> @@ -11,5 +11,15 @@ >> <ControlFlowGuard>Guard</ControlFlowGuard> >> </ClCompile> >> </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> + <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> + <Link> >> + <CETCompat>true</CETCompat> >> + </Link> >> + </ItemDefinitionGroup> >> <ItemGroup /> >> </Project> >> \ No newline at end of file >> diff --git a/src/openvpnserv/openvpnserv.vcxproj b/src/openvpnserv/openvpnserv.vcxproj >> index 5fd7d60b..65d03e3b 100644 >> --- a/src/openvpnserv/openvpnserv.vcxproj >> +++ b/src/openvpnserv/openvpnserv.vcxproj >> @@ -130,6 +130,7 @@ >> <Link> >> <AdditionalDependencies>Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'"> >> @@ -141,6 +142,7 @@ >> <Link> >> <AdditionalDependencies>legacy_stdio_definitions.lib;Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'"> >> @@ -163,6 +165,7 @@ >> <Link> >> <AdditionalDependencies>Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'"> >> @@ -174,6 +177,7 @@ >> <Link> >> <AdditionalDependencies>legacy_stdio_definitions.lib;Userenv.lib;Iphlpapi.lib;ntdll.lib;Fwpuclnt.lib;Netapi32.lib;Shlwapi.lib;%(AdditionalDependencies)</AdditionalDependencies> >> <SubSystem>Console</SubSystem> >> + <CETCompat>true</CETCompat> >> </Link> >> </ItemDefinitionGroup> >> <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'"> >> -- >> 2.29.2.windows.2 >> > _______________________________________________ > Openvpn-devel mailing list > Ope...@li... > https://lists.sourceforge.net/lists/listinfo/openvpn-devel -- -Lev 

Not tested beyond "make check", change is trivial enough. Your patch has been applied to the master branch. commit 2b6fcdc0280c9e56c4ce0f348cab7511b809b839 Author: Arne Schwabe Date: Tue Dec 7 18:02:01 2021 +0100 Remove pointless do_init_frame_tls function Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@rf...> URL: https://www.mail-archive.com/ope...@li.../msg23337.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Acked-by: Gert Doering <ge...@gr...> Code looks reasonably, and comes with an extensive test suite - so I did not test "does it configure the same link-mtu for all possible combinations as the code before that". I did test some basic stuff (TLS/BF-CBC/SHA1, TLS/AES-256-GCM, TLS/none/none) and it did produce "different values". BF-CBC matched what the server wanted, with the same config. I have left out the hunk that adds "option.h" to test_misc.c - that looks stray, no change to that test otherwise (maybe it's needed in a later patch, then I'll notice and re-add it). test_crypto.c also has this added, and *there* it's definitely needed :-) As mentioned on IRC, we're losing a few msg(D_MTU_DEBUG, ...) in this process - do we want to bring them back at some point? Your patch has been applied to the master branch. commit 01b7cd44669896d91696e936cc38a2f57bc6081e Author: Arne Schwabe Date: Thu Dec 30 18:21:36 2021 +0100 Rework occ link-mtu calculation Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@rf...> URL: https://www.mail-archive.com/search?l=mid&q=2...@rf... Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Hi, On Thu, Dec 30, 2021 at 07:16:25PM +0100, Steffan Karger wrote: > On 30-12-2021 18:28, Arne Schwabe wrote: > > That BF-CBC seems have an extra 8 bytes that I somehow missed. CBC is a > > odd since it always gives you a multiple of the blocksize (64 bit or 8 > > byte) and if you evenly divide by the blocksize you get an extra block > > just for the padding. I need to reinvestigate that code and send a fixup > > patch for it. > > You probably know this, but for clarity: this is how CBC padding works, > not just for BF. It is easier to trigger with BF though, because of the > smaller (64-bit) block, compared to AES (128-bit block). The comment in the code acknowledges this :-) - but the math seems to be not quite right. We've tested with a few different --mssfix values and BF-CBC + AES-CBC and packets (UDP payload) are consistently up to 8 bytes larger than ordered... 18:34 <@plaisthos> I think I am missing the rounding up to blocksize step 18:35 <@plaisthos> I basically handle the corner that you do NOT round up and get an extra block but I completely forgot the rounding up for all other values 18:40 <@plaisthos> I will look into that CBC thing later, that needs more testing than just writing a small quick fix gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... 

Hi, On 30-12-2021 18:28, Arne Schwabe wrote: > That BF-CBC seems have an extra 8 bytes that I somehow missed. CBC is a > odd since it always gives you a multiple of the blocksize (64 bit or 8 > byte) and if you evenly divide by the blocksize you get an extra block > just for the padding. I need to reinvestigate that code and send a fixup > patch for it. You probably know this, but for clarity: this is how CBC padding works, not just for BF. It is easier to trigger with BF though, because of the smaller (64-bit) block, compared to AES (128-bit block). -Steffan 

Am 30.12.21 um 17:38 schrieb Gert Doering: > I've stared at the code (nice, things get simpler :-) ) and done > a few tests (v4 over v4, v4 over v6, ...) with "--mssfix 1000" and > looked at the resulting MSS values. These are way different from > "master without this" - but arguably, closer to reality than what > we had before. > > Old: BF-CBC, --mssfix 1000 -> MSS 804 / 784 > AES256GCM, --mssfix 1000 -> MSS 856 / 876 > New: BF-CBC, --mssfix 1000 -> MSS 892 / 872 > AES256GCM, --mssfix 1000 -> MSS 904 / 884 > > I have not done more sophisticated counting on whether the new values > are *correct*, but tcpdump claims they are: > > - "AES256GCM, mssfix 1000", "ssh -4 $remote ls -lR" > > 16:55:40.171255 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 > 16:55:40.171267 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 > 16:55:40.171270 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 > > - "AES256GCM, mssfix 1000", "ssh -6 $remote ls -lR" > > 16:57:04.871238 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 > > - IPv6 transport is a also correct (which surprises me a bit, I have > not seen a reference to the underlying protocol, which changes > the "headroom" we have). At least, sometimes... with AES256GCM: > > 16:58:31.681280 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.65297: UDP, length 1000 > 16:58:31.681289 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.65297: UDP, length 1000 > > - over IPv6 transport, with BF, max packets are off by 8 bytes > > 17:13:54.561605 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.28958: UDP, length 1008 > 17:13:54.561760 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.28958: UDP, length 1008 > > (which is closer to what I'd expect, but still weird, why not 1020? > ... my machines do weird stuff here...) That BF-CBC seems have an extra 8 bytes that I somehow missed. CBC is a odd since it always gives you a multiple of the blocksize (64 bit or 8 byte) and if you evenly divide by the blocksize you get an extra block just for the padding. I need to reinvestigate that code and send a fixup patch for it. > That said, the code has been subjected to t_client tests as well, which were > uneventful (there is nothing that excercises TCP in there), as expected. > > > The new frame_calculate_mssfix() looks a bit confused regarding > "do we declare + initialize variables in one or two steps", and > "do we call it 'unsigned' or 'unsigned int'". But this can be > fixed in a followup cleanup patch ;-) Some of those are due trying to not to break the 80 char limit which they would do if we declare them just in one line. > Not sure I understand what the old code does here, looking at > "tun_mtu_dynamic" - this looks like "adjust to discovered MTU", > but the documentation of --mssfix does not speak about this ... so > it will be interesting to revisit this (documentation + option) > eventually and see if/how we want to consider MTU discovery. MTU discovery is only enabled if we also have fragment enabled and then it affects both fragment and mss since they both use the mtu_dynamic variable. Arne 

Use the functions that directly compute the link mtu instead relying on the frame logic. Patch V2: rebase on master Signed-off-by: Arne Schwabe <ar...@rf...> --- src/openvpn/mtu.c | 43 +++++++++ src/openvpn/mtu.h | 11 +++ src/openvpn/options.c | 51 ---------- tests/unit_tests/openvpn/Makefile.am | 6 +- tests/unit_tests/openvpn/test_crypto.c | 128 ++++++++++++++++++++++++- tests/unit_tests/openvpn/test_misc.c | 1 + 6 files changed, 186 insertions(+), 54 deletions(-) diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c index 662984f6..dac0c1de 100644 --- a/src/openvpn/mtu.c +++ b/src/openvpn/mtu.c @@ -138,6 +138,49 @@ frame_calculate_payload_size(const struct frame *frame, const struct options *op return payload_size; } +size_t +calc_options_string_link_mtu(const struct options *o, const struct frame *frame) +{ + unsigned int payload = frame_calculate_payload_size(frame, o); + + /* neither --secret nor TLS mode */ + if (!o->tls_client && !o->tls_server && !o->shared_secret_file) + { + return payload; + } + + struct key_type occ_kt; + + /* o->ciphername might be BF-CBC even though the underlying SSL library + * does not support it. For this reason we workaround this corner case + * by pretending to have no encryption enabled and by manually adding + * the required packet overhead to the MTU computation. + */ + const char* ciphername = o->ciphername; + + unsigned int overhead = 0; + + if (strcmp(o->ciphername, "BF-CBC") == 0) + { + /* none has no overhead, so use this to later add only --auth + * overhead */ + + /* overhead of BF-CBC: 64 bit block size, 64 bit IV size */ + overhead += 64/8 + 64/8; + /* set ciphername to none, so its size does get added in the + * fake_kt and the cipher is not tried to be resolved */ + ciphername = "none"; + } + + /* We pass tlsmode always true here since as we do not need to check if + * the ciphers are actually valid for non tls in occ calucation */ + init_key_type(&occ_kt, ciphername, o->authname, true, false); + + overhead += frame_calculate_protocol_header_size(&occ_kt, o, 0, true); + + return payload + overhead; +} + void frame_finalize(struct frame *frame, bool link_mtu_defined, diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h index ae83d3e7..f6013860 100644 --- a/src/openvpn/mtu.h +++ b/src/openvpn/mtu.h @@ -280,6 +280,17 @@ frame_calculate_protocol_header_size(const struct key_type *kt, unsigned int payload_size, bool occ); +/** + * Calculate the link-mtu to advertise to our peer. The actual value is not + * relevant, because we will possibly perform data channel cipher negotiation + * after this, but older clients will log warnings if we do not supply them the + * value they expect. This assumes that the traditional cipher/auth directives + * in the config match the config of the peer. + */ +size_t +calc_options_string_link_mtu(const struct options *options, + const struct frame *frame); + /* * frame_set_mtu_dynamic and flags */ diff --git a/src/openvpn/options.c b/src/openvpn/options.c index fb049621..2ca24685 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3764,57 +3764,6 @@ pre_connect_restore(struct options *o, struct gc_arena *gc) o->data_channel_crypto_flags = 0; } -/** - * Calculate the link-mtu to advertise to our peer. The actual value is not - * relevant, because we will possibly perform data channel cipher negotiation - * after this, but older clients will log warnings if we do not supply them the - * value they expect. This assumes that the traditional cipher/auth directives - * in the config match the config of the peer. - */ -static size_t -calc_options_string_link_mtu(const struct options *o, const struct frame *frame) -{ - size_t link_mtu = EXPANDED_SIZE(frame); - - if (o->pull || o->mode == MODE_SERVER) - { - struct frame fake_frame = *frame; - struct key_type fake_kt; - - frame_remove_from_extra_frame(&fake_frame, crypto_max_overhead()); - - - /* o->ciphername might be BF-CBC even though the underlying SSL library - * does not support it. For this reason we workaround this corner case - * by pretending to have no encryption enabled and by manually adding - * the required packet overhead to the MTU computation. - */ - const char* ciphername = o->ciphername; - - if (strcmp(o->ciphername, "BF-CBC") == 0) - { - /* none has no overhead, so use this to later add only --auth - * overhead */ - - /* overhead of BF-CBC: 64 bit block size, 64 bit IV size */ - frame_add_to_extra_frame(&fake_frame, 64/8 + 64/8); - /* set ciphername to none, so its size does get added in the - * fake_kt and the cipher is not tried to be resolved */ - ciphername = "none"; - } - - init_key_type(&fake_kt, ciphername, o->authname, true, false); - - crypto_adjust_frame_parameters(&fake_frame, &fake_kt, o->replay, - cipher_kt_mode_ofb_cfb(fake_kt.cipher)); - frame_finalize(&fake_frame, o->ce.link_mtu_defined, o->ce.link_mtu, - o->ce.tun_mtu_defined, o->ce.tun_mtu); - msg(D_MTU_DEBUG, "%s: link-mtu %u -> %d", __func__, (unsigned int) link_mtu, - EXPANDED_SIZE(&fake_frame)); - link_mtu = EXPANDED_SIZE(&fake_frame); - } - return link_mtu; -} /* * Build an options string to represent data channel encryption options. * This string must match exactly between peers. The keysize is checked diff --git a/tests/unit_tests/openvpn/Makefile.am b/tests/unit_tests/openvpn/Makefile.am index 44b77cc5..f681b353 100644 --- a/tests/unit_tests/openvpn/Makefile.am +++ b/tests/unit_tests/openvpn/Makefile.am @@ -46,7 +46,9 @@ crypto_testdriver_SOURCES = test_crypto.c mock_msg.c mock_msg.h \	$(openvpn_srcdir)/crypto_openssl.c \	$(openvpn_srcdir)/otime.c \	$(openvpn_srcdir)/packet_id.c \ -	$(openvpn_srcdir)/platform.c +	$(openvpn_srcdir)/platform.c \ +	$(openvpn_srcdir)/mtu.c \ +	$(openvpn_srcdir)/mss.c packet_id_testdriver_CFLAGS = @TEST_CFLAGS@ \	-I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) @@ -137,4 +139,4 @@ misc_testdriver_SOURCES = test_misc.c mock_msg.c \ mock_get_random.c \	$(openvpn_srcdir)/buffer.c \	$(openvpn_srcdir)/ssl_util.c \ -	$(openvpn_srcdir)/platform.c \ No newline at end of file +	$(openvpn_srcdir)/platform.c diff --git a/tests/unit_tests/openvpn/test_crypto.c b/tests/unit_tests/openvpn/test_crypto.c index 51672f9b..19ce174e 100644 --- a/tests/unit_tests/openvpn/test_crypto.c +++ b/tests/unit_tests/openvpn/test_crypto.c @@ -37,6 +37,7 @@ #include <cmocka.h> #include "crypto.h" +#include "options.h" #include "ssl_backend.h" #include "mock_msg.h" @@ -234,6 +235,130 @@ test_des_encrypt(void **state) free(src2); } +/* This test is in test_crypto as it calls into the functions that calculate + * the crypto overhead */ +static void +test_occ_mtu_calculation(void **state) +{ + struct gc_arena gc = gc_new(); + + struct frame f = { 0 }; + struct options o = { 0 }; + size_t linkmtu; + + /* common defaults */ + o.ce.tun_mtu = 1400; + o.replay = true; + o.ce.proto = PROTO_UDP; + + /* No crypto at all */ + o.ciphername = "none"; + o.authname = "none"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1400); + + /* Static key OCC examples */ + o.shared_secret_file = "not null"; + + /* secret, auth none, cipher none */ + o.ciphername = "none"; + o.authname = "none"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1408); + + /* secret, cipher AES-128-CBC, auth none */ + o.ciphername = "AES-128-CBC"; + o.authname = "none"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1440); + + /* secret, cipher none, auth SHA256 */ + o.ciphername = "none"; + o.authname = "SHA256"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1440); + + /* --secret, cipher BF-CBC, auth SHA1 */ + o.ciphername = "BF-CBC"; + o.authname = "SHA1"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1444); + + /* --secret, cipher BF-CBC, auth SHA1, tcp-client */ + o.ce.proto = PROTO_TCP_CLIENT; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1446); + + o.ce.proto = PROTO_UDP; + + /* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1 */ + o.comp.alg = COMP_ALG_LZO; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1445); + + /* --secret, comp-lzo yes, cipher BF-CBC, auth SHA1, fragment 1200 */ + o.ce.fragment = 1200; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1449); + + o.comp.alg = COMP_ALG_UNDEF; + o.ce.fragment = 0; + + /* TLS mode */ + o.shared_secret_file = NULL; + o.tls_client = true; + o.pull = true; + + /* tls client, cipher AES-128-CBC, auth SHA1, tls-auth*/ + o.authname = "SHA1"; + o.ciphername = "AES-128-CBC"; + o.tls_auth_file = "dummy"; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1457); + + /* tls client, cipher AES-128-CBC, auth SHA1 */ + o.tls_auth_file = NULL; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1457); + + /* tls client, cipher none, auth none */ + o.authname = "none"; + o.ciphername = "none"; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1405); + + /* tls client, auth none, cipher none, no-replay */ + o.replay = false; + + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1401); + + + o.replay = true; + + /* tls client, auth SHA1, cipher AES-256-GCM */ + o.authname = "SHA1"; + o.ciphername = "AES-256-GCM"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1449); + + + /* tls client, auth SHA1, cipher AES-256-GCM, fragment, comp-lzo yes */ + o.comp.alg = COMP_ALG_LZO; + o.ce.fragment = 1200; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1454); + + /* tls client, auth SHA1, cipher AES-256-GCM, fragment, comp-lzo yes, socks */ + o.ce.socks_proxy_server = "socks.example.com"; + linkmtu = calc_options_string_link_mtu(&o, &f); + assert_int_equal(linkmtu, 1464); + + gc_free(&gc); +} int main(void) @@ -243,7 +368,8 @@ main(void) cmocka_unit_test(crypto_translate_cipher_names), cmocka_unit_test(crypto_test_tls_prf), cmocka_unit_test(crypto_test_hmac), - cmocka_unit_test(test_des_encrypt) + cmocka_unit_test(test_des_encrypt), + cmocka_unit_test(test_occ_mtu_calculation) }; #if defined(ENABLE_CRYPTO_OPENSSL) diff --git a/tests/unit_tests/openvpn/test_misc.c b/tests/unit_tests/openvpn/test_misc.c index 70f726d0..867fa1bb 100644 --- a/tests/unit_tests/openvpn/test_misc.c +++ b/tests/unit_tests/openvpn/test_misc.c @@ -37,6 +37,7 @@ #include <cmocka.h> #include "ssl_util.h" +#include "options.h" static void test_compat_lzo_string(void **state) -- 2.33.0 

Hi, On Tue, Dec 07, 2021 at 06:01:59PM +0100, Arne Schwabe wrote: > Use the functions that directly compute the link mtu instead relying on the > frame logic. [..] > --- a/src/openvpn/mtu.c > +++ b/src/openvpn/mtu.c > @@ -61,6 +61,8 @@ frame_calculate_protocol_header_size(const struct key_type *kt, > /* Sum of all the overhead that reduces the usable packet size */ > size_t header_size = 0; > > + bool tlsmode = options->tls_server || options->tls_client; > + > /* A socks proxy adds 10 byte of extra header to each packet */ > if (options->ce.socks_proxy_server && proto_is_udp(options->ce.proto)) > { > @@ -74,7 +76,10 @@ frame_calculate_protocol_header_size(const struct key_type *kt, > } > > /* Add the opcode and peerid */ > - header_size += options->use_peer_id ? 4 : 1; > + if (tlsmode) > + { > + header_size += options->use_peer_id ? 4 : 1; > + } We need a v2 of this - these two hunks are already in master. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... 

I've stared at the code (nice, things get simpler :-) ) and done a few tests (v4 over v4, v4 over v6, ...) with "--mssfix 1000" and looked at the resulting MSS values. These are way different from "master without this" - but arguably, closer to reality than what we had before. Old: BF-CBC, --mssfix 1000 -> MSS 804 / 784 AES256GCM, --mssfix 1000 -> MSS 856 / 876 New: BF-CBC, --mssfix 1000 -> MSS 892 / 872 AES256GCM, --mssfix 1000 -> MSS 904 / 884 I have not done more sophisticated counting on whether the new values are *correct*, but tcpdump claims they are: - "AES256GCM, mssfix 1000", "ssh -4 $remote ls -lR" 16:55:40.171255 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 16:55:40.171267 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 16:55:40.171270 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 - "AES256GCM, mssfix 1000", "ssh -6 $remote ls -lR" 16:57:04.871238 IP 194.97.140.11.51194 > 193.149.48.178.50377: UDP, length 1000 - IPv6 transport is a also correct (which surprises me a bit, I have not seen a reference to the underlying protocol, which changes the "headroom" we have). At least, sometimes... with AES256GCM: 16:58:31.681280 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.65297: UDP, length 1000 16:58:31.681289 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.65297: UDP, length 1000 - over IPv6 transport, with BF, max packets are off by 8 bytes 17:13:54.561605 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.28958: UDP, length 1008 17:13:54.561760 IP6 2001:608:0:814::f000:11.51194 > 2001:608:4::ce:c0f.28958: UDP, length 1008 (which is closer to what I'd expect, but still weird, why not 1020? ... my machines do weird stuff here...) That said, the code has been subjected to t_client tests as well, which were uneventful (there is nothing that excercises TCP in there), as expected. The new frame_calculate_mssfix() looks a bit confused regarding "do we declare + initialize variables in one or two steps", and "do we call it 'unsigned' or 'unsigned int'". But this can be fixed in a followup cleanup patch ;-) Not sure I understand what the old code does here, looking at "tun_mtu_dynamic" - this looks like "adjust to discovered MTU", but the documentation of --mssfix does not speak about this ... so it will be interesting to revisit this (documentation + option) eventually and see if/how we want to consider MTU discovery. Your patch has been applied to the master branch. commit d4458eed0c3cf582f787893f033a19cb4629cd76 Author: Arne Schwabe Date: Tue Dec 14 16:09:01 2021 +0100 Decouple MSS fix calculation from frame calculation Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@rf...> URL: https://www.mail-archive.com/ope...@li.../msg23423.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Acked-by: Gert Doering <ge...@gr...> With the latest comment changes (v2/2) and wiscii's grammar fix these code parts are now much easier to understand. There is not much to actually test yet, as the new functions are not called. I have visually compared with crypto_adjust_frame_parameters() and "it seems to do the smae thing" (if(occ)). The comments in mtu.h are helpful to understand the terminology used regarding "header" and "payload (including compression *header*"). Your patch has been applied to the master branch. commit de018b5e93d6a85f39e5e8ecd3c986320fde8a2f Author: Arne Schwabe Date: Wed Dec 29 17:34:45 2021 +0100 Add helper functions to calculate header/payload sizes Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@rf...> URL: https://www.mail-archive.com/ope...@li.../msg23476.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Your patch has been applied to the master branch. commit 7747e0bcdb92f0a3f0f7b27ca7d97194fb8efa97 Author: Antonio Quartulli Date: Wed Dec 29 18:27:14 2021 +0100 options.c: fix version reported in --cipher warning message Signed-off-by: Antonio Quartulli <a...@un...> Acked-by: Arne Schwabe <ar...@rf...> Message-Id: <202...@un...> URL: https://www.mail-archive.com/ope...@li.../msg23477.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Am 29.12.21 um 18:27 schrieb Antonio Quartulli: > BF-CBC is the default value for the --cipher option in OpenVPN <2.5 > and not <2.6. However, the warning printed to screen talks about > "OpenVPN before 2.6", which is wrong and needs to be fixed. > > Fix message by saying ".. before 2.5" Technically it is still the default value but not added to data-ciphers anymore but that doesn't matter for the message :) Acked-By: Arne Schwabe <ar...@rf...> 

BF-CBC is the default value for the --cipher option in OpenVPN <2.5 and not <2.6. However, the warning printed to screen talks about "OpenVPN before 2.6", which is wrong and needs to be fixed. Fix message by saying ".. before 2.5" Cc: Arne Schwabe <ar...@rf...> Signed-off-by: Antonio Quartulli <a...@un...> --- src/openvpn/options.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index b840b767..6fdcf764 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3120,7 +3120,7 @@ options_postprocess_cipher(struct options *o) * parts of OpenVPN assert that the ciphername is set */ o->ciphername = "BF-CBC"; - msg(M_INFO, "Note: --cipher is not set. OpenVPN versions before 2.6 " + msg(M_INFO, "Note: --cipher is not set. OpenVPN versions before 2.5 " "defaulted to BF-CBC as fallback when cipher negotiation " "failed in this case. If you need this fallback please add " "'--data-ciphers-fallback 'BF-CBC' to your configuration " -- 2.34.1 

These functions are intended to lay the groundwork to later replace the distributed frame calculations and centralise the calculation in one place. Patch v2: clarify that the socks comments is assuming IPv4 and improve other comments Signed-off-by: Arne Schwabe <ar...@rf...> --- src/openvpn/crypto.c | 55 ++++++++++++++++++++++++++++ src/openvpn/crypto.h | 18 +++++++++ src/openvpn/mtu.c | 87 ++++++++++++++++++++++++++++++++++++++++++++ src/openvpn/mtu.h | 54 +++++++++++++++++++++++++++ 4 files changed, 214 insertions(+) diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index dc7ba542..5626e2b6 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -667,6 +667,61 @@ openvpn_decrypt(struct buffer *buf, struct buffer work, return ret; } +unsigned int +calculate_crypto_overhead(const struct key_type *kt, + bool packet_id, + bool packet_id_long_form, + unsigned int payload_size, + bool occ) +{ + unsigned int crypto_overhead = 0; + + /* We always have a packet id, no matter if encrypted or unencrypted */ + if (packet_id) + { + crypto_overhead += packet_id_size(packet_id_long_form); + } + + if (cipher_kt_mode_aead(kt->cipher)) + { + /* For AEAD ciphers, we basically use a stream cipher/CTR for + * the encryption, so no overhead apart from the extra bytes + * we add */ + crypto_overhead += cipher_kt_tag_size(kt->cipher); + + if (occ) + { + /* the frame calculation of old clients adds these to the link-mtu + * even though they are not part of the actual packet */ + crypto_overhead += cipher_kt_iv_size(kt->cipher); + crypto_overhead += cipher_kt_block_size(kt->cipher); + } + } + else + { + if (cipher_defined(kt->cipher)) + { + /* CBC, OFB or CFB mode */ + /* This is a worst case upper bound of needing to add + * a full extra block for padding when the payload + * is exactly a multiple of the block size */ + if (occ || (cipher_kt_mode_cbc(kt->cipher) && + (payload_size % cipher_kt_block_size(kt->cipher) == 0))) + { + crypto_overhead += cipher_kt_block_size(kt->cipher); + } + /* IV is always added (no-iv has been removed a while ago) */ + crypto_overhead += cipher_kt_iv_size(kt->cipher); + } + if (md_defined(kt->digest)) + { + crypto_overhead += md_kt_size(kt->digest); + } + } + + return crypto_overhead; +} + void crypto_adjust_frame_parameters(struct frame *frame, const struct key_type *kt, diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h index ad3543c1..5a67b7ac 100644 --- a/src/openvpn/crypto.h +++ b/src/openvpn/crypto.h @@ -415,6 +415,24 @@ void crypto_adjust_frame_parameters(struct frame *frame, bool packet_id, bool packet_id_long_form); +/** Calculate the maximum overhead that our encryption has + * on a packet. This does not include needed additional buffer size + * + * @param kt Struct with the crypto algorithm to use + * @param packet_id Whether packet_id is used + * @param packet_id_long_form Whether the packet id has the long form + * @param payload_size payload size, only used if occ is false + * @param occ if true calculates the overhead for crypto in the same + * incorrect way as all previous OpenVPN versions did, to + * end up with identical numbers for OCC compatibility + */ +unsigned int +calculate_crypto_overhead(const struct key_type *kt, + bool packet_id, + bool packet_id_long_form, + unsigned int payload_size, + bool occ); + /** Return the worst-case OpenVPN crypto overhead (in bytes) */ unsigned int crypto_max_overhead(void); diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c index 0ab716d7..70ef9599 100644 --- a/src/openvpn/mtu.c +++ b/src/openvpn/mtu.c @@ -35,6 +35,7 @@ #include "integer.h" #include "mtu.h" #include "options.h" +#include "crypto.h" #include "memdbg.h" @@ -51,6 +52,92 @@ alloc_buf_sock_tun(struct buffer *buf, ASSERT(buf_safe(buf, 0)); } +size_t +frame_calculate_protocol_header_size(const struct key_type *kt, + const struct options *options, + unsigned int payload_size, + bool occ) +{ + /* Sum of all the overhead that reduces the usable packet size */ + size_t header_size = 0; + + bool tlsmode = options->tls_server || options->tls_client; + + /* A socks proxy adds 10 byte of extra header to each packet + * (we only support Socks with IPv4, this value is different for IPv6) */ + if (options->ce.socks_proxy_server && proto_is_udp(options->ce.proto)) + { + header_size += 10; + } + + /* TCP stream based packets have a 16 bit length field */ + if (proto_is_tcp(options->ce.proto)) + { + header_size += 2; + } + + /* Add the opcode and peerid */ + if (tlsmode) + { + header_size += options->use_peer_id ? 4 : 1; + } + + /* Add the crypto overhead */ + bool packet_id = options->replay; + bool packet_id_long_form = !tlsmode || cipher_kt_mode_ofb_cfb(kt->cipher); + + /* For figuring out the crypto overhead, we need to the size of the payload + * including all headers that also get encrypted as part of the payload */ + header_size += calculate_crypto_overhead(kt, packet_id, + packet_id_long_form, + payload_size, occ); + return header_size; +} + + +size_t +frame_calculate_payload_overhead(const struct frame *frame, + const struct options *options, + bool extra_tun) +{ + size_t overhead = 0; + + /* This is the overhead of tap device that is not included in the MTU itself + * i.e. Ethernet header that we still need to transmit as part of the + * payload*/ + if (extra_tun) + { + overhead += frame->extra_tun; + } + +#if defined(USE_COMP) + /* v1 Compression schemes add 1 byte header. V2 only adds a header when it + * does not increase the packet length. We ignore the unlikely escaping + * for tap here */ + if (options->comp.alg == COMP_ALG_LZ4 || options->comp.alg == COMP_ALG_STUB + || options->comp.alg == COMP_ALG_LZO) + { + overhead += 1; + } +#endif +#if defined(ENABLE_FRAGMENT) + /* Add the size of the fragment header (uint32_t) */ + if (options->ce.fragment) + { + overhead += 4; + } +#endif + return overhead; +} + +size_t +frame_calculate_payload_size(const struct frame *frame, const struct options *options) +{ + size_t payload_size = options->ce.tun_mtu; + payload_size += frame_calculate_payload_overhead(frame, options, true); + return payload_size; +} + void frame_finalize(struct frame *frame, bool link_mtu_defined, diff --git a/src/openvpn/mtu.h b/src/openvpn/mtu.h index c1148c31..5ad0931f 100644 --- a/src/openvpn/mtu.h +++ b/src/openvpn/mtu.h @@ -221,6 +221,60 @@ void set_mtu_discover_type(socket_descriptor_t sd, int mtu_type, sa_family_t pro int translate_mtu_discover_type_name(const char *name); +/** + * Calculates the size of the payload according to tun-mtu and tap overhead. + * This also includes compression and fragmentation overhead if they are + * enabled. + * + * * [IP][UDP][OPENVPN PROTOCOL HEADER][ **PAYLOAD incl compression header** ] + * @param frame + * @param options + * @return + */ +size_t +frame_calculate_payload_size(const struct frame *frame, + const struct options *options); + +/** + * Calculates the size of the payload overhead according to tun-mtu and + * tap overhead. This all the overhead that is considered part of the payload + * itself. The compression and fragmentation header and extra header from tap + * are considered part of this overhead that increases the payload larger than + * tun-mtu. + * + * * [IP][UDP][OPENVPN PROTOCOL HEADER][ **PAYLOAD incl compression header** ] + * @param frame + * @param options + * @param extra_tun + * @return + */ +size_t +frame_calculate_payload_overhead(const struct frame *frame, + const struct options *options, + bool extra_tun); + +/* forward declaration of key_type */ +struct key_type; + +/** + * Calculates the size of the OpenVPN protocol header. This includes + * the crypto IV/tag/HMAC but does not include the IP encapsulation + * + * + * [IP][UDP][ **OPENVPN PROTOCOL HEADER**][PAYLOAD incl compression header] + * + * @param kt the key_type to use to calculate the crypto overhead + * @param options the options struct to be used to calculate + * @param payload_size the payload size, ignored if occ is true + * @param occ if the calculation should be done for occ compatibility + * @return size of the overhead in bytes + */ +size_t +frame_calculate_protocol_header_size(const struct key_type *kt, + const struct options *options, + unsigned int payload_size, + bool occ); + /* * frame_set_mtu_dynamic and flags */ -- 2.33.0 

Hi, On 29/12/2021 14:34, Gert Doering wrote: > Hi, > > On Tue, Dec 07, 2021 at 01:11:37PM +0100, Antonio Quartulli wrote: >> Signed-off-by: Antonio Quartulli <a...@un...> >> --- >> configure.ac | 32 ++++++++++---------------------- >> 1 file changed, 10 insertions(+), 22 deletions(-) > > I think this should be squashed into the first patch, that adds DCO > support to configure.ac in the first place. > Yeah, this is done already. > Plus, I wonder how we should do this in the first place - if we want > widespread DCO adoption, maybe we should include the necessary header > file into the openvpn community tree, and always compile "with DCO" > on Linux (if libnl is available)? > > Having to fiddle with an extra library dependency *and* "you need this > header file which is found elsewhere" feels extra clumsy... > > Alternatively, maybe linuxdco should just install the header file when > installing the kernel module? How's this typically done for other > kernel modules that expose an API that needs a header file? > Normally the required header is just shipped with the program itself (i.e. check the 'iw' program which comes with its own copy of 'nl80211.h'). Being this API forward compatible, it won't hurt if our header is slightly out-of-date. Ideally, when DCO will be upstream, the header will be installed automatically along with other kernel headers. But we still can't assume they are installed, hence it's just easier to ship the header with the OpenVPN source code. I'd just do that at the end. Regards, -- Antonio Quartulli 

Hi, On Tue, Dec 07, 2021 at 01:11:37PM +0100, Antonio Quartulli wrote: > Signed-off-by: Antonio Quartulli <a...@un...> > --- > configure.ac | 32 ++++++++++---------------------- > 1 file changed, 10 insertions(+), 22 deletions(-) I think this should be squashed into the first patch, that adds DCO support to configure.ac in the first place. Plus, I wonder how we should do this in the first place - if we want widespread DCO adoption, maybe we should include the necessary header file into the openvpn community tree, and always compile "with DCO" on Linux (if libnl is available)? Having to fiddle with an extra library dependency *and* "you need this header file which is found elsewhere" feels extra clumsy... Alternatively, maybe linuxdco should just install the header file when installing the kernel module? How's this typically done for other kernel modules that expose an API that needs a header file? gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... 

Am 29.12.21 um 11:39 schrieb Gert Doering: > Acked-by: Gert Doering <ge...@gr...> > > Looks reasonable. > > (I'm a bit confused that we seem to have lost OpenSSL 3.0 actions - I > dimly remember we had one, or at least a patch for one...) > > Your patch has been applied to the master branch. > We have OpenSSL 3.0 actions for macos. Maybe that is what you are remembering. Arne 

Acked-by: Gert Doering <ge...@gr...> Looks reasonable. (I'm a bit confused that we seem to have lost OpenSSL 3.0 actions - I dimly remember we had one, or at least a patch for one...) Your patch has been applied to the master branch. commit 919d10ad4a51404a41e594c2b8985647ae1d6b3e Author: Arne Schwabe Date: Wed Dec 15 13:34:49 2021 +0100 Make github actions names nicer, include Ubuntu18+OpenSSL 1.0.2 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@rf...> URL: https://www.mail-archive.com/ope...@li.../msg23452.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

From: Camille Guérin <gue...@gm...> Signed-off-by: Camille Guerin <gue...@gm...> Closes: OpenVPN/openvpn#164 --- src/openvpn/options.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index ac13412a..f59d89cb 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -6870,7 +6870,7 @@ add_option(struct options *options, } } } - else if (streq(p[0], "server-ipv6") && p[1] && !p[3]) + else if (streq(p[0], "server-ipv6") && p[1] && !p[2]) { const int lev = M_WARN; struct in6_addr network; @@ -6893,12 +6893,6 @@ add_option(struct options *options, options->server_ipv6_defined = true; options->server_network_ipv6 = network; options->server_netbits_ipv6 = netbits; - - if (p[2]) /* no "nopool" options or similar for IPv6 */ - { - msg(msglevel, "error parsing --server-ipv6: %s is not a recognized flag", p[3]); - goto err; - } } else if (streq(p[0], "server-bridge") && p[1] && p[2] && p[3] && p[4] && !p[5]) { -- 2.26.3 

Am 28.12.21 um 09:26 schrieb yuanxun: > Hi, > > I compile and install ovpn-dco/openvpn-dco on centos7. But it didn't work. > > The main problem is libnl but centos doesn t seem to have 3.4. > > http://www.infradead.org/~tgr/libnl/ > > Can't dco run on centos7 as yet? CentOS 7 is a very old operating system that was initially released 7 years ago. While support for CentOS 7 might come later, we currently focus on modern operating systems. Arne 

Hi, I compile and install ovpn-dco/openvpn-dco on centos7. But it didn't work. The main problem is libnl but centos doesn t seem to have 3.4. http://www.infradead.org/~tgr/libnl/ Can't dco run on centos7 as yet? 

Patch has been applied to the release/2.5 branch. commit e3bac09f6a128260e23d94463862757b576a12c3 Author: Gert Doering Date: Mon Dec 27 21:16:16 2021 +0100 fix Changes.rst errors in 2.5.3 and 2.5.5 announcement Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Selva Nair <sel...@gm...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/search?l=mid&q=2...@gr... Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering 

Acked-By: Selva Nair <sel...@gm...> On Mon, Dec 27, 2021 at 3:17 PM Gert Doering <ge...@gr...> wrote: > > - 2.5.3 had a typo in the CVE ID (CVE-2121-3606 should be -2021-) > - 2.5.5 had windows paths with backslashes, which need to be doubled > > (CVE ID typo also reported by "@attritionorg" in Github PR 165) > > v2: SSL -> ssl, and .cfg -> .cnf > > Signed-off-by: Gert Doering <ge...@gr...> > --- > Changes.rst | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Changes.rst b/Changes.rst > index b6f98d51..4e4f2018 100644 > --- a/Changes.rst > +++ b/Changes.rst > @@ -18,8 +18,8 @@ New features > - Windows build: use CFG and Spectre mitigations on MSVC builds > > - bring back OpenSSL config loading to Windows builds. > - OpenSSL config is loaded from %installdir%\SSL\openssl.cfg > - (typically: c:\program files\openvpn\SSL\openssl.cfg) if it exists. > + OpenSSL config is loaded from %installdir%\\ssl\\openssl.cnf > + (typically: c:\\program files\\openvpn\\ssl\\openssl.cnf) if it exists. > > This is important for some hardware tokens which need special > OpenSSL config for correct operation. Trac #1296 > @@ -102,7 +102,7 @@ Overview of changes in 2.5.3 > ============================ > Bugfixes > -------- > -- CVE-2121-3606 > +- CVE-2021-3606 > see https://community.openvpn.net/openvpn/wiki/SecurityAnnouncements > > OpenVPN windows builds could possibly load OpenSSL Config files from -- Selva