Re: [Openvpn-devel] [PATCH 3/3] Document management request >ECDSA_SIGN and response ecdsa-sig

[Selva Nair]

Hi,

FWIW, some remarks on the hash and the signature below.

On Tue, Jan 16, 2018 at 5:23 PM, Arne Schwabe  wrote:
> Am 14.01.18 um 20:44 schrieb selva.n...@gmail.com:
>> From: Selva Nair 
>>
>> Signed-off-by: Selva Nair 
>> ---
>>  doc/management-notes.txt | 30 ++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/doc/management-notes.txt b/doc/management-notes.txt
>> index a9ba18a..e2e8249 100644
>> --- a/doc/management-notes.txt
>> +++ b/doc/management-notes.txt
>> @@ -795,6 +795,36 @@ Base64 encoded output of RSA_private_encrypt() 
>> (OpenSSL) or mbedtls_pk_sign()
>>  This capability is intended to allow the use of arbitrary cryptographic
>>  service providers with OpenVPN via the management interface.
>>
>> +COMMAND -- ecdsa-sig (OpenVPN 2.5 or higher)
>> +--
>> +Same as rsa-sig but for EC keys: requires openssl 1.1
>> +
>> +Provides support for external storage of the EC private key. Requires the
>> +--management-external-key option. This option can be used instead of "key"
>> +in client mode, and allows the client to run without the need to load the
>> +actual private key. When the SSL protocol needs to perform a sign
>> +operation, the data to be signed will be sent to the management interface
>> +via a notification as follows:
>> +
>> +>ECDSA_SIGN:[BASE64_DATA]
>> +
>> +The management interface client should then create a DER encoded signature 
>> of
>> +the (decoded) BASE64_DATA using the private key and return the SSL 
>> signature as
>> +follows:
>> +
>> +ecdsa-sig
>> +[BASE64_SIG_LINE]
>> +.
>> +.
>> +.
>> +END
>> +
>> +Base64 encoded output of ECDSA_sign() (OpenSSL) or mbedtls_pk_sign()
>> +(mbed TLS) will provide a correct signature.
>> +
>
> Signature.getInstance("NONEwithECDSA") worked for me in Java for this.
> Any other signature algorithm did _NOT_ work e.g. SHA384withECDSA. I
> think ecdsa-sign might already provided a hash to sign.

Yes, the hash is what is provided in this signature request. In this
particular case,
the data is handshake history and may not be even saved. Its the accumulated
hash that is passed in by the SSL/TLS library to the callback.

So "NONEwithECDSA" should be the right choice for Java (Sun/Oracle) -- or
ECDSAforSSL in case of IBM Java.

>
> On the other hand mbedtls documentation states:
>
>For RSA, md_alg may be MBEDTLS_MD_NONE if hash_len != 0. For
>ECDSA md_alg may never be MBEDTLS_MD_NONE.

External RSA keys currently work with both openssl and mbedtls.

To elaborate, for RSA, its more complicated as the hash to be encoded
with the OID of the hash algo prepended to it as specified in PKCS1 v1.5,
then padded and signed. Unless the pre TLS1.2  MD5+SHA1 mixed hash is
in use. In the latter case only padding is needed and that's the case where
mbedtls specifies hash type as none.

For RSA, openssl callback gets the encoded hash (not padded) and signing
involves padding + encryption only (hash type is not needed). But mbedtls
provides the hash and hash algorithm and the signing routine has to encode it.
However, the pkcs1 v1.5 encoding part is already present in ssl_mbedtls.c
(implemented by Stefan, I suppose) so what the management interface gets is
the same  data for both openssl and  mbedtls builds as far as RSA is concerned.

>
> So this interface might not work with mbedtls.

It should work if only we could find a way to set a callback for this
in mbedtls.
It seems they provide that facility only for RSA keys. Stefan may know.
Not sure how pkcs11-helper hooks on to it -- does it support EC keys?

Regards,

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/3] Allow external EC key through --management-external-key

[Selva Nair]

Hi,

> On Tue, Jan 16, 2018 at 5:40 PM, Arne Schwabe  wrote:
>> Am 14.01.18 um 20:44 schrieb selva.n...@gmail.com:
>>> From: Selva Nair 
>>>
>>> - This automatically supports EC certificates through
>>>   --management-external-cert
>>> - EC signature request from management has the same format
>>>   as for rsa with >RSA_SIGN replaced by >ECDSA_SIGN
>>>   Response should be of the form 'ecdsa-sig' followed
>>>   by DER encoded signature as base64 followed by 'END'
>>>
>>

snipped..

