[Openvpn-devel] [PATCH] Fix various spelling mistakes
From: Jonathan Tooker New patch, omitted changes to copyrights/licenses & changelog. --- Changes.rst | 6 +++--- INSTALL | 2 +- TODO.IPv6 | 6 +++--- configure.ac | 2 +- distro/rpm/openvpn.init.d.rhel| 2 +- distro/rpm/openvpn.init.d.suse| 4 ++-- doc/keying-material-exporter.txt | 2 +- doc/openvpn.8 | 14 +++--- m4/pkg.m4 | 2 +- sample/sample-config-files/client.conf| 2 +- sample/sample-keys/openssl.cnf| 4 ++-- src/openvpn/buffer.c | 2 +- src/openvpn/console.h | 6 +++--- src/openvpn/crypto.h | 2 +- src/openvpn/crypto_backend.h | 2 +- src/openvpn/fragment.c| 2 +- src/openvpn/init.c| 18 +- src/openvpn/mss.c | 2 +- src/openvpn/options.c | 14 +++--- src/openvpn/packet_id.h | 2 +- src/openvpn/route.c | 2 +- src/openvpn/run_command.c | 4 ++-- src/openvpn/socket.c | 12 ++-- src/openvpn/socket.h | 2 +- src/openvpn/ssl.c | 2 +- src/openvpn/ssl_verify_backend.h | 2 +- src/openvpn/tun.c | 8 src/openvpn/win32.c | 2 +- src/openvpn/win32.h | 2 +- src/openvpnmsica/msica_op.h | 2 +- src/plugins/auth-pam/README.auth-pam | 4 ++-- src/plugins/auth-pam/utils.h | 6 +++--- src/tapctl/tap.c | 2 +- tests/t_client.sh.in | 2 +- tests/unit_tests/openvpn/test_tls_crypt.c | 2 +- 35 files changed, 75 insertions(+), 75 deletions(-) diff --git a/Changes.rst b/Changes.rst index a7429b11..00dd6ed8 100644 --- a/Changes.rst +++ b/Changes.rst @@ -26,7 +26,7 @@ Seamless client IP/port floating the new format. When a data packet arrives, the server identifies peer by peer-id. If peer's ip/port has changed, server assumes that client has floated, verifies HMAC and updates ip/port in internal structs. -This allows the connection to be immediatly restored, instead of requiring +This allows the connection to be immediately restored, instead of requiring a TLS handshake before the server accepts packets from the new client ip/port. @@ -223,7 +223,7 @@ User-visible Changes of a field get _$N appended to it's field name, starting at N=1. For the example above, that would result in e.g. X509_0_OU=one, X509_0_OU_1=two. Note that this breaks setups that rely on the fact that OpenVPN would - previously (incorrectly) only export the last occurence of a field. + previously (incorrectly) only export the last occurrence of a field. - ``proto udp`` and ``proto tcp`` now use both IPv4 and IPv6. The new options ``proto udp4`` and ``proto tcp4`` use IPv4 only. @@ -371,7 +371,7 @@ Security - CVE-2017-7521: Fix post-authentication remote-triggerable memory leaks A client could cause a server to leak a few bytes each time it connects to the - server. That can eventuall cause the server to run out of memory, and thereby + server. That can eventually cause the server to run out of memory, and thereby causing the server process to terminate. Discovered and reported to the OpenVPN security team by Guido Vranken. (OpenSSL builds only.) diff --git a/INSTALL b/INSTALL index 0f12a636..d0c7dfa6 100644 --- a/INSTALL +++ b/INSTALL @@ -200,7 +200,7 @@ OPTIONS for ./configure: --enable-strict-options enable strict options check between peers (debugging option) [default=no] --enable-selinuxenable SELinux support [default=no] - --enable-systemdenable systemd suppport [default=no] + --enable-systemdenable systemd support [default=no] ENVIRONMENT for ./configure: diff --git a/TODO.IPv6 b/TODO.IPv6 index 24bf865a..465eaa66 100644 --- a/TODO.IPv6 +++ b/TODO.IPv6 @@ -21,7 +21,7 @@ TODO for IPv6 payload support 4.) do "ifconfig tun0 inet6 unplumb" or "ifconfig tun0 destroy" for Solaris, *BSD, ... at program termination time, to clean up leftovers -(unless tunnel persistance is desired). +(unless tunnel persistence is desired). For Solaris, only the "ipv6 tun0" is affected, for the *BSDs all tun0 stay around. @@ -47,7 +47,7 @@ tun0: flags=8051 mtu 1500 4b.) verify this - on FreeBSD, tun0 is auto-destroyed if created by opening /dev/tun (and lingers if created by "ifconfig tun0 create") - -> use for persistant tunnels on not-linux? + -> use for persistent tunnels on not-linux? * 2012-06-10 tun interface behaviour is
Re: [Openvpn-devel] [PATCH applied] Re: Move OpenSSL vs CNG signature digest type mapping to a function
Hi,
I noticed the uncrustify changes only this morning when rebasing the next
patch for a v2. Thanks for taking care of that.
This refactoring is used in the following patch 2/2 (PSS padding needed for
OpenSSL 1.1.1) which Arne is reviewing. If 2.5 is far away, we may want to
make 2.4.x work with openssl 1.1.1. That said, we could continue shipping
2.4.x for Windows built against OpenSSL 1.1.0, so I'm fine with no PSS (and
hence no OpenSSL 1.1.1) support in 2.4.
Selva
On Wed, Jan 23, 2019 at 2:08 PM Gert Doering wrote:
> Your patch has been applied to the master branch.
>
> It needed a bit of massaging due to the uncrustify patches to cryptoapi.c
> that happened in between (f57431cdc88f2), but since the relevant conflicts
> were all "simple whitespace changes", fairly easily solved.
>
> From a quick clance on cryptoapi.c in release/2.4 it looks like this
> might also be applicable - but "by default" I won't apply pure refactoring
> changes. So: if you need this for a change that falls into release/2.4
> land ("long-term compatibility" or "bugfix"), please let me know.
>
> Test built on ubuntu 16.04/mingw.
>
> commit 0cab3475a83e9bad35b0eeb39b9ca886e6afaf1e
> Author: Selva Nair
> Date: Fri Dec 7 14:17:37 2018 -0500
>
> Move OpenSSL vs CNG signature digest type mapping to a function
>
> Signed-off-by: Selva Nair
> Acked-by: Arne Schwabe
> Message-Id: <1544210258-8754-1-git-send-email-selva.n...@gmail.com>
> URL:
> https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17999.html
> Signed-off-by: Gert Doering
>
>
> --
> kind regards,
>
> Gert Doering
>
>
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Fix various spelling mistakes
Hi, On Tue, Jan 22, 2019 at 09:31:03PM -0600, Jonathan Tooker wrote: > Fork @ github: https://github.com/JDTX/openvpn > (76ab12606155f51aaaf376a46f4a52a459af105c) > > From: Jonathan Tooker > Date: Tue, 22 Jan 2019 18:27:39 -0600 > Subject: [PATCH] Fix various spelling mistakes > > Fix spelling mistakes in code/headers/manpages/etc. While I'm always happy to receive spelling/documentation fixes, *this* patch got mangled by your mail client and git refuses to apply it - your mail client removed leading blanks in *some* lines, so pieces like this: > diff --git a/distro/rpm/openvpn.init.d.suse b/distro/rpm/openvpn.init.d.suse > index 270024e8..1b4bcf06 100644 > --- a/distro/rpm/openvpn.init.d.suse > +++ b/distro/rpm/openvpn.init.d.suse > @@ -72,7 +72,7 @@ > #- removed sourcing "network" > #- removed network checking. it seemed not to > work with SuSE. > #- added sourcing "rc.status", comments and > "rc_reset" command > -#- removed "succes; echo" and "failure; echo" > lines > +# - removed "success; echo" and "failure; echo" > lines ... are no longer a valid patch. Can you please re-send with "git send-email", which normally gets things right even though everything out there is intent on massacring mails these days? thanks, 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 g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Move OpenSSL vs CNG signature digest type mapping to a function
Your patch has been applied to the master branch.
It needed a bit of massaging due to the uncrustify patches to cryptoapi.c
that happened in between (f57431cdc88f2), but since the relevant conflicts
were all "simple whitespace changes", fairly easily solved.
>From a quick clance on cryptoapi.c in release/2.4 it looks like this
might also be applicable - but "by default" I won't apply pure refactoring
changes. So: if you need this for a change that falls into release/2.4
land ("long-term compatibility" or "bugfix"), please let me know.
Test built on ubuntu 16.04/mingw.
commit 0cab3475a83e9bad35b0eeb39b9ca886e6afaf1e
Author: Selva Nair
Date: Fri Dec 7 14:17:37 2018 -0500
Move OpenSSL vs CNG signature digest type mapping to a function
Signed-off-by: Selva Nair
Acked-by: Arne Schwabe
Message-Id: <1544210258-8754-1-git-send-email-selva.n...@gmail.com>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg17999.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH 2/2 v2] Handle PSS padding in cryptoapicert
From: Selva Nair
For PSS padding, CNG requires the digest to be signed
and the digest algorithm in use, which are not accessible
via the rsa_sign and rsa_priv_enc callbacks of OpenSSL.
This patch uses the EVP_KEY interface to hook to
evp_pkey_sign callback if OpenSSL version is > 1.1.0.
To test this code path, both the server and client should
be built with OpenSSL 1.1.1 and use TLS version >= 1.2
Tested on Windows 7 client against a Linux server.
Signed-off-by: Selva Nair
---
v2: Changes:
- Meaningless call to EVP_MD_size() removed.
- /** for function doc strings
- Multiline comments: beginning /* and ending */ left blank
- White space in cast as (TYPE)foo and (TYPE *)bar
- Some extra white-space changes suggested by uncrustify
src/openvpn/cryptoapi.c | 267 +---
1 file changed, 252 insertions(+), 15 deletions(-)
diff --git a/src/openvpn/cryptoapi.c b/src/openvpn/cryptoapi.c
index 139845b..0c11712 100644
--- a/src/openvpn/cryptoapi.c
+++ b/src/openvpn/cryptoapi.c
@@ -40,6 +40,7 @@
#ifdef ENABLE_CRYPTOAPI
#include
+#include
#include
#include
#include
@@ -105,6 +106,12 @@ static ERR_STRING_DATA CRYPTOAPI_str_functs[] = {
/* index for storing external data in EC_KEY: < 0 means uninitialized */
static int ec_data_idx = -1;
+/* Global EVP_PKEY_METHOD used to override the sign operation */
+static EVP_PKEY_METHOD *pmethod;
+static int (*default_pkey_sign_init) (EVP_PKEY_CTX *ctx);
+static int (*default_pkey_sign) (EVP_PKEY_CTX *ctx, unsigned char *sig,
+ size_t *siglen, const unsigned char *tbs,
size_t tbslen);
+
typedef struct _CAPI_DATA {
const CERT_CONTEXT *cert_context;
HCRYPTPROV_OR_NCRYPT_KEY_HANDLE crypt_prov;
@@ -177,6 +184,7 @@ cng_hash_algo(int md_type)
case 0:
alg = NULL;
break;
+
default:
msg(M_WARN|M_INFO, "cryptoapicert: Unknown hash type NID=0x%x",
md_type);
break;
@@ -320,28 +328,44 @@ rsa_pub_dec(int flen, const unsigned char *from, unsigned
char *to, RSA *rsa, in
* Sign the hash in 'from' using NCryptSignHash(). This requires an NCRYPT
* key handle in cd->crypt_prov. On return the signature is in 'to'. Returns
* the length of the signature or 0 on error.
+ * This is used only for RSA and padding should be BCRYPT_PAD_PKCS1 or
+ * BCRYPT_PAD_PSS.
* If the hash_algo is not NULL, PKCS #1 DigestInfo header gets added
- * to 'from', else it is signed as is.
- * For now we support only RSA and the padding is assumed to be PKCS1 v1.5
+ * to |from|, else it is signed as is. Use NULL for MD5 + SHA1 hash used
+ * in TLS 1.1 and earlier.
+ * In case of PSS padding, |saltlen| should specify the size of salt to use.
+ * If |to| is NULL returns the required buffer size.
*/
static int
priv_enc_CNG(const CAPI_DATA *cd, const wchar_t *hash_algo, const unsigned
char *from,
- int flen, unsigned char *to, int tlen, DWORD padding)
+ int flen, unsigned char *to, int tlen, DWORD padding, DWORD
saltlen)
{
NCRYPT_KEY_HANDLE hkey = cd->crypt_prov;
DWORD len = 0;
ASSERT(cd->key_spec == CERT_NCRYPT_KEY_SPEC);
-msg(D_LOW, "Signing hash using CNG: data size = %d", flen);
-
-/* The hash OID is already in 'from'. So set the hash algorithm
- * in the padding info struct to NULL.
- */
-BCRYPT_PKCS1_PADDING_INFO padinfo = {hash_algo};
DWORD status;
-status = NCryptSignHash(hkey, padding? : NULL, (BYTE *)from, flen,
-to, tlen, , padding);
+msg(D_LOW, "Signing hash using CNG: data size = %d padding = %lu", flen,
padding);
+
+if (padding == BCRYPT_PAD_PKCS1)
+{
+BCRYPT_PKCS1_PADDING_INFO padinfo = {hash_algo};
+status = NCryptSignHash(hkey, , (BYTE *)from, flen,
+to, tlen, , padding);
+}
+else if (padding == BCRYPT_PAD_PSS)
+{
+BCRYPT_PSS_PADDING_INFO padinfo = {hash_algo, saltlen};
+status = NCryptSignHash(hkey, , (BYTE *)from, flen,
+to, tlen, , padding);
+}
+else
+{
+RSAerr(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT, RSA_R_UNKNOWN_PADDING_TYPE);
+return 0;
+}
+
if (status != ERROR_SUCCESS)
{
SetLastError(status);
@@ -367,16 +391,18 @@ rsa_priv_enc(int flen, const unsigned char *from,
unsigned char *to, RSA *rsa, i
RSAerr(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT, ERR_R_PASSED_NULL_PARAMETER);
return 0;
}
+
if (padding != RSA_PKCS1_PADDING)
{
/* AFAICS, CryptSignHash() *always* uses PKCS1 padding. */
RSAerr(RSA_F_RSA_OSSL_PRIVATE_ENCRYPT, RSA_R_UNKNOWN_PADDING_TYPE);
return 0;
}
+
if (cd->key_spec == CERT_NCRYPT_KEY_SPEC)
{
return priv_enc_CNG(cd, NULL, from, flen, to, RSA_size(rsa),
-cng_padding_type(padding));
+cng_padding_type(padding), 0);
Re: [Openvpn-devel] [PATCH 2/2] Handle PSS padding in cryptoapicert
Hi On Wed, Jan 23, 2019 at 7:55 AM Arne Schwabe wrote: > > > Overall the code looks good. The overriding of the global RSA method is > a bit of a hack but I also do not have any better solution for this. It > might break using OpenSSL engines but that is a corner case that I would > not worry about here. > > Apart from the minor issues this gets an ACK from me. > Thanks for taking time to review this. All good comments and v2 is in the next mail. Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2] Fix various compiler warnings
Am 30.10.18 um 09:53 schrieb Lev Stipakov: > From: Lev Stipakov > > This patch fixes "unused variable/unreferenced format parameter" > warnings in different places, kudos to Visual Studio compiler > for discoveing some of those. > > This also also removes unneeded uninit_management_callback_multi() > wrapper. > This looks good to me, compiles and David already give his ACK on the patch and demanded a small change, which has happend, so: Acked-by: Arne Schwabe signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Fix various spelling mistakes
Am 23.01.19 um 04:31 schrieb Jonathan Tooker: > Fork @ github: https://github.com/JDTX/openvpn > (76ab12606155f51aaaf376a46f4a52a459af105c) > > From: Jonathan Tooker > Date: Tue, 22 Jan 2019 18:27:39 -0600 > Subject: [PATCH] Fix various spelling mistakes > I have gone through all of the correction and they are all correct and some of them are my fault. Acked-By: Arne Schwabe Arne signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Fix various spelling mistakes
Am 23.01.19 um 06:15 schrieb Jonathan Tooker:
> Looks like I missed that and a few others! I fixed some more spelling
> errors across other things. Follow up patch/commit below. If I just need
> to re-make the original patch let me know.
>
> From: Jonathan Tooker
> Date: Tue, 22 Jan 2019 23:10:33 -0600
> Subject: [PATCH] Another set of spelling fixes
>
> ---
> COPYING | 6 +-
> ChangeLog| 140 +++
> Changes.rst | 6 +-
> INSTALL | 2 +-
> TODO.IPv6| 6 +-
> configure.ac | 2 +-
> m4/pkg.m4| 2 +-
> src/openvpn/console.h| 2 +-
> src/openvpn/ssl_verify_backend.h | 2 +-
> 9 files changed, 84 insertions(+), 84 deletions(-)
>
> diff --git a/COPYING b/COPYING
> index 9c21c177..54cc9f24 100644
> --- a/COPYING
> +++ b/COPYING
> @@ -157,7 +157,7 @@ OpenSSL License:
> * The implementation was written so as to conform with Netscapes SSL.
> *
> * This library is free for commercial and non-commercial use as long as
> - * the following conditions are aheared to. The following conditions
> + * the following conditions are adhered to. The following conditions
> * apply to all code found in this distribution, be it the RC4, RSA,
> * lhash, DES, etc., code; not just the SSL code. The SSL documentation
> * included with this distribution is covered by the same copyright terms
> @@ -182,7 +182,7 @@ OpenSSL License:
> *must display the following acknowledgement:
> *"This product includes cryptographic software written by
> * Eric Young (e...@cryptsoft.com)"
> - *The word 'cryptographic' can be left out if the rouines from the
> library
> + *The word 'cryptographic' can be left out if the routines from the
> library
> *being used are not cryptographic related :-).
> * 4. If you include any Windows specific code (or a derivative thereof)
> from
> *the apps directory (application code) you must include an
> acknowledgement:
> @@ -200,7 +200,7 @@ OpenSSL License:
> * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> * SUCH DAMAGE.
> *
> - * The licence and distribution terms for any publically available version or
> + * The licence and distribution terms for any publicly available version or
> * derivative of this code cannot be changed. i.e. this code cannot simply
> be
> * copied and put under another distribution licence
> * [including the GNU Public Licence.]
Yes there are spelling mistakes in the license but changing someone
else's license is an absolute nogo, no matter how wrong it is spelt.
Even the current OpenSSL version (1.1.1a) has the text with the spelling
mistakes in it.
[Changelog entries]
I am not sure about the changelog. Changes.rst can be spell checked but
Changelog also doubles as a list of the git commit, so I feel it should
be the same as the commits.
> diff --git a/src/openvpn/console.h b/src/openvpn/console.h
> index 62beacae..f9481684 100644
> --- a/src/openvpn/console.h
> +++ b/src/openvpn/console.h
> @@ -35,7 +35,7 @@ struct _query_user {
> char *prompt; /**< Prompt to present to the user */
> size_t prompt_len;/**< Length of the prompt string */
> char *response; /**< The user's response */
> -size_t response_len; /**< Length the of the user reposone */
> +size_t response_len; /**< Length the of the user response */
> bool echo;/**< True: The user should see what is being
> typed, otherwise mask it */
> };
>
> diff --git a/src/openvpn/ssl_verify_backend.h
> b/src/openvpn/ssl_verify_backend.h
> index f4cc2c54..d6b31bfa 100644
> --- a/src/openvpn/ssl_verify_backend.h
> +++ b/src/openvpn/ssl_verify_backend.h
> @@ -176,7 +176,7 @@ void x509_setenv(struct env_set *es, int cert_depth,
> openvpn_x509_cert_t *cert);
> *
> * The tracked attributes are stored in ll_head.
> *
> - * @param ll_head The x509_track to store tracked atttributes in
> + * @param ll_head The x509_track to store tracked attributes in
> * @param name Name of the attribute to track
> * @param msglevel Message level for errors
> * @param gcGarbage collection arena for temp data
> --
> 2.20.1.windows.1
>
>
>
> On 1/22/2019 9:51 PM, Simon Matter wrote:
>> Hi,
>>
>>> diff --git a/src/openvpn/console.h b/src/openvpn/console.h
>>> index 0ffd6683..62beacae 100644
>>> --- a/src/openvpn/console.h
>>> +++ b/src/openvpn/console.h
>>> @@ -33,9 +33,9 @@
>>> */
>>> struct _query_user {
>>> char *prompt; /**< Prompt to present to the user */
>>> -size_t prompt_len;/**< Lenght of the prompt string */
>>> +size_t prompt_len;/**< Length of the prompt string */
>>> char *response; /**< The user's response */
>>> -size_t
Re: [Openvpn-devel] [PATCH 2/2] Handle PSS padding in cryptoapicert
Am 07.12.18 um 20:17 schrieb selva.n...@gmail.com:
> From: Selva Nair
>
> For PSS padding, CNG requires the digest to be signed
> and the digest algorithm in use, which are not accessible
> via the rsa_sign and rsa_priv_enc callbacks of OpenSSL.
> This patch uses the EVP_KEY interface to hook to
> evp_pkey_sign callback if OpenSSL version is > 1.1.0.
>
> To test this code path, both the server and client should
> be built with OpenSSL 1.1.1 and use TLS version >= 1.2
>
> Tested on Windows 7 client against a Linux server.
Tested on Windows 10 1809 against a Linux server.
>
> #include
> +#include
> #include
> #include
> #include
> @@ -105,6 +106,12 @@ static ERR_STRING_DATA CRYPTOAPI_str_functs[] = {
> /* index for storing external data in EC_KEY: < 0 means uninitialized */
> static int ec_data_idx = -1;
>
> +/* Global EVP_PKEY_METHOD used to override the sign operation */
> +static EVP_PKEY_METHOD *pmethod;
> +static int (*default_pkey_sign_init) (EVP_PKEY_CTX *ctx);
> +static int (*default_pkey_sign) (EVP_PKEY_CTX *ctx, unsigned char *sig,
> +size_t *siglen, const unsigned char *tbs, size_t tbslen);
> +
> typedef struct _CAPI_DATA {
> const CERT_CONTEXT *cert_context;
> HCRYPTPROV_OR_NCRYPT_KEY_HANDLE crypt_prov;
> @@ -318,28 +325,44 @@ rsa_pub_dec(int flen, const unsigned char *from,
> unsigned char *to, RSA *rsa, in
> * Sign the hash in 'from' using NCryptSignHash(). This requires an NCRYPT
> * key handle in cd->crypt_prov. On return the signature is in 'to'. Returns
> * the length of the signature or 0 on error.
> + * This is used only for RSA and padding should be BCRYPT_PAD_PKCS1 or
> + * BCRYPT_PAD_PSS.
> * If the hash_algo is not NULL, PKCS #1 DigestInfo header gets added
> - * to 'from', else it is signed as is.
> - * For now we support only RSA and the padding is assumed to be PKCS1 v1.5
> + * to |from|, else it is signed as is. Use NULL for MD5 + SHA1 hash used
> + * in TLS 1.1 and earlier.
> + * In case of PSS padding, |saltlen| should specify the size of salt to use.
> + * If |to| is NULL returns the required buffer size.
> */
If you modify the comments anyway, you might consider making them
documentation of the functions (start with /**). This is true for all
multiline comment and I don't comment further on it.
>
> /* Unfortunately, there is no "CryptSign()" function in CryptoAPI, that
> would
Not 100% sure but I think for multiline comments the first line should
be just /* (empty)
> @@ -444,6 +469,7 @@ rsa_priv_enc(int flen, const unsigned char *from,
> unsigned char *to, RSA *rsa, i
> * NCryptSignHash() is used to sign and it is instructed to add the
> * the PKCS #1 DigestInfo header to |m| unless the hash algorithm is
> * the MD5/SHA1 combination used in TLS 1.1 and earlier versions.
> + * OpenSSL exercises this callback only when padding is PKCS1 v1.5.
> */
> +
> +/* We intercept all sign requests, not just the one's for our key.
> + * Check the key and call the saved OpenSSL method for unknown keys.
> + */
> +if (!pkey || !cd)
> +{
> +if (default_pkey_sign)
> +{
> + return default_pkey_sign(ctx, sig, siglen, tbs, tbslen);
> +}
> +else /* This should not happen */
> +{
> +msg(M_FATAL, "cryptopaicert: Unknown key and no default sign
> operation to fallback on");
> +return -1;
> +}
> +}
> +msg(M_NONFATAL, "cryptoapicert: could not determine the signature
> digest algorithm");
> +RSAerr(RSA_F_PKEY_RSA_SIGN, RSA_R_UNKNOWN_ALGORITHM_TYPE);
> +return -1;
> +}
> +
> +if (tbslen != (size_t) hashlen)
no space after cast
> +{
> +RSAerr(RSA_F_PKEY_RSA_SIGN, RSA_R_INVALID_DIGEST_LENGTH);
> +return -1;
> +}
> +
> +
> +if (saltlen < 0)
> +{
> +RSAerr(RSA_F_PKEY_RSA_SIGN, RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
> +return -1;
> +}
> +msg(D_LOW, "cryptoapicert: PSS padding using saltlen = %d", saltlen);
> +EVP_MD_size(md);
This call here should not have side effect (unless md is nullptr) and
the result is not used.
> +}
> +
> +msg(D_LOW, "cryptoapicert: calling priv_enc_CNG with alg = %ls", alg);
> +*siglen = priv_enc_CNG(cd, alg, tbs, (int) tbslen, sig, *siglen,
> + cng_padding_type(padding), (DWORD) saltlen);
> +
Inconsistent casting, we should always use (cast)variable without space
I am told.
> +/* Keep a copy of the default sign and sign_init methods */
> +
> +#if (OPENSSL_VERSION_NUMBER < 0x1010009fL) /* > version 1.1.0i */
> +/* The function signature is not const-correct in these versions
> */
> +EVP_PKEY_meth_get_sign((EVP_PKEY_METHOD*)default_pmethod,
> _pkey_sign_init,
Style dictates a (EVP_PKEY_METHOD *)
Overall the code looks good. The overriding of the global RSA method is
a bit of a hack but I also do
Re: [Openvpn-devel] [PATCH 1/2] Move OpenSSL vs CNG signature digest type mapping to a function
Am 07.12.18 um 20:17 schrieb selva.n...@gmail.com: > From: Selva Nair > > Also add a function to map OpenSSL padding identifier to > corresponding CNG constant. > > This is to help add support for additional padding > types: only refactoring, no functional changes. > I have no compile and used tested it but the code looks good. Acked-By: Arne Schwabe Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add a warning that we do not officially support LibreSSL
>> I considered that (modulo the sleep 60) and wrote the code to do the >> configure check, but then thought that a not-suppressible warning in the >> logs would be sufficient. I still think it is, but don't mind re-adding >> it to configure.ac if you prefer that. > > As an OpenBSD developer and the maintainer of our OpenVPN port, > I certainly care about building and using OpenVPN with LibreSSL. > I have already provided patches in the past, and yesterday I pushed > compat glue in LibreSSL so that openvpn-2.4.5 can build on OpenBSD > (-current). > > So I'm wondering what would be needed to consider LibreSSL "supported". > > I hear that there are concerns over LibreSSL not being API-compatible > with OpenSSL. As you may have noticed, LibreSSL recently introduced > lots of OpenSSL-1.1+ interfaces. While there is no plan to support the > full OpenSSL-1.1 API (tons of functions were added, not all of them seem > useful...), the intent is to provide what the ecosystem actually needs. > I can probably serve as a bridge between the two projects here. > > If you see other points where I - or anyone else - can help, please > share. :) > With my recent commits that use more OpenSSL 1.1.1 features building with LibreSSL breaks again since it claims to be OpenSSL 2.0 or something ridiculous like that. In my book claiming to support APIs you don't even know is a bad move. With the current situation I would also ACK this patch. I see no good way at the moment for supporting LibreSSL. Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
