On 13/06/2019 16:41, Arne Schwabe wrote:
> When signalling the client that it should do Challenge response
> without reconnecting (IV_SSO=crtext/INFOPRE=CR_TEXT), the server
> needs forward the response via the management console.
> 
> Signed-off-by: Arne Schwabe <a...@rfc2549.org>
> ---
>  doc/management-notes.txt | 19 +++++++++++++++++++
>  src/openvpn/forward.c    |  4 ++++
>  src/openvpn/manage.c     | 28 +++++++++++++++++++++++++++-
>  src/openvpn/manage.h     |  4 ++++
>  src/openvpn/push.c       | 21 +++++++++++++++++++++
>  src/openvpn/push.h       |  2 ++
>  6 files changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/management-notes.txt b/doc/management-notes.txt
> index 17645c1d..05d30b0a 100644
> --- a/doc/management-notes.txt
> +++ b/doc/management-notes.txt
> @@ -979,6 +979,24 @@ CLIENT notification types:
>  
>      >CLIENT:ADDRESS,{CID},{ADDR},{PRI}
>  
> +(5) Single Sign On Based Challenge/Response
> +
> +   >CLIENT:CR_RESPONSE,{CID},{KID},{response_base64}
> +   >CLIENT:ENV,name1=val1
> +   >CLIENT:ENV,name2=val2
> +   >CLIENT:ENV,...
> +   >CLIENT:ENV,END
> +   
> +   CR_RESPONSE notifcation. The >CR_RESPONSE fulfils the same purpose as the
> +   CRV1 response in the traditional challenge/response. See that section 
> below for more
> +   details. Since this still uses the same cid as the original response, we 
> do
> +   not use the username and opaque session data in this response.

The difference between CR_RESPONSE and CRV1, is that the CRV1 response is
passed as part of the AUTH_FAILED in the server response and the client
responds back using the password field.  And this causes the client to need to
reconnect to the server, as the AUTH_FAILED disconnects the client.

With this approach the server and client handles the challenge/response
chatter over the control channel; I like this much more as it doesn't
disconnect the client and server - so it will be much faster to handle the
connection setup once the authentication is done.

I see one challenge with this approach, and it is that it locks us to one
specific format for the CR_RESPONSE.  I think it would be appropriate to
extend it with a "version" field before {CID}, so we have a chance to extend
the protocol without updating much of the core OpenVPN 2.x code base.  For
now, it could be hard coded as version 1.

So:  CLIENT:CR_RESPONSE,{VERSION},{CID},{KID},{response_base64}

I'm also wondering if this would be a reasonable approach to use to implement
GSSAPI authentication support as well; where there is a back-and-forth
handshake happening as well.


> diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
> index 2d86dad4..a2b58296 100644
> --- a/src/openvpn/manage.c
> +++ b/src/openvpn/manage.c
[...snip...]
> +management_notify_client_cr_response(unsigned mda_key_id,
> +        const struct man_def_auth_context *mdac,
> +        const struct env_set *es,
> +        const char* response)
> +{
> +    struct gc_arena gc;
> +    if (management)
> +    {
> +        gc = gc_new();
> +
> +        struct buffer out = alloc_buf_gc(256, &gc);
> +        msg(M_CLIENT, ">CLIENT:CR_RESPONSE,%lu,%u,%s",
> +                mdac->cid, mda_key_id, response);
> +        man_output_extra_env(management, "CLIENT");
> +        if (management->connection.env_filter_level>0)

<code_style_police>
It should be spacing around '>'.
</code_style_police>

It would also be good if we could extend these new functions with doxygen
comments on what these functions work.  What information they parse, what they
expect of data, from where the data comes and how they respond/produce results.

Other than that, it looks good.


-- 
kind regards,

David Sommerseth
OpenVPN Inc


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to