>>
>>>  static void
>>> +man_ecdsa_sig(struct management *man)
>>> +{
>>> +struct man_connection *mc = >connection;
>>> +if (mc->ext_key_state == EKS_SOLICIT)
>>> +{
>>> +mc->ext_key_state = EKS_INPUT;
>>> +mc->in_extra_cmd = IEC_ECDSA_SIGN;
>>> +in_extra_reset(mc, IER_NEW);
>>> +}
>>> +else
>>> +{
>>> +msg(M_CLIENT, "ERROR: The ecdsa-sig command is not currently 
>>> available");
>>> +}
>>> +}
>>> +
>>
>> This function is almost identical to man_rsa_sign. I would like to have
>> them both combined into one and then called by man_ecdsa_sig/man_rsa_sig.

Refactored code that addresses this and other suggestions is here
https://github.com/selvanair/openvpn/commits/external-ec-cert
(last 3 commits left unsquashed for now).

Will send in v2 after testing and squashing but comments welcome.

Regarding amending --management-external-cert command, better to
address it separately, so not handled here.

Also I'm toying with the idea of renaming ecdsa-sig/ECDSA-SIGN by
pkey-sig/PKEY-SIGN so that eventually we may be able to use it for
all types of keys and retire rsa-sig. Any thoughts on that?

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/3] Allow external EC key through --management-external-key

[Selva Nair]

Hi,

Wow that was fast :)

Thanks for the review. I'll provide a v2, but some quick remarks/rfc

On Tue, Jan 16, 2018 at 5:40 PM, Arne Schwabe  wrote:
> Am 14.01.18 um 20:44 schrieb selva.n...@gmail.com:
>> From: Selva Nair 
>>
>> - This automatically supports EC certificates through
>>   --management-external-cert
>> - EC signature request from management has the same format
>>   as for rsa with >RSA_SIGN replaced by >ECDSA_SIGN
>>   Response should be of the form 'ecdsa-sig' followed
>>   by DER encoded signature as base64 followed by 'END'
>>
>
>
> Thanks for the code. I implemented and tested this on my Android client.
> It works very well. However I found some minor issues (see below).
>
> Tested-By: Arne Schwabe 
>
> But first a high level issue. I think if you enable manange-external-key
> with an "old" ui that only understand rsa keys the OpenVPN client will
> hang after ECDSA prompt since the ui will never reply with anything.
>
> We might need an argument to management-external-key to say "yes I am
> ecdsa ready".

This needs some thought  as config option is not really a way of indicating
client capabilities, or so it seems.

I can see this could pose a problem for clients that currently support
external key.
If there are only a few such known clients we can handle this transparently by
looking at IV_GUI_VER?

