| Message ID | 20200821062443.69245-1-eric@sparklabs.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] Adds client-auth-pending-extra management functionality. | expand |
Am 21.08.20 um 08:24 schrieb Eric Thorpe: > This allows extra INFO_PRE mesasges to be sent to a client during an > authentication stage. This may be required to send additional challenges, > or allow longer messages to be sent by breaking them up and sending in parts. Could you describe for what feature you need this? If we are extending the protocol for some multiline AUTH_PENDING feature, we should document how this works etc... Arne
Hi Arne, The main scenario this addresses is 2FA authentication which needs to transmit very long responses such as those requiring keys. In these cases, the responses can be upwards of 1500 bytes. Management is restricted (currently) to 256 bytes and the control channel I believe to 1024, however the larger problem is low MTU connections or poorly configured connection with fragmentation problems would refuse to transmit packets of this size over the control channel, so we need the ability to break these up. At the moment I'm experimenting with the following layout which has shown success: client-auth-pending <CID> CR_TEXT:R,C:2 client-auth-pending-extra <CID> CR_TEXT:CN,1:<data> client-auth-pending-extra <CID> CR_TEXT:CN,2:<data> Where flag C=Chunked data and 2 is the number of chunks to expect. Then CN,<chunk number>:<data>. The other issue this addresses is there are scenario where more than one challenge reply is required. A simple example of this is resident key registration, where the username is pulled from the clients certificate, a password is queried, then attestation is queried, and then authentication is queried. client-auth-pending-extra is a bit of a catch-all addition where it effectively opens up the client-auth-pending spec to allow the control channel to be used without having to constantly drop the connection like the current AUTH_DENY method pre-2.5, but without spamming the user with AUTH_PENDING notifications, and allows a connected management script to handle it without adding further complexity internally to OpenVPN. Cheers, Eric --- Eric Thorpe SparkLabs Developer https://www.sparklabs.com https://twitter.com/sparklabs support@sparklabs.com On 22/08/2020 7:12 pm, Arne Schwabe wrote: > Am 21.08.20 um 08:24 schrieb Eric Thorpe: >> This allows extra INFO_PRE mesasges to be sent to a client during an >> authentication stage. This may be required to send additional challenges, >> or allow longer messages to be sent by breaking them up and sending in parts. > Could you describe for what feature you need this? If we are extending > the protocol for some multiline AUTH_PENDING feature, we should document > how this works etc... > > Arne >
Am 24.08.20 um 09:59 schrieb Eric Thorpe: > Hi Arne, > > The main scenario this addresses is 2FA authentication which needs to > transmit very long responses such as those requiring keys. In these > cases, the responses can be upwards of 1500 bytes. Management is > restricted (currently) to 256 bytes and the control channel I believe to > 1024, however the larger problem is low MTU connections or poorly > configured connection with fragmentation problems would refuse to > transmit packets of this size over the control channel, so we need the > ability to break these up. > > At the moment I'm experimenting with the following layout which has > shown success: > > client-auth-pending <CID> CR_TEXT:R,C:2 > client-auth-pending-extra <CID> CR_TEXT:CN,1:<data> > client-auth-pending-extra <CID> CR_TEXT:CN,2:<data> > > Where flag C=Chunked data and 2 is the number of chunks to expect. Then > CN,<chunk number>:<data>. > > The other issue this addresses is there are scenario where more than one > challenge reply is required. A simple example of this is resident key > registration, where the username is pulled from the clients certificate, > a password is queried, then attestation is queried, and then > authentication is queried. Depends on if these should asked sequentially, the you just send another CR_TEXT challenge after the first is answer or in parallel. > client-auth-pending-extra is a bit of a catch-all addition where it > effectively opens up the client-auth-pending spec to allow the control > channel to be used without having to constantly drop the connection like > the current AUTH_DENY method pre-2.5, but without spamming the user with > AUTH_PENDING notifications, and allows a connected management script to > handle it without adding further complexity internally to OpenVPN. CR_TEXT is challenge response text and those clients that currently implement it only allow a single line. I would suggest the following to get the patch in a acceptable state: - Document your authentication method also in management.txt and also what IV_SSO flag indicates this authentication method. - to avoid the 256 byte management limit and multiple commands use maybe the same approach as client-auth that allows a longer frame, you can still limit that to 1024. Otherwise I think this the right direction. Arne
Hi Arne, > - to avoid the 256 byte management limit and multiple commands use maybe > the same approach as client-auth that allows a longer frame, you can > still limit that to 1024. To be clear here, it isn't so much the limitation of the management or control channel, it's situations where a tun-mtu may be set to say 750 for example, meaning the data size can only realistically be about 730 minus INFO_PRE, flags, etc, or OpenVPN or the network will drop the control channel message. I would not recommend making any changes to management or the control channel here, simply just have the ability to send the data in smaller parts. > CR_TEXT is challenge response text and those clients that currently > implement it only allow a single line. I would suggest the following to > get the patch in a acceptable state: > > - Document your authentication method also in management.txt and also > what IV_SSO flag indicates this authentication method. Are you suggesting that I do not use CR_TEXT, and instead introduce a new Challenge Response type (CR_DATA for example)? Cheers, Eric --- Eric Thorpe SparkLabs Developer https://www.sparklabs.com https://twitter.com/sparklabs support@sparklabs.com On 25/08/2020 1:02 am, Arne Schwabe wrote: > Am 24.08.20 um 09:59 schrieb Eric Thorpe: >> Hi Arne, >> >> The main scenario this addresses is 2FA authentication which needs to >> transmit very long responses such as those requiring keys. In these >> cases, the responses can be upwards of 1500 bytes. Management is >> restricted (currently) to 256 bytes and the control channel I believe to >> 1024, however the larger problem is low MTU connections or poorly >> configured connection with fragmentation problems would refuse to >> transmit packets of this size over the control channel, so we need the >> ability to break these up. >> >> At the moment I'm experimenting with the following layout which has >> shown success: >> >> client-auth-pending <CID> CR_TEXT:R,C:2 >> client-auth-pending-extra <CID> CR_TEXT:CN,1:<data> >> client-auth-pending-extra <CID> CR_TEXT:CN,2:<data> >> >> Where flag C=Chunked data and 2 is the number of chunks to expect. Then >> CN,<chunk number>:<data>. >> >> The other issue this addresses is there are scenario where more than one >> challenge reply is required. A simple example of this is resident key >> registration, where the username is pulled from the clients certificate, >> a password is queried, then attestation is queried, and then >> authentication is queried. > Depends on if these should asked sequentially, the you just send > another CR_TEXT challenge after the first is answer or in parallel. > >> client-auth-pending-extra is a bit of a catch-all addition where it >> effectively opens up the client-auth-pending spec to allow the control >> channel to be used without having to constantly drop the connection like >> the current AUTH_DENY method pre-2.5, but without spamming the user with >> AUTH_PENDING notifications, and allows a connected management script to >> handle it without adding further complexity internally to OpenVPN. > CR_TEXT is challenge response text and those clients that currently > implement it only allow a single line. I would suggest the following to > get the patch in a acceptable state: > > - Document your authentication method also in management.txt and also > what IV_SSO flag indicates this authentication method. > > - to avoid the 256 byte management limit and multiple commands use maybe > the same approach as client-auth that allows a longer frame, you can > still limit that to 1024. > > Otherwise I think this the right direction. > > Arne >
Am 25.08.20 um 01:58 schrieb Eric Thorpe: > Hi Arne, > >> - to avoid the 256 byte management limit and multiple commands use maybe >> the same approach as client-auth that allows a longer frame, you can >> still limit that to 1024. > To be clear here, it isn't so much the limitation of the management or > control channel, it's situations where a tun-mtu may be set to say 750 > for example, meaning the data size can only realistically be about 730 > minus INFO_PRE, flags, etc, or OpenVPN or the network will drop the > control channel message. I would not recommend making any changes to > management or the control channel here, simply just have the ability to > send the data in smaller parts. > >> CR_TEXT is challenge response text and those clients that currently >> implement it only allow a single line. I would suggest the following to >> get the patch in a acceptable state: >> >> - Document your authentication method also in management.txt and also >> what IV_SSO flag indicates this authentication method. > Are you suggesting that I do not use CR_TEXT, and instead introduce a > new Challenge Response type (CR_DATA for example)? IV_SSO=crtext is a basically the AUTH_PENDING variant of presenting the user the text of CR_TEXT and responding with the answer. What you doing seems to be different. And if we add something that is different we should it its own IV_SSO flag to announce itself. And if you need "just" a multiline or longer text than current crtext can give to the user, we still need a flag for this new method, so we can signal if the client supports this. I am just asking that if we add new methods to AUTH_PENDING to properly document them. Arne
Hi Arne, I'm happy to resubmit the patch with further documentation to what I have already included with this patch, however I need to know what is likely to be accepted. Per my previous question and example, is it acceptable to keep using CR_TEXT and document the C and CR flags, or, as I think you have hinted, would you prefer I create a fourth INFOPRE spec, for example CR_DATA, and document that instead? It would also be highly appreciated if I could get some indication if this patch is acceptable outside of the requirement for further documentation or if the code needs further changes. Thanks, Eric --- Eric Thorpe SparkLabs Developer https://www.sparklabs.com https://twitter.com/sparklabs support@sparklabs.com On 25/08/2020 5:25 pm, Arne Schwabe wrote: > Am 25.08.20 um 01:58 schrieb Eric Thorpe: >> Hi Arne, >> >>> - to avoid the 256 byte management limit and multiple commands use maybe >>> the same approach as client-auth that allows a longer frame, you can >>> still limit that to 1024. >> To be clear here, it isn't so much the limitation of the management or >> control channel, it's situations where a tun-mtu may be set to say 750 >> for example, meaning the data size can only realistically be about 730 >> minus INFO_PRE, flags, etc, or OpenVPN or the network will drop the >> control channel message. I would not recommend making any changes to >> management or the control channel here, simply just have the ability to >> send the data in smaller parts. >> >>> CR_TEXT is challenge response text and those clients that currently >>> implement it only allow a single line. I would suggest the following to >>> get the patch in a acceptable state: >>> >>> - Document your authentication method also in management.txt and also >>> what IV_SSO flag indicates this authentication method. >> Are you suggesting that I do not use CR_TEXT, and instead introduce a >> new Challenge Response type (CR_DATA for example)? > IV_SSO=crtext is a basically the AUTH_PENDING variant of presenting the > user the text of CR_TEXT and responding with the answer. > > What you doing seems to be different. And if we add something that is > different we should it its own IV_SSO flag to announce itself. And if > you need "just" a multiline or longer text than current crtext can give > to the user, we still need a flag for this new method, so we can signal > if the client supports this. > > I am just asking that if we add new methods to AUTH_PENDING to properly > document them. > > Arne >
Am 26.08.20 um 03:15 schrieb Eric Thorpe: > Hi Arne, > > I'm happy to resubmit the patch with further documentation to what I > have already included with this patch, however I need to know what is > likely to be accepted. > > Per my previous question and example, is it acceptable to keep using > CR_TEXT and document the C and CR flags, or, as I think you have hinted, > would you prefer I create a fourth INFOPRE spec, for example CR_DATA, > and document that instead? Maybe. I am not against the idea of allowing more authentication methods. But I am not sure what you are trying to achieve. > It would also be highly appreciated if I could get some indication if > this patch is acceptable outside of the requirement for further > documentation or if the code needs further changes. Currently I don't understand what authentication method you are trying to implement. It is frustrating for me that you avoid that question. For accepting it is very simple. Either is something everyone using OpenVPN can implement and understand and you are willing to share with OpenVPN. Then the patches can go. If it a special private authentication method and you don't want to share that, then your patches can also be kept private and applied your own branch/client/server. Arne
Hi Arne, The first we are trying to migrate across is U2F - https://www.sparklabs.com/support/kb/article/yubikey-u2f-two-factor-authentication-with-openvpn-and-viscosity/ <https://www.sparklabs.com/support/kb/article/yubikey-u2f-two-factor-authentication-with-openvpn-and-viscosity/> Even though the patches in the above article work for the vast majority, they are a bit of a hack and we want to get away from them as they're still prone to failing on connections with low MTU or fragmentation issues as previously mentioned. All our 2FA supported methods that we want to migrate across to AUTH_PENDING instead of AUTH_FAILED are available here as well if you'd like some further examples of what this would be used for - https://github.com/thesparklabs/openvpn-two-factor-extensions <https://github.com/thesparklabs/openvpn-two-factor-extensions> Anything using this new method would also end up in the above repo. Regards, Eric --- Eric Thorpe SparkLabs Developer https://www.sparklabs.com https://twitter.com/sparklabs support@sparklabs.com On 26/08/2020 6:55 pm, Arne Schwabe wrote: > Am 26.08.20 um 03:15 schrieb Eric Thorpe: >> Hi Arne, >> >> I'm happy to resubmit the patch with further documentation to what I >> have already included with this patch, however I need to know what is >> likely to be accepted. >> >> Per my previous question and example, is it acceptable to keep using >> CR_TEXT and document the C and CR flags, or, as I think you have hinted, >> would you prefer I create a fourth INFOPRE spec, for example CR_DATA, >> and document that instead? > Maybe. I am not against the idea of allowing more authentication > methods. But I am not sure what you are trying to achieve. > >> It would also be highly appreciated if I could get some indication if >> this patch is acceptable outside of the requirement for further >> documentation or if the code needs further changes. > Currently I don't understand what authentication method you are trying > to implement. It is frustrating for me that you avoid that question. > > For accepting it is very simple. Either is something everyone using > OpenVPN can implement and understand and you are willing to share with > OpenVPN. Then the patches can go. If it a special private authentication > method and you don't want to share that, then your patches can also be > kept private and applied your own branch/client/server. > > Arne >
Am 27.08.20 um 01:34 schrieb Eric Thorpe: > Hi Arne, > > The first we are trying to migrate across is U2F - > https://www.sparklabs.com/support/kb/article/yubikey-u2f-two-factor-authentication-with-openvpn-and-viscosity/ > <https://www.sparklabs.com/support/kb/article/yubikey-u2f-two-factor-authentication-with-openvpn-and-viscosity/> Okay makes more sense now and I see that U2F needs larger messages. But especially U2F is important/common enough to properly document how this should be done in detail. The last thing we want is to end up with two different U2F implementation in OpenVPN one from your side and one from us (OpenVPN inc) that are incompatible with each other. So we probably want a IV_SSO=u2f, so the client can signal U2F support and then a documentation what messages are sent back and forth and then I see no problem in merging your patches. > Even though the patches in the above article work for the vast majority, > they are a bit of a hack and we want to get away from them as they're > still prone to failing on connections with low MTU or fragmentation > issues as previously mentioned. > > All our 2FA supported methods that we want to migrate across to > AUTH_PENDING instead of AUTH_FAILED are available here as well if you'd > like some further examples of what this would be used for - > https://github.com/thesparklabs/openvpn-two-factor-extensions > <https://github.com/thesparklabs/openvpn-two-factor-extensions> > > Anything using this new method would also end up in the above repo. If you interested in the SAML/webauth/other web based authentication protocols, here is a document that I am preparing to document that properly (should also be soon on the official openvpn3 repo): https://github.com/schwabe/openvpn3/blob/schwabe/web_auth_spec/doc/webauth.md Arne
diff --git a/doc/management-notes.txt b/doc/management-notes.txt index 61daaf07..74e05414 100644 --- a/doc/management-notes.txt +++ b/doc/management-notes.txt @@ -595,10 +595,10 @@ notification for more info. COMMAND -- client-pending-auth (OpenVPN 2.5 or higher) ---------------------------------------------------- -Instruct OpenVPN server to send AUTH_PENDING and INFO_PRE message +Instruct the OpenVPN server to send AUTH_PENDING and INFO_PRE message to signal a pending authenticating to the client. A pending auth means -that the connecting requires extra authentication like a one time -password or doing a single sign one via web. +that the connection requires extra authentication like a one time +password or doing a single sign on via web. client-pending-auth {CID} {EXTRA} @@ -611,8 +611,8 @@ out of band authentication). Before issuing a client-pending-auth to a client instead of a client-auth/client-deny, the server should check the IV_SSO -environment variable if the method is support. The currently -defined method are crtext for challenge/response using text +environment variable if the method is supported. The currently +defined methods are crtext for challenge/response using text (e.g. TOTP), openurl and proxy_url for opening an URL in the client to continue authentication. A client supporting the first two methods would set @@ -676,7 +676,29 @@ and <challgenge_text> fields are used: <challenge_text>: the challenge text to be shown to the user. +COMMAND -- client-auth-pending-extra (OpenVPN 2.5 or higher) +------------------------------------------------------------- +Instruct the OpenVPN server to send an INFO_PRE message to the client. +This should be used following client-auth-pending to send extra messages +to the client during an authentication stage, or respond to CR_RESPONSE messages +if further challenges are required. + + client-pending-auth-extra {CID} {EXTRA} + +The server will send INFO_PRE,{EXTRA} to the client. +The client is expected to display the extra information to the user. For the +format of EXTRA, see the client-pending-auth section of this document. +For the OpenVPN server this is stateless operation and needs to be +followed by a client-deny/client-auth[-nt] command (that is the result of the +out of band authentication). + +Before issuing a client-pending-auth-extra to a client, the server should +check the IV_SSO environment variable if the method is supported. + + setenv IV_SSO openurl,crtext + +Refer to client-auth-pending for further information. COMMAND -- client-deny (OpenVPN 2.1 or higher) ----------------------------------------------- diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index 898cb3b3..b5a7d0df 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -107,6 +107,8 @@ man_help(void) msg(M_CLIENT, " text R and optional client reason text CR"); msg(M_CLIENT, "client-pending-auth CID MSG : Instruct OpenVPN to send AUTH_PENDING and INFO_PRE msg" " to the client and wait for a final client-auth/client-deny"); + msg(M_CLIENT, "client-pending-auth-extra CID MSG : Instruct OpenVPN to send INFO_PRE msg to the client" + " without AUTH_PENDING. For additional messages"); msg(M_CLIENT, "client-kill CID [M] : Kill client instance CID with message M (def=RESTART)"); msg(M_CLIENT, "env-filter [level] : Set env-var filter level"); #ifdef MANAGEMENT_PF @@ -1040,6 +1042,42 @@ man_client_pending_auth(struct management *man, const char *cid_str, const char } } +/** + * Send additional INFO_PRE information to the client for additional authentication steps + * + * @param man The management interface struct + * @param cid_str The CID in string form + * @param extra The string to be send to the client containing + * the information of the additional steps + */ +static void +man_client_pending_auth_extra(struct management* man, const char* cid_str, const char* extra) +{ + unsigned long cid = 0; + if (parse_cid(cid_str, &cid)) + { + if (man->persist.callback.client_pending_auth_extra) + { + bool ret = (*man->persist.callback.client_pending_auth_extra) + (man->persist.callback.arg, cid, extra); + + if (ret) + { + msg(M_CLIENT, "SUCCESS: client-pending-auth-extra command succeeded"); + } + else + { + msg(M_CLIENT, "SUCCESS: client-pending-auth-extra command failed." + " Extra paramter might be too long"); + } + } + else + { + msg(M_CLIENT, "ERROR: The client-pending-auth-extra command is not supported by the current daemon mode"); + } + } +} + static void man_client_auth(struct management *man, const char *cid_str, const char *kid_str, const bool extra) { @@ -1587,6 +1625,13 @@ man_dispatch_command(struct management *man, struct status_output *so, const cha man_client_pending_auth(man, p[1], p[2]); } } + else if (streq(p[0], "client-pending-auth-extra")) + { + if (man_need(man, p, 2, 0)) + { + man_client_pending_auth_extra(man, p[1], p[2]); + } + } #ifdef MANAGEMENT_PF else if (streq(p[0], "client-pf")) { diff --git a/src/openvpn/manage.h b/src/openvpn/manage.h index 881bfb14..e586f9ca 100644 --- a/src/openvpn/manage.h +++ b/src/openvpn/manage.h @@ -177,6 +177,9 @@ struct management_callback bool (*client_pending_auth) (void *arg, const unsigned long cid, const char *url); + bool (*client_pending_auth_extra) (void* arg, + const unsigned long cid, + const char* url); char *(*get_peer_info) (void *arg, const unsigned long cid); #endif #ifdef MANAGEMENT_PF diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index 13738180..157db302 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3931,6 +3931,22 @@ management_client_pending_auth(void *arg, return false; } +static bool +management_client_pending_auth_extra(void* arg, + const unsigned long cid, + const char* extra) +{ + struct multi_context* m = (struct multi_context*)arg; + struct multi_instance* mi = lookup_by_cid(m, cid); + if (mi) + { + /* sends INFO_PRE message to client */ + bool ret = send_auth_info_pre_message(&mi->context, extra); + multi_schedule_context_wakeup(m, mi); + return ret; + } + return false; +} static bool management_client_auth(void *arg, @@ -4040,6 +4056,7 @@ init_management_callback_multi(struct multi_context *m) cb.kill_by_cid = management_kill_by_cid; cb.client_auth = management_client_auth; cb.client_pending_auth = management_client_pending_auth; + cb.client_pending_auth_extra = management_client_pending_auth_extra; cb.get_peer_info = management_get_peer_info; #endif #ifdef MANAGEMENT_PF diff --git a/src/openvpn/push.c b/src/openvpn/push.c index e0d2eeaf..beb3223b 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -292,8 +292,13 @@ send_auth_pending_messages(struct context *c, const char *extra) { send_control_channel_string(c, "AUTH_PENDING", D_PUSH); - static const char info_pre[] = "INFO_PRE,"; + return send_auth_info_pre_message(c, extra); +} +bool +send_auth_info_pre_message(struct context* c, const char* extra) +{ + static const char info_pre[] = "INFO_PRE,"; size_t len = strlen(extra)+1 + sizeof(info_pre); if (len > PUSH_BUNDLE_SIZE) diff --git a/src/openvpn/push.h b/src/openvpn/push.h index 2faf19a6..e067f8b1 100644 --- a/src/openvpn/push.h +++ b/src/openvpn/push.h @@ -78,6 +78,7 @@ void send_auth_failed(struct context *c, const char *client_reason); * more details on message format */ bool send_auth_pending_messages(struct context *c, const char *extra); +bool send_auth_info_pre_message(struct context* c, const char* extra); void send_restart(struct context *c, const char *kill_msg);
This allows extra INFO_PRE mesasges to be sent to a client during an authentication stage. This may be required to send additional challenges, or allow longer messages to be sent by breaking them up and sending in parts. Signed-off-by: Eric Thorpe <eric@sparklabs.com> --- doc/management-notes.txt | 32 +++++++++++++++++++++++----- src/openvpn/manage.c | 45 ++++++++++++++++++++++++++++++++++++++++ src/openvpn/manage.h | 3 +++ src/openvpn/multi.c | 17 +++++++++++++++ src/openvpn/push.c | 7 ++++++- src/openvpn/push.h | 1 + 6 files changed, 99 insertions(+), 6 deletions(-)