>>
>>  static void
>> +man_ecdsa_sig(struct management *man)
>> +{
>> +struct man_connection *mc = >connection;
>> +if (mc->ext_key_state == EKS_SOLICIT)
>> +{
>> +mc->ext_key_state = EKS_INPUT;
>> +mc->in_extra_cmd = IEC_ECDSA_SIGN;
>> +in_extra_reset(mc, IER_NEW);
>> +}
>> +else
>> +{
>> +msg(M_CLIENT, "ERROR: The ecdsa-sig command is not currently 
>> available");
>> +}
>> +}
>> +
>
> This function is almost identical to man_rsa_sign. I would like to have
> them both combined into one and then called by man_ecdsa_sig/man_rsa_sig.

Could combine these into one function named man_external_sig with
an extra parameter, say, cmd = IEC_ECDSA_SIGN or IEC_RSA_SIGN

>
>>
>>
>> +char *
>> +management_query_ecdsa_sig(struct management *man,
>> +   const char *b64_data)
>> +{
>> +return management_query_multiline_flatten(man, b64_data, "ECDSA_SIGN", 
>> "ecdsa-sign",
>> +  
>> >connection.ext_key_state, >connection.ext_key_input);
>> +}
>
> Hm, could be merged with management_query_rsa_sig but since it a one
> liner I have no strong preference.
>
>
>> +#if OPENSSL_VERSION_NUMBER >= 0x1011L && !defined(OPENSSL_NO_EC)
>> +
>> +/* called when EC_KEY is destroyed */
>> +static void
>> +openvpn_extkey_ec_finish(EC_KEY *ec)
>> +{
>> +/* release the method structure */
>> +const EC_KEY_METHOD *ec_meth = EC_KEY_get_method(ec);
>> +EC_KEY_METHOD_free((EC_KEY_METHOD *) ec_meth);
>> +}
>> +
>> +/* EC_KEY_METHOD callback: sign().
>> + * Sign the hash using EC key and return DER encoded signature in sig,
>> + * its length in siglen. Return value is 1 on success, 0 on error.
>> + */
>> +static int
>> +ecdsa_sign(int type, const unsigned char *dgst, int dgstlen, unsigned char 
>> *sig,
>> +   unsigned int *siglen, const BIGNUM *kinv, const BIGNUM *r, 
>> EC_KEY *ec)
>> +{
>> +char *in_b64 = NULL;
>> +char *out_b64 = NULL;
>> +int ret = 0;
>> +int len;
>
> len could just as well be done als inline C99 decleration. But no
> problem. (Also I am not so firm on our new style here)
>
>> +
>> +/* convert 'from' to base64 *
>> +if (openvpn_base64_encode(dgst, dgstlen, _b64) <= 0)
>> +{
>> +goto done;
>> +}
>> +
>> +/* call MI for signature */
>> +if (management)
>> +{
>> +out_b64 = management_query_ecdsa_sig(management, in_b64);
>> +}
>> +if (!out_b64)
>> +{
>> +goto done;
>> +}
>> +
>> +/* decode base64 signature to binary */
>> +len = ECDSA_size(ec);
>> +*siglen = openvpn_base64_decode(out_b64, sig, len);
>> +if (*siglen > 0)
>> +{
>> +ret = 1;
>> +}
>> +
>> +done:
>> +if (in_b64)
>> +{
>> +free(in_b64);
>> +}
>> +if (out_b64)
>> +{
>> +free(out_b64);
>> +}
>> +return ret;
>> +}
>
>
> This method has a bit code duplication to rsa_priv_enc but I think it is
> still okay here since you have an rather ugly looking function that is
> hard to read.

Will give it another try to see whether those can be combined without uglifying
things further.

>
>> +
>> +/* EC_KEY_METHOD callback: sign_setup(). We do no precomputations */
>> +static int
>> +ecdsa_sign_setup(EC_KEY *ec, BN_CTX *ctx_in, BIGNUM **kinvp, BIGNUM **rp)
>> +{
>> +return 1;
>> +}
>> +
>> +/* EC_KEY_METHOD callback: sign_sig().
>> + * Sign the hash and return the result as a newly allocated ECDS_SIG
>> + * struct or NULL on error.
>> + */
>> +static ECDSA_SIG *
>> +ecdsa_sign_sig(const 

Re: [Openvpn-devel] [PATCH 1/3] Refactor ssl_openssl.c in prep for external EC key support

[Arne Schwabe]

Am 14.01.18 um 20:44 schrieb selva.n...@gmail.com:
> From: Selva Nair 
> 
> - Move setting of key method callbacks into a function
> 
> No change in functionality.
> 
>

This patch is fairly simple and does exactly what it says.

Acked-By: Arne Schwabe

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 3/3] Document management request >ECDSA_SIGN and response ecdsa-sig

[Arne Schwabe]

Am 14.01.18 um 20:44 schrieb selva.n...@gmail.com:
> From: Selva Nair 
> 
> Signed-off-by: Selva Nair 
> ---
>  doc/management-notes.txt | 30 ++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/doc/management-notes.txt b/doc/management-notes.txt
> index a9ba18a..e2e8249 100644
> --- a/doc/management-notes.txt
> +++ b/doc/management-notes.txt
> @@ -795,6 +795,36 @@ Base64 encoded output of RSA_private_encrypt() (OpenSSL) 
> or mbedtls_pk_sign()
>  This capability is intended to allow the use of arbitrary cryptographic
>  service providers with OpenVPN via the management interface.
>  
> +COMMAND -- ecdsa-sig (OpenVPN 2.5 or higher)
> +--
> +Same as rsa-sig but for EC keys: requires openssl 1.1
> +
> +Provides support for external storage of the EC private key. Requires the
> +--management-external-key option. This option can be used instead of "key"
> +in client mode, and allows the client to run without the need to load the
> +actual private key. When the SSL protocol needs to perform a sign
> +operation, the data to be signed will be sent to the management interface
> +via a notification as follows:
> +
> +>ECDSA_SIGN:[BASE64_DATA]
> +
> +The management interface client should then create a DER encoded signature of
> +the (decoded) BASE64_DATA using the private key and return the SSL signature 
> as
> +follows:
> +
> +ecdsa-sig
> +[BASE64_SIG_LINE]
> +.
> +.
> +.
> +END
> +
> +Base64 encoded output of ECDSA_sign() (OpenSSL) or mbedtls_pk_sign()
> +(mbed TLS) will provide a correct signature.
> +

Signature.getInstance("NONEwithECDSA") worked for me in Java for this.
Any other signature algorithm did _NOT_ work e.g. SHA384withECDSA. I
think ecdsa-sign might already provided a hash to sign.

On the other hand mbedtls documentation states:

   For RSA, md_alg may be MBEDTLS_MD_NONE if hash_len != 0. For
   ECDSA md_alg may never be MBEDTLS_MD_NONE.

So this interface might not work with mbedtls.

Arne

Acked-By: Arne Schwabe



--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Replace buffer backed strings for management_android_control with simple stack variables

[Arne Schwabe]

This simplifies the code a bit and also silences compiler warnings about 
uint8_t pointers passed to char pointers without cast
---
 src/openvpn/route.c | 14 +++---
 src/openvpn/tun.c   | 12 ++--
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 2bd5845b..6826b4cc 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1610,17 +1610,17 @@ add_route(struct route_ipv4 *r,
 status = openvpn_execve_check(, es, 0, "ERROR: Linux route add 
command failed");
 
 #elif defined (TARGET_ANDROID)
-struct buffer out = alloc_buf_gc(128, );
+char out[128];
 
 if (rgi)
 {
-buf_printf(, "%s %s %s dev %s", network, netmask, gateway, 
rgi->iface);
+snprintf(out, sizeof(out), "%s %s %s dev %s", network, netmask, 
gateway, rgi->iface);
 }
 else
 {
-buf_printf(, "%s %s %s", network, netmask, gateway);
+snprintf(out, sizeof(out), "%s %s %s", network, netmask, gateway);
 }
-management_android_control(management, "ROUTE", buf_bptr());
+management_android_control(management, "ROUTE", out);
 
 #elif defined (_WIN32)
 {
@@ -1963,11 +1963,11 @@ add_route_ipv6(struct route_ipv6 *r6, const struct 
tuntap *tt, unsigned int flag
 status = openvpn_execve_check(, es, 0, "ERROR: Linux route -6/-A 
inet6 add command failed");
 
 #elif defined (TARGET_ANDROID)
-struct buffer out = alloc_buf_gc(64, );
+char out[64];
 
-buf_printf(, "%s/%d %s", network, r6->netbits, device);
+snprintf(out, sizeof(out), "%s/%d %s", network, r6->netbits, device);
 
-management_android_control(management, "ROUTE6", buf_bptr());
+management_android_control(management, "ROUTE6", out);
 
 #elif defined (_WIN32)
 
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 6e163489..46ed5d0d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1031,12 +1031,12 @@ do_ifconfig(struct tuntap *tt,
 
 if (do_ipv6)
 {
-struct buffer out6 = alloc_buf_gc(64, );
-buf_printf(, "%s/%d", ifconfig_ipv6_local,tt->netbits_ipv6);
-management_android_control(management, 
"IFCONFIG6",buf_bptr());
+char out6[64];
+snprintf(out6, sizeof(out6), "%s/%d", 
ifconfig_ipv6_local,tt->netbits_ipv6);
+management_android_control(management, "IFCONFIG6", out6);
 }
 
-struct buffer out = alloc_buf_gc(64, );
+char out[64];
 
 char *top;
 switch (tt->topology)
@@ -1057,8 +1057,8 @@ do_ifconfig(struct tuntap *tt,
 top = "undef";
 }
 
-buf_printf(, "%s %s %d %s", ifconfig_local, 
ifconfig_remote_netmask, tun_mtu, top);
-management_android_control(management, "IFCONFIG", buf_bptr());
+snprintf(out, sizeof(out), "%s %s %d %s", ifconfig_local, 
ifconfig_remote_netmask, tun_mtu, top);
+management_android_control(management, "IFCONFIG", out);
 
 #elif defined(TARGET_SOLARIS)
 /* Solaris 2.6 (and 7?) cannot set all parameters in one go...
-- 
2.15.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] travis builds

[Selva Nair]

On Tue, Jan 16, 2018 at 3:16 AM, Илья Шипицин  wrote:
>
>
> 2018-01-15 22:28 GMT+05:00 Selva Nair :
>>
>> Hi,
>>
>> Out of the 12 or so jobs we have in the travis build matrix a few
>> sometime fail with "write error" (happened a number of times for some
>> local feature branches). On restarting only the failed jobs, one by
>> one, they succeed indicating possible resource starvation?
>>
>> Is there anything one can do to avoid this?
>>
>> Thanks,
>>
>> Selva
>
> it started to happen after travis-ci image was updated to
> clang-5.0, somehow it does not work well with ccache (that's why
> we removed ccache recently)

Indeed, on removing ccache builds do not error anymore. Thanks.

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] travis builds

[Илья Шипицин]

it started to happen after travis-ci image was updated to clang-5.0,
somehow it does not work well with ccache (that's why we removed ccache
recently)

2018-01-15 22:28 GMT+05:00 Selva Nair :

> Hi,
>
> Out of the 12 or so jobs we have in the travis build matrix a few
> sometime fail with "write error" (happened a number of times for some
> local feature branches). On restarting only the failed jobs, one by
> one, they succeed indicating possible resource starvation?
>
> Is there anything one can do to avoid this?
>
> Thanks,
>
> Selva
>
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel