[Openvpn-devel] Implement "status 4" (JSON) for management interface

Message ID 20171110163946.7656-1-fkooman@tuxed.net
State Changes Requested
Headers show
Series [Openvpn-devel] Implement "status 4" (JSON) for management interface | expand

Commit Message

François Kooman Nov. 10, 2017, 5:39 a.m. UTC
Parsing CSV can be cumbersome in depending software, so instead
offer JSON as well using "status 4".

The "status 4" command uses the same keys/values as "status 2"
and "status 3", just formatted as JSON.
---
 src/openvpn/multi.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

Comments

Gert van Dijk Nov. 11, 2017, 4:55 a.m. UTC | #1
Hi François,

This is not a full review, but as just discussed face-to-face, here are
some points from me:

1) Could we also please please enable this for non-management interface
use case? I guess it's just adding the status version number 4 in
option/config validator (and the manpage).
2) With 1), perhaps it would be more clear to use *version* in your
message/description so that it's about the --status-version option, not
the --status option.
3) I believe the output structure is not valid JSON with the "END"
string at the end.

(FYI I'm about to upload a patch to include more documentation on the
--status-version option and to include a new field as well.)

--
Gert

On 10-11-17 17:39, François Kooman wrote:
> Parsing CSV can be cumbersome in depending software, so instead
> offer JSON as well using "status 4".
> 
> The "status 4" command uses the same keys/values as "status 2"
> and "status 3", just formatted as JSON.
> ---
>  src/openvpn/multi.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 96 insertions(+)
> 
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 82a0b9d9..3e82431b 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -1014,6 +1014,102 @@ multi_print_status(struct multi_context *m, struct status_output *so, const int
>  
>              status_printf(so, "END");
>          }
> +        else if (version == 4)
> +        {
> +            /*
> +             * Status file version 4 (JSON)
> +             */
> +            status_printf(so, "{");
> +            status_printf(so, "\"TITLE\": \"%s\",", title_string);
> +            status_printf(so, "\"TIME\": \"%s\",", time_string(now, 0, false, &gc_top));
> +            /* UNIX_TIME added here instead of making it part of TIME as well
> +               as in status 2/3 */
> +            status_printf(so, "\"UNIX_TIME\": %u,", (unsigned int)now);
> +            status_printf(so, "\"CLIENT_LIST\": [");
> +
> +            /* JSON needs a "," between items, but not at the last position,
> +               because I was unable to figure out how to determine if we
> +               iterate over the last item, we inverse it by prepending
> +               additional items with a comma */
> +            int is_first = 1;
> +            hash_iterator_init(m->hash, &hi);
> +            while ((he = hash_iterator_next(&hi)))
> +            {
> +                struct gc_arena gc = gc_new();
> +                const struct multi_instance *mi = (struct multi_instance *) he->value;
> +
> +                if (!mi->halt)
> +                {
> +                    if(0 == is_first) {
> +                        status_printf(so, ",");
> +                    } else {
> +                        is_first = 0;
> +                    }
> +                    status_printf(so, "{");
> +                    status_printf(so, "\"Common Name\": \"%s\",", tls_common_name(mi->context.c2.tls_multi, false));
> +                    status_printf(so, "\"Real Address\": \"%s\",", mroute_addr_print(&mi->real, &gc));
> +                    status_printf(so, "\"Virtual Address\": \"%s\",", print_in_addr_t(mi->reporting_addr, IA_EMPTY_IF_UNDEF, &gc));
> +                    status_printf(so, "\"Virtual IPv6 Address\": \"%s\",", print_in6_addr(mi->reporting_addr_ipv6, IA_EMPTY_IF_UNDEF, &gc));
> +                    status_printf(so, "\"Bytes Received\": " counter_format ",", mi->context.c2.link_read_bytes);
> +                    status_printf(so, "\"Bytes Sent\": " counter_format ",", mi->context.c2.link_write_bytes);
> +                    status_printf(so, "\"Connected Since\": \"%s\",", time_string(mi->created, 0, false, &gc));
> +                    status_printf(so, "\"Connected Since (time_t)\": %u,", (unsigned int)mi->created);
> +                    status_printf(so, "\"Username\": \"%s\",", tls_username(mi->context.c2.tls_multi, false));
> +#ifdef MANAGEMENT_DEF_AUTH
> +                    status_printf(so, "\"Client ID\": %lu,", mi->context.c2.mda_context.cid);
> +#endif
> +                    status_printf(so, "\"Peer ID\": %" PRIu32, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->peer_id : UINT32_MAX);
> +                    status_printf(so, "}");
> +                }
> +                gc_free(&gc);
> +            }
> +            hash_iterator_free(&hi);
> +
> +            status_printf(so, "],");
> +            status_printf(so, "\"ROUTING_TABLE\": [");
> +
> +            is_first = 1;
> +            hash_iterator_init(m->vhash, &hi);
> +            while ((he = hash_iterator_next(&hi)))
> +            {
> +                struct gc_arena gc = gc_new();
> +                const struct multi_route *route = (struct multi_route *) he->value;
> +
> +                if (multi_route_defined(m, route))
> +                {
> +                    const struct multi_instance *mi = route->instance;
> +                    const struct mroute_addr *ma = &route->addr;
> +                    char flags[2] = {0, 0};
> +
> +                    if (route->flags & MULTI_ROUTE_CACHE)
> +                    {
> +                        flags[0] = 'C';
> +                    }
> +                        if(0 == is_first) {
> +                            status_printf(so, ",");
> +                        } else {
> +                            is_first = 0;
> +                        }
> +                        status_printf(so, "{\"Virtual Address\": \"%s%s\",", mroute_addr_print(ma, &gc), flags);
> +                        status_printf(so, "\"Common Name\": \"%s\",", tls_common_name(mi->context.c2.tls_multi, false));
> +                        status_printf(so, "\"Real Address\": \"%s\",", mroute_addr_print(&mi->real, &gc));
> +                        status_printf(so, "\"Last Ref\": \"%s\",", time_string(route->last_reference, 0, false, &gc));
> +                        status_printf(so, "\"Last Ref (time_t)\": %u}", (unsigned int)route->last_reference);
> +                }
> +                gc_free(&gc);
> +            }
> +            hash_iterator_free(&hi);
> +            status_printf(so, "]");
> +
> +            if (m->mbuf)
> +            {
> +                status_printf(so, ",\"GLOBAL_STATS\": {");
> +                status_printf(so, "\"Max bcast/mcast queue length\": %d", mbuf_maximum_queued(m->mbuf));
> +                status_printf(so, "}");
> +            }
> +            status_printf(so, "}");
> +            status_printf(so, "END");
> +        }
>          else
>          {
>              status_printf(so, "ERROR: bad status format version number");
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
François Kooman Nov. 12, 2017, 12:15 a.m. UTC | #2
On 11/11/2017 04:55 PM, Gert van Dijk wrote:
> Hi François,

Hi Gert,

> This is not a full review, but as just discussed face-to-face, here are
> some points from me:

Thanks for taking the time to look at it!

> 1) Could we also please please enable this for non-management interface
> use case? I guess it's just adding the status version number 4 in
> option/config validator (and the manpage).

Yes, I will add this as well.

> 2) With 1), perhaps it would be more clear to use *version* in your
> message/description so that it's about the --status-version option, not
> the --status option.

This got confused because in the management interface (via telnet) you'd
use "status" and not "status-version".

> 3) I believe the output structure is not valid JSON with the "END"
> string at the end.

All (multiline) responses end with "END", so that's why I did that as
well. Ideally I'd construct a JSON object that does not include newlines
at all and just responds as:

SUCCESS: { ... }

> (FYI I'm about to upload a patch to include more documentation on the
> --status-version option and to include a new field as well.)

Great!

Cheers,
François

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair Nov. 12, 2017, 6:21 a.m. UTC | #3
Hi

> 3) I believe the output structure is not valid JSON with the "END"
> > string at the end.
>
> All (multiline) responses end with "END", so that's why I did that as
> well. Ideally I'd construct a JSON object that does not include newlines
> at all and just responds as:
>
> SUCCESS: { ... }
>

One of the niceties of JSON is its readability which is greatly reduced
if formatted without line breaks. Multi-line with the same
format used for management and console would be nice. Except the
former gets the extra footer line with "END".

Selva
<div dir="ltr">Hi<div><br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
&gt; 3) I believe the output structure is not valid JSON with the &quot;END&quot;<br>
&gt; string at the end.<br>
<br>
</span>All (multiline) responses end with &quot;END&quot;, so that&#39;s why I did that as<br>
well. Ideally I&#39;d construct a JSON object that does not include newlines<br>
at all and just responds as:<br>
<br>
SUCCESS: { ... }<br></blockquote><div><br></div><div>One of the niceties of JSON is its readability which is greatly reduced</div><div>if formatted without line breaks. Multi-line with the same</div><div>format used for management and console would be nice. Except the</div><div>former gets the extra footer line with &quot;END&quot;.</div><div><br></div><div>Selva</div></div></div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering Nov. 12, 2017, 10:42 a.m. UTC | #4
Hi,

On Sun, Nov 12, 2017 at 12:21:06PM -0500, Selva wrote:
> One of the niceties of JSON is its readability which is greatly reduced
> if formatted without line breaks. 

+1

the difference in efficienty is not large, but "human readability for
manual proofreading" is good.

gert
François Kooman Nov. 12, 2017, 10:44 p.m. UTC | #5
On 11/12/2017 10:42 PM, Gert Doering wrote:
> On Sun, Nov 12, 2017 at 12:21:06PM -0500, Selva wrote:
>> One of the niceties of JSON is its readability which is greatly reduced
>> if formatted without line breaks. 
> 
> +1
> 
> the difference in efficienty is not large, but "human readability for
> manual proofreading" is good.

Selva, Gert,

You know, the funny thing is that my main reason for implementing JSON
support for status was to have it easily _machine_ readable, for higher
level programming languages where JSON is "first class citizen", and not
primarily human readability. Besides you could easily do this to make
JSON human readable:

$ cat /path/to/status.json | python -mjson.tool

Thinking some more about it now, it doesn't really make sense to
implement JSON at all, because CSV _is_ actually meant to be machine
readable and a lot more efficient in the amount of data it uses (much
less duplication). So it seems my reasons for implementing JSON support
seems to completely miss the point. For machine readable output it would
make more sense to use a binary format instead of JSON.

What do you guys think? Are there any other reasons to implement JSON
support?

Cheers,
François
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Antonio Quartulli Nov. 12, 2017, 10:54 p.m. UTC | #6
Hi,

On 13/11/17 17:44, François Kooman wrote:
> On 11/12/2017 10:42 PM, Gert Doering wrote:
>> On Sun, Nov 12, 2017 at 12:21:06PM -0500, Selva wrote:
>>> One of the niceties of JSON is its readability which is greatly reduced
>>> if formatted without line breaks. 
>>
>> +1
>>
>> the difference in efficienty is not large, but "human readability for
>> manual proofreading" is good.
> 
> Selva, Gert,
> 
> You know, the funny thing is that my main reason for implementing JSON
> support for status was to have it easily _machine_ readable, for higher
> level programming languages where JSON is "first class citizen", and not
> primarily human readability. Besides you could easily do this to make
> JSON human readable:
> 
> $ cat /path/to/status.json | python -mjson.tool
> 
> Thinking some more about it now, it doesn't really make sense to
> implement JSON at all, because CSV _is_ actually meant to be machine
> readable and a lot more efficient in the amount of data it uses (much
> less duplication). So it seems my reasons for implementing JSON support
> seems to completely miss the point. For machine readable output it would
> make more sense to use a binary format instead of JSON.
> 
> What do you guys think? Are there any other reasons to implement JSON
> support?

Although JSON is a bloated language (meaning that there is lots of
overhead next to the data itself), I think that it allows you to easily
extend the objects being sent back and forth with a fairly high chance
to keep backwards compatibility.

This is barely true for csv.

A binary presentation per se is not bad, but then you need to make sure
that the format is properly encoded/decoded (especially if the data is
transferred across the network from/to different platforms) and this can
be error prone.


Honestly I did not read the rest of the thread, but if this data is
supposed to be exposed through an API (i.e. REST API or any other
interface) and it is meant to travel across the network, I wouldn't mind
seeing the JSON format being used.


JSON consumers nowadays are extremely easy to implement compared to others.


Regarding the JSON "prettiness", I think that printing it in a nice
readable format makes it just extremely easy to debug and possibly spot
errors on the fly (even when not searching for them). For this reason
I'd rather add a couple of tabs and \n to make it easier to read.

my2cents


Cheers,
Steffan Karger Nov. 12, 2017, 11:15 p.m. UTC | #7
Hi,

Since I don't use the mgmt interface / status file a lot, I didn't chip
in earlier.  But since this quite closely matches my initial thoughts
I'll do so anyway.

On 13-11-17 10:54, Antonio Quartulli wrote:
> On 13/11/17 17:44, François Kooman wrote:
>> On 11/12/2017 10:42 PM, Gert Doering wrote:
>>> On Sun, Nov 12, 2017 at 12:21:06PM -0500, Selva wrote:
>>>> One of the niceties of JSON is its readability which is greatly reduced
>>>> if formatted without line breaks. 
>>>
>>> +1
>>>
>>> the difference in efficienty is not large, but "human readability for
>>> manual proofreading" is good.
>>
>> Selva, Gert,
>>
>> You know, the funny thing is that my main reason for implementing JSON
>> support for status was to have it easily _machine_ readable, for higher
>> level programming languages where JSON is "first class citizen", and not
>> primarily human readability. Besides you could easily do this to make
>> JSON human readable:
>>
>> $ cat /path/to/status.json | python -mjson.tool
>>
>> Thinking some more about it now, it doesn't really make sense to
>> implement JSON at all, because CSV _is_ actually meant to be machine
>> readable and a lot more efficient in the amount of data it uses (much
>> less duplication). So it seems my reasons for implementing JSON support
>> seems to completely miss the point. For machine readable output it would
>> make more sense to use a binary format instead of JSON.
>>
>> What do you guys think? Are there any other reasons to implement JSON
>> support?

This was my original question: why should we *add* JSON?  I agree that
if we would have to pick something now, I'd prefer JSON over CSV.  But
we already have CSV, and as far as I understand it suffices just fine.

> Although JSON is a bloated language (meaning that there is lots of
> overhead next to the data itself), I think that it allows you to easily
> extend the objects being sent back and forth with a fairly high chance
> to keep backwards compatibility.
> 
> This is barely true for csv.

The biggest difference is that CSV is 'flat' while JSON is
"hierarchical".  But since we already support CSV (and want to keep
supporting it, I guess?), I think we're kind of bound to a flat
structure anyway.

> A binary presentation per se is not bad, but then you need to make sure
> that the format is properly encoded/decoded (especially if the data is
> transferred across the network from/to different platforms) and this can
> be error prone.

Slightly offtopic: I recently came across CBOR, which seems a good
attempt to merge the benefits of JSON with a binary representation:
https://tools.ietf.org/html/rfc7049

Haven't really used it yet though, so no practical experiences
unfortunately.

> Honestly I did not read the rest of the thread, but if this data is
> supposed to be exposed through an API (i.e. REST API or any other
> interface) and it is meant to travel across the network, I wouldn't mind
> seeing the JSON format being used.
> 
> JSON consumers nowadays are extremely easy to implement compared to others.

Even compared to CSV?  I can't really come up with a situation where I
can consume JSON easily, but can't consume CSV.

> Regarding the JSON "prettiness", I think that printing it in a nice
> readable format makes it just extremely easy to debug and possibly spot
> errors on the fly (even when not searching for them). For this reason
> I'd rather add a couple of tabs and \n to make it easier to read.

+1, one of the strong sides of JSON is that is easy to make it both user
and machine readable.

-Steffan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
David Sommerseth Nov. 13, 2017, 1:16 a.m. UTC | #8
On 13/11/17 11:15, Steffan Karger wrote:
[...snip...]
>>> You know, the funny thing is that my main reason for implementing JSON
>>> support for status was to have it easily _machine_ readable, for higher
>>> level programming languages where JSON is "first class citizen", and not
>>> primarily human readability. Besides you could easily do this to make
>>> JSON human readable:
>>>
>>> $ cat /path/to/status.json | python -mjson.tool
>>>
>>> Thinking some more about it now, it doesn't really make sense to
>>> implement JSON at all, because CSV _is_ actually meant to be machine
>>> readable and a lot more efficient in the amount of data it uses (much
>>> less duplication). So it seems my reasons for implementing JSON support
>>> seems to completely miss the point. For machine readable output it would
>>> make more sense to use a binary format instead of JSON.
>>>
>>> What do you guys think? Are there any other reasons to implement JSON
>>> support?
> 
> This was my original question: why should we *add* JSON?  I agree that
> if we would have to pick something now, I'd prefer JSON over CSV.  But
> we already have CSV, and as far as I understand it suffices just fine.

The tooling libraries around JSON vs CSV are more commonly more mature
these days for JSON; plus it is so widely used the interwebs are filled
with examples.  And this does not only go for scripted languages; even
there are also nice libraries for C and C++ too, giving you a machine
parseable object with better predictability on the result.

If I had a project parsing a CSV and had been given a chance to use JSON
instead; I'd migrate over ASAP as long as the risks of breaking
something badly was not very high (It would depend on where in the call
chain such changes happens, and how much happens after that point).

So I'm fine with adding --status 4 which dumps JSON.

>> Although JSON is a bloated language (meaning that there is lots of
>> overhead next to the data itself), I think that it allows you to easily
>> extend the objects being sent back and forth with a fairly high chance
>> to keep backwards compatibility.
>>
>> This is barely true for csv.
> 
> The biggest difference is that CSV is 'flat' while JSON is
> "hierarchical".  But since we already support CSV (and want to keep
> supporting it, I guess?), I think we're kind of bound to a flat
> structure anyway.

Yeah, we cannot rip out CSV now.  We could probably propose that further
into the future once JSON is widely used.  And we could provide some
tooling which converts JSON to CSV outside of OpenVPN at that time.

>> A binary presentation per se is not bad, but then you need to make sure
>> that the format is properly encoded/decoded (especially if the data is
>> transferred across the network from/to different platforms) and this can
>> be error prone.
> 
> Slightly offtopic: I recently came across CBOR, which seems a good
> attempt to merge the benefits of JSON with a binary representation:
> https://tools.ietf.org/html/rfc7049
> 
> Haven't really used it yet though, so no practical experiences
> unfortunately.

I think that if we want to have a binary approach, that should be a
variant of --status 4; like --status 4_binary.

>> Honestly I did not read the rest of the thread, but if this data is
>> supposed to be exposed through an API (i.e. REST API or any other
>> interface) and it is meant to travel across the network, I wouldn't mind
>> seeing the JSON format being used.
>>
>> JSON consumers nowadays are extremely easy to implement compared to others.
> 
> Even compared to CSV?  I can't really come up with a situation where I
> can consume JSON easily, but can't consume CSV.

The library tooling is in my experience far easier and predictable.  And
the JSON libraries have generally been far better at "object-ifying"
JSON to objects you use in your code than what CSV parses have been.
But I'll admit, I've mostly avoided CSV for the last decade or so if I
could use JSON or even XML.

CSV just looks structured - until the layout is changed.  While JSON
(and even XML for that matter, _not_ saying we should have XML support!)
is not strictly bound to the order of fields, so adding new fields is a
no-brainer with JSON compared to CSV.  And before you say "We only
append fields to CSV", some of those changes we have done in the past to
--status 3 have even broken some users setup - because the CSV parser
was stupid.  The risk of that happening to JSON is far less.

>> Regarding the JSON "prettiness", I think that printing it in a nice
>> readable format makes it just extremely easy to debug and possibly spot
>> errors on the fly (even when not searching for them). For this reason
>> I'd rather add a couple of tabs and \n to make it easier to read.
> 
> +1, one of the strong sides of JSON is that is easy to make it both user
> and machine readable.

+1 from me too.


Just a few words to the management interface vs --status file.  We
should support JSON via both channels.  And implementing that shouldn't
be rocket science as it should be the same function producing that result.

But we should consider if we want to make use of a JSON library
producing the JSON streams.  The reason is to ensure the output is
according to the specification and that escaping if contents is
consistent and proper.  Imagine if someone puts a double-quote into the
CN field of a certificate?

 CN="} Lets explode things, O=Hacktivist0

Or other characters which needs escaping.

I also see the proposed patch adds "END" at the very end.  To me that
makes more sense in text/plain representations to indicate that this is
the end of a list of unknown length.  JSON have that implicitly with the
last '}'.


Just my few cents for this round :)
François Kooman Nov. 13, 2017, 2:44 a.m. UTC | #9
On 11/13/2017 01:16 PM, David Sommerseth wrote:
> CSV just looks structured - until the layout is changed.  While JSON
> (and even XML for that matter, _not_ saying we should have XML support!)
> is not strictly bound to the order of fields, so adding new fields is a
> no-brainer with JSON compared to CSV.  And before you say "We only
> append fields to CSV", some of those changes we have done in the past to
> --status 3 have even broken some users setup - because the CSV parser
> was stupid.  The risk of that happening to JSON is far less.

This is a very good point. This was one of the problems I had before
parsing OpenVPN's CSV when it changed (a little) between 2.3 and 2.4.

> Just a few words to the management interface vs --status file.  We
> should support JSON via both channels.  And implementing that shouldn't
> be rocket science as it should be the same function producing that result.

Agreed, also talked about with Gert (van Dijk) @ Hackathon.

> But we should consider if we want to make use of a JSON library
> producing the JSON streams.  The reason is to ensure the output is
> according to the specification and that escaping if contents is
> consistent and proper.  Imagine if someone puts a double-quote into the
> CN field of a certificate?
> 
>  CN="} Lets explode things, O=Hacktivist0
> 
> Or other characters which needs escaping.

Another very good point. I do have some experience with json-c [1],
would it be acceptable to add this as a project dependency? That is the
main reason I started out by just outputting a string and conveniently
forgot escaping issues...

So for now my plan is:

1. use json-c to generate (human readable) JSON;
2. also support --status (command line) in addition to "status" in
management interface
3. do not print "END" marker any longer

This is an excellent way for me to gain some more experience with
writing C (and OpenVPN), even if in the end the patch is not accepted. I
do think, like Steffan said, we need a good reason to support JSON,
especially if it adds an additional external dependency. The
future-proof-ness of the JSON format could be a good selling point,
maybe new fields would only be added to the JSON format and not to the
other status formats to promote adoption of the JSON format and slowly
phasing out the CSV format(s).

Cheers,
François

[1] https://github.com/json-c/json-c/
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
David Sommerseth Nov. 13, 2017, 9:40 a.m. UTC | #10
On 13/11/17 14:44, François Kooman wrote:
> On 11/13/2017 01:16 PM, David Sommerseth wrote:
[...snip...]
>> But we should consider if we want to make use of a JSON library
>> producing the JSON streams.  The reason is to ensure the output is
>> according to the specification and that escaping if contents is
>> consistent and proper.  Imagine if someone puts a double-quote into the
>> CN field of a certificate?
>>
>>  CN="} Lets explode things, O=Hacktivist0
>>
>> Or other characters which needs escaping.
> 
> Another very good point. I do have some experience with json-c [1],
> would it be acceptable to add this as a project dependency? That is the
> main reason I started out by just outputting a string and conveniently
> forgot escaping issues...

Yes, I think that should be fine.  I see it is available in RHEL6 (the
oldest Linux distribution we support, json-c-0.11-13.el6), so this
should work out just fine.  So keep in mind to not use a bleeding fresh
version only available in the latest faster-moving Linux distros.

What would be needed though is to enable/disable this via ./configure
(tackled in configure.ac).  We basically can have three approaches here:

 a)  If json-c is found, enable --status 4 by default. If not found,
     this feature is unavailable.

 b)  Require json-c to be installed, which enables --status 4.  To build
     without json-c dependency, --disable-json must be used with
     ./configure

 c) Do not require json-c by default, but if adding --enable-json
    to ./configure then json-c must be present.

I don't have any strong opinion either way, but I believe alternative c)
is the one which will give the least fuzz in the community - thus
overall be more acceptable.


> So for now my plan is:
> 
> 1. use json-c to generate (human readable) JSON;
> 2. also support --status (command line) in addition to "status" in
> management interface
> 3. do not print "END" marker any longer

Sounds like a good plan!

> This is an excellent way for me to gain some more experience with
> writing C (and OpenVPN), even if in the end the patch is not accepted. I
> do think, like Steffan said, we need a good reason to support JSON,
> especially if it adds an additional external dependency. The
> future-proof-ness of the JSON format could be a good selling point,
> maybe new fields would only be added to the JSON format and not to the
> other status formats to promote adoption of the JSON format and slowly
> phasing out the CSV format(s).

That's a good attitude :)  I for one is not hard to get sold on the JSON
format though, even though I don't have any particular use right now.
But I can easily see this being preferred for others outside the core
project.  And this can simplify some ideas I do have pondered on in my
own infrastructure.

Again the alternative c) approach will fit nicely into this too.  I am
definitely willing to enable a JSON feature in the Fedora builds when
patches hits an upstream release.  And if that works out nice, I'll
consider to add that even in the EPEL builds too - I'll most likely
mention that on the Fedora devel mailing list to get some thoughts on
the EPEL side before enabling it.
Selva Nair Nov. 13, 2017, 11:57 a.m. UTC | #11
Hi,

On Mon, Nov 13, 2017 at 3:40 PM, David Sommerseth <openvpn@sf.lists.
topphemmelig.net> wrote:

> On 13/11/17 14:44, François Kooman wrote:
> > On 11/13/2017 01:16 PM, David Sommerseth wrote:
> [...snip...]
> >> But we should consider if we want to make use of a JSON library
> >> producing the JSON streams.  The reason is to ensure the output is
> >> according to the specification and that escaping if contents is
> >> consistent and proper.  Imagine if someone puts a double-quote into the
> >> CN field of a certificate?
> >>
> >>  CN="} Lets explode things, O=Hacktivist0
> >>
> >> Or other characters which needs escaping.
> >
> > Another very good point. I do have some experience with json-c [1],
> > would it be acceptable to add this as a project dependency? That is the
> > main reason I started out by just outputting a string and conveniently
> > forgot escaping issues...
>
> Yes, I think that should be fine.  I see it is available in RHEL6 (the
> oldest Linux distribution we support, json-c-0.11-13.el6), so this
> should work out just fine.  So keep in mind to not use a bleeding fresh
> version only available in the latest faster-moving Linux distros.


> What would be needed though is to enable/disable this via ./configure
> (tackled in configure.ac).  We basically can have three approaches here:
>
>  a)  If json-c is found, enable --status 4 by default. If not found,
>      this feature is unavailable.
>

There is no libjson-c for Windows, I guess... Probably no one cares if
--status 4 is not supported on Windows.


>
> > 1. use json-c to generate (human readable) JSON;
> > 2. also support --status (command line) in addition to "status" in
> > management interface
> > 3. do not print "END" marker any longer
>

I suppose that means no "END" as a part of the JSON output (ie., not written
to console or file) but if the output is multi-line, management still needs
to get a
terminating  "END\n" line. Else we will be breaking the management
interface protocol as it stands now.

Selva
<div dir="ltr">Hi,<br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 13, 2017 at 3:40 PM, David <span class="" id=":aj5.1" tabindex="-1">Sommerseth</span> <span dir="ltr">&lt;<a href="mailto:openvpn@sf.lists.topphemmelig.net" target="_blank"><span class="" id=":aj5.2" tabindex="-1">openvpn</span>@sf.lists.<span class="" id=":aj5.3" tabindex="-1">topphemmelig</span>.net</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On 13/11/17 14:44, François Kooman wrote:<br>
&gt; On 11/13/2017 01:16 PM, David Sommerseth wrote:<br>
</span>[...snip...]<br>
<span class="gmail-">&gt;&gt; But we should consider if we want to make use of a JSON library<br>
&gt;&gt; producing the JSON streams.  The reason is to ensure the output is<br>
&gt;&gt; according to the specification and that escaping if contents is<br>
&gt;&gt; consistent and proper.  Imagine if someone puts a double-quote into the<br>
&gt;&gt; CN field of a certificate?<br>
&gt;&gt;<br>
&gt;&gt;  CN=&quot;} Lets explode things, O=Hacktivist0<br>
&gt;&gt;<br>
&gt;&gt; Or other characters which needs escaping.<br>
&gt;<br>
&gt; Another very good point. I do have some experience with json-c [1],<br>
&gt; would it be acceptable to add this as a project dependency? That is the<br>
&gt; main reason I started out by just outputting a string and conveniently<br>
&gt; forgot escaping issues...<br>
<br>
</span>Yes, I think that should be fine.  I see it is available in RHEL6 (the<br>
oldest Linux distribution we support, json-c-0.11-13.el6), so this<br>
should work out just fine.  So keep in mind to not use a bleeding fresh<br>
version only available in the latest faster-moving Linux distros.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
What would be needed though is to enable/disable this via ./configure<br>
(tackled in <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>).  We basically can have three approaches here:<br>
<br>
 a)  If json-c is found, enable --status 4 by default. If not found,<br>
     this feature is unavailable.<br></blockquote><div><div><br></div><div>There is no <span class="" id=":aj5.4" tabindex="-1">libjson</span>-c for Windows, I guess... Probably no one cares if</div><div>--status 4 is not supported on Windows.</div><div> </div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-"><br>
&gt; 1. use json-c to generate (human readable) JSON;<br>
&gt; 2. also support --status (command line) in addition to &quot;status&quot; in<br>
&gt; management interface<br>
&gt; 3. do not print &quot;END&quot; marker any longer<br></span></blockquote><div><br></div><div>I suppose that means no &quot;END&quot; as a part of the <span class="" id=":aj5.5" tabindex="-1">JSON</span> output (<span class="" id=":aj5.6" tabindex="-1">ie</span>., not written</div><div>to console or file) but if the output is multi-line, management still needs to get a</div><div>terminating  &quot;END\n&quot; line. Else we will be breaking the management</div><div>interface protocol as it stands now.</div><div><br></div><div><span class="" id=":aj5.7" tabindex="-1">Selva</span></div></div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering Nov. 13, 2017, 9:31 p.m. UTC | #12
Hi,

On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote:
> But we should consider if we want to make use of a JSON library
> producing the JSON streams.  The reason is to ensure the output is
> according to the specification and that escaping if contents is
> consistent and proper.  Imagine if someone puts a double-quote into the
> CN field of a certificate?
> 
>  CN="} Lets explode things, O=Hacktivist0
> 
> Or other characters which needs escaping.

I'm not convinced we want to import a new library dependency and a heap
of #ifdef for this.

Escaping on *output* is pretty trivial ("characters from <this set> 
need to be encoded <like this>") - and as long as we do not need to parse 
*incoming* JSON, a full-blown new library is mainly adding complications
(like, configure flags, #ifdefs, library version dependencies, ...).

But you knew that this response would be coming :-)

gert
Antonio Quartulli Nov. 13, 2017, 9:35 p.m. UTC | #13
Hi,

On 14/11/17 16:31, Gert Doering wrote:
> Hi,
> 
> On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote:
>> But we should consider if we want to make use of a JSON library
>> producing the JSON streams.  The reason is to ensure the output is
>> according to the specification and that escaping if contents is
>> consistent and proper.  Imagine if someone puts a double-quote into the
>> CN field of a certificate?
>>
>>  CN="} Lets explode things, O=Hacktivist0
>>
>> Or other characters which needs escaping.
> 
> I'm not convinced we want to import a new library dependency and a heap
> of #ifdef for this.
> 
> Escaping on *output* is pretty trivial ("characters from <this set> 
> need to be encoded <like this>") - and as long as we do not need to parse 
> *incoming* JSON, a full-blown new library is mainly adding complications
> (like, configure flags, #ifdefs, library version dependencies, ...).
> 
> But you knew that this response would be coming :-)

+1

if we want to do output only, we should not need another library.
Especially if we are not going to do complex stuff.


Cheers,
David Sommerseth Nov. 13, 2017, 11:56 p.m. UTC | #14
On 14/11/17 09:31, Gert Doering wrote:
> Hi,
> 
> On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote:
>> But we should consider if we want to make use of a JSON library
>> producing the JSON streams.  The reason is to ensure the output is
>> according to the specification and that escaping if contents is
>> consistent and proper.  Imagine if someone puts a double-quote into the
>> CN field of a certificate?
>>
>>  CN="} Lets explode things, O=Hacktivist0
>>
>> Or other characters which needs escaping.
> 
> I'm not convinced we want to import a new library dependency and a heap
> of #ifdef for this.
> 
> Escaping on *output* is pretty trivial ("characters from <this set> 
> need to be encoded <like this>") - and as long as we do not need to parse 
> *incoming* JSON, a full-blown new library is mainly adding complications
> (like, configure flags, #ifdefs, library version dependencies, ...).
> 
> But you knew that this response would be coming :-)
Not surprised ;-)

But I do disagree.  I tend to dislike re-inventing wheels when others do
the job for you.  Quite some years ago, I would probably have agreed
with you.  But I've been in projects where I implemented producing XML
data manually in a similar manner.  After fixing numerous of escaping
errors, I switched to a library and all these bugs disappeared and never
came back.  It is so easy to think you know exactly what needs to be
escaped beforehand and firmly believe you have covered each corner-case,
but it always sneaks back at you - as you didn't spot the hidden doors.

And to me this feels like it actually contradicts our netlink discussion
on the Hackathon, where you initially wanted to use an external library
... ;-)

That said, json-c is packaged and available on Linux, FreeBSD and even
OpenWRT/LEDE got a recent version available. I don't know the situation
on macOS (but that's similar to FreeBSD, I'd presume) and Windows is
also not clear to me (if this is a relevant feature there).  json-c is
also fairly small (approx 7k lines of code, stripped binary lib ~60KB)
and is being maintained (last commits from late October).  So this isn't
a big massive dependency.

With ./configure --enable-json we also don't change the current
behaviour in OpenVPN.  Which should only require a single #ifdef
ENABLE_JSON in the multi_print_status() [multi.c:859] function.  But I
also see that the status printing stuff could be improved as well, to
simplify the implementation a bit too (which is a different task).
Gert Doering Nov. 14, 2017, 12:02 a.m. UTC | #15
Hi,

On Tue, Nov 14, 2017 at 11:56:10AM +0100, David Sommerseth wrote:
> And to me this feels like it actually contradicts our netlink discussion
> on the Hackathon, where you initially wanted to use an external library
> ... ;-)

It's a matter of complexity.   Netlink is binary, complex, and hard to
verify (unless you write your own decoder, which might have bugs on its
own).

JSON is very trivial to produce (unlike XML, or netlink).  The escaping
rules on producing are also very easy - basically, encode things in double
quotes, and escape the set of { BS, FF, NL, CR, Tab, Backslash, Quote }
with a single backslash before the corresponding character.

https://stackoverflow.com/questions/19176024/how-to-escape-special-characters-in-building-a-json-string


For *parsing*, I'd agree with you...

gert
Jonathan K. Bullard Nov. 14, 2017, 1:01 a.m. UTC | #16
Hi,

On Tue, Nov 14, 2017 at 3:31 AM, Gert Doering <gert@greenie.muc.de> wrote:
> Hi,
>
> On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote:
>> But we should consider if we want to make use of a JSON library
>> producing the JSON streams.  The reason is to ensure the output is
>> according to the specification and that escaping if contents is
>> consistent and proper.  Imagine if someone puts a double-quote into the
>> CN field of a certificate?
>>
>>  CN="} Lets explode things, O=Hacktivist0
>>
>> Or other characters which needs escaping.
>
> I'm not convinced we want to import a new library dependency and a heap
> of #ifdef for this.
>
> Escaping on *output* is pretty trivial ("characters from <this set>
> need to be encoded <like this>") - and as long as we do not need to parse
> *incoming* JSON, a full-blown new library is mainly adding complications
> (like, configure flags, #ifdefs, library version dependencies, ...).

+1

Best regards,

Jon Bullard

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Jan Just Keijser Nov. 14, 2017, 1:17 a.m. UTC | #17
On 14/11/17 09:31, Gert Doering wrote:
> On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote:
>> But we should consider if we want to make use of a JSON library
>> producing the JSON streams.  The reason is to ensure the output is
>> according to the specification and that escaping if contents is
>> consistent and proper.  Imagine if someone puts a double-quote into the
>> CN field of a certificate?
>>
>>   CN="} Lets explode things, O=Hacktivist0
>>
>> Or other characters which needs escaping.
> I'm not convinced we want to import a new library dependency and a heap
> of #ifdef for this.
>
> Escaping on *output* is pretty trivial ("characters from <this set>
> need to be encoded <like this>") - and as long as we do not need to parse
> *incoming* JSON, a full-blown new library is mainly adding complications
> (like, configure flags, #ifdefs, library version dependencies, ...).
>
> But you knew that this response would be coming :-)
>
>
+1

JJK


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
David Sommerseth Nov. 14, 2017, 1:40 a.m. UTC | #18
On 14/11/17 12:02, Gert Doering wrote:
> JSON is very trivial to produce (unlike XML, or netlink).  The escaping
> rules on producing are also very easy - basically, encode things in double
> quotes, and escape the set of { BS, FF, NL, CR, Tab, Backslash, Quote }
> with a single backslash before the corresponding character.
>
>
<https://stackoverflow.com/questions/19176024/how-to-escape-special-characters-in-building-a-json-string>

Right, all those are single byte characters - and that's fairly simple
... but that ignores various quirks which easily appears with multi-byte
characters - especially when "looping" through a value, byte by byte.
We support UTF-8 in certificate subject fields.  So this escaper needs
to handle those classes the stackoverflow mentions, plus beware of
multi-byte strings (so we need to use the plethora of mb* related
functions).

In a clean 8-bit ASCII only-world, things are less complicated.

Heiko and I have looked into the "simple" world of revamping the argv
parser (to avoid our own "homebrewed" printf-like processing and base it
on what is in the C library) and even this pre-parsing we need to do
have popped up with surprises.  The argv caller scope mostly covers
parsing strings which is defined by us developers so the variations are
not as broad, and luckily format strings is not expected to contain
UTF-8 chars.  But I do not for a second think processing certificate
subject strings will as easy as those values we need to parse (typically
CN) are not generated by us but a broad range of users.  Who knows what
kind of funny tricks they'll throw at our code?

But sure, we can start with our own string parser to escape JSON and see
how it works and then revisit using a library if the maintenance gets
too annoying.
Jonathan K. Bullard Nov. 15, 2017, 1:34 a.m. UTC | #19
Hi,

On Tue, Nov 14, 2017 at 7:40 AM, David Sommerseth
<openvpn@sf.lists.topphemmelig.net> wrote:
>
> On 14/11/17 12:02, Gert Doering wrote:
>> JSON is very trivial to produce (unlike XML, or netlink).  The escaping
>> rules on producing are also very easy - basically, encode things in double
>> quotes, and escape the set of { BS, FF, NL, CR, Tab, Backslash, Quote }
>> with a single backslash before the corresponding character.

My reading of the RFC [1] (maybe not the only RFC for JSON?) is that
all control characters must be escaped (all characters < 0x20). And
the quotation mark that must be escaped is the double-quote (0x22),
not the single-quote (0x27).

Also, the escaping needs to be of the form \uXXXX where XXXX is the
URF-16 code unit for the character. That's easy for these characters
-- I think it's printf("\\u%04d", ch) since all the characters we are
escaping are in the Basic Multilingual Plane (U+0000 through U+FFFF).


> <https://stackoverflow.com/questions/19176024/how-to-escape-special-characters-in-building-a-json-string>
>
> Right, all those are single byte characters - and that's fairly simple
> ... but that ignores various quirks which easily appears with multi-byte
> characters - especially when "looping" through a value, byte by byte.
> We support UTF-8 in certificate subject fields.  So this escaper needs
> to handle those classes the stackoverflow mentions, plus beware of
> multi-byte strings (so we need to use the plethora of mb* related
> functions).
>
> In a clean 8-bit ASCII only-world, things are less complicated.
>
> Heiko and I have looked into the "simple" world of revamping the argv
> parser (to avoid our own "homebrewed" printf-like processing and base it
> on what is in the C library) and even this pre-parsing we need to do
> have popped up with surprises.  The argv caller scope mostly covers
> parsing strings which is defined by us developers so the variations are
> not as broad, and luckily format strings is not expected to contain
> UTF-8 chars.  But I do not for a second think processing certificate
> subject strings will as easy as those values we need to parse (typically
> CN) are not generated by us but a broad range of users.  Who knows what
> kind of funny tricks they'll throw at our code?

How does OpenVPN currently handle escaping when generating CSV? If we
don't do it correctly for multi-byte strings for CSV, do we need to
for JSON?

From what I can tell, the only problematic character that needs to be
escaped is the backslash (0x5C). All the other characters that need to
be escaped are less than 0x40, so they will never appear as part of a
multi-byte sequence. That one character makes this difficult, but IMO
the problem is simple enough that it could be done within OpenVPN.
That would be my preference, instead of adding another dependency.

Best regards,

Jon Bullard

[1] https://www.ietf.org/rfc/rfc4627.txt

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair Nov. 15, 2017, 4:09 a.m. UTC | #20
On Tue, Nov 14, 2017 at 7:17 AM, Jan Just Keijser <janjust@nikhef.nl> wrote:

> On 14/11/17 09:31, Gert Doering wrote:
>
>> On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote:
>>
>>> But we should consider if we want to make use of a JSON library
>>> producing the JSON streams.  The reason is to ensure the output is
>>> according to the specification and that escaping if contents is
>>> consistent and proper.  Imagine if someone puts a double-quote into the
>>> CN field of a certificate?
>>>
>>>   CN="} Lets explode things, O=Hacktivist0
>>>
>>> Or other characters which needs escaping.
>>>
>> I'm not convinced we want to import a new library dependency and a heap
>> of #ifdef for this.
>>
>> Escaping on *output* is pretty trivial ("characters from <this set>
>> need to be encoded <like this>") - and as long as we do not need to parse
>> *incoming* JSON, a full-blown new library is mainly adding complications
>> (like, configure flags, #ifdefs, library version dependencies, ...).
>>
>> But you knew that this response would be coming :-)
>>
>>
>> +1



Piling on... with another +1 :)

I would even say that if an external library is required reconsider whether
we really
need JSON output.

Selva
<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 14, 2017 at 7:17 AM, Jan Just Keijser <span dir="ltr">&lt;<a href="mailto:janjust@nikhef.nl" target="_blank">janjust@nikhef.nl</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 14/11/17 09:31, Gert Doering wrote:<br>
</span><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Nov 13, 2017 at 01:16:46PM +0100, David Sommerseth wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
But we should consider if we want to make use of a JSON library<br>
producing the JSON streams.  The reason is to ensure the output is<br>
according to the specification and that escaping if contents is<br>
consistent and proper.  Imagine if someone puts a double-quote into the<br>
CN field of a certificate?<br>
<br>
  CN=&quot;} Lets explode things, O=Hacktivist0<br>
<br>
Or other characters which needs escaping.<br>
</blockquote>
I&#39;m not convinced we want to import a new library dependency and a heap<br>
of #ifdef for this.<br>
<br>
Escaping on *output* is pretty trivial (&quot;characters from &lt;this set&gt;<br>
need to be encoded &lt;like this&gt;&quot;) - and as long as we do not need to parse<br>
*incoming* JSON, a full-blown new library is mainly adding complications<br>
(like, configure flags, #ifdefs, library version dependencies, ...).<br>
<br>
But you knew that this response would be coming :-)<br>
<br>
<br>
</blockquote></span>
+1</blockquote><div><br></div><div><br></div><div>Piling on... with another +1 :)</div><div><br></div><div>I would even say that if an external library is required reconsider whether we really</div><div>need JSON output.</div><div><br></div><div>Selva</div></div></div></div>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Gert Doering Oct. 6, 2018, 8:10 p.m. UTC | #21
Hi,

On Fri, Nov 10, 2017 at 05:39:46PM +0100, François Kooman wrote:
> Parsing CSV can be cumbersome in depending software, so instead
> offer JSON as well using "status 4".
> 
> The "status 4" command uses the same keys/values as "status 2"
> and "status 3", just formatted as JSON.

So... we had a nice and long discussion about this last year, which I
think can be wrapped like this

 - JSON is nice because good parser support exist on the receiving end
   today

 - human readability is a plus for manually verifying output

 - if we do JSON, we should do it for management *and* status file

 - most voices argued for "do not require an external library"
   (for *creation*).  Not sure if we managed to convince David in 
   the end, though :-)

... but then it just died off, and the patch is sitting in patchwork
(where I moved it to "changes requested" just now).

Is there interest in moving forward, or shall we just let it go?

gert
François Kooman Oct. 7, 2018, 8:57 a.m. UTC | #22
On 10/7/18 9:10 AM, Gert Doering wrote:
> Hi,

Hi Gert,

>  - most voices argued for "do not require an external library"
>    (for *creation*).  Not sure if we managed to convince David in 
>    the end, though :-)

In the meantime I had some "great" experience with manually generating
JSON in another project. That was a terrible mistake ;-) So I would not
recommend following this path any longer. Maybe not even JSON, but
something typed and/or even binary.

> ... but then it just died off, and the patch is sitting in patchwork
> (where I moved it to "changes requested" just now).
> 
> Is there interest in moving forward, or shall we just let it go?

I'm not really interested in it anymore, and I brought it up ;-) So if
no one else is interested, maybe better let it go...

Cheers,
François

Patch

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 82a0b9d9..3e82431b 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1014,6 +1014,102 @@  multi_print_status(struct multi_context *m, struct status_output *so, const int
 
             status_printf(so, "END");
         }
+        else if (version == 4)
+        {
+            /*
+             * Status file version 4 (JSON)
+             */
+            status_printf(so, "{");
+            status_printf(so, "\"TITLE\": \"%s\",", title_string);
+            status_printf(so, "\"TIME\": \"%s\",", time_string(now, 0, false, &gc_top));
+            /* UNIX_TIME added here instead of making it part of TIME as well
+               as in status 2/3 */
+            status_printf(so, "\"UNIX_TIME\": %u,", (unsigned int)now);
+            status_printf(so, "\"CLIENT_LIST\": [");
+
+            /* JSON needs a "," between items, but not at the last position,
+               because I was unable to figure out how to determine if we
+               iterate over the last item, we inverse it by prepending
+               additional items with a comma */
+            int is_first = 1;
+            hash_iterator_init(m->hash, &hi);
+            while ((he = hash_iterator_next(&hi)))
+            {
+                struct gc_arena gc = gc_new();
+                const struct multi_instance *mi = (struct multi_instance *) he->value;
+
+                if (!mi->halt)
+                {
+                    if(0 == is_first) {
+                        status_printf(so, ",");
+                    } else {
+                        is_first = 0;
+                    }
+                    status_printf(so, "{");
+                    status_printf(so, "\"Common Name\": \"%s\",", tls_common_name(mi->context.c2.tls_multi, false));
+                    status_printf(so, "\"Real Address\": \"%s\",", mroute_addr_print(&mi->real, &gc));
+                    status_printf(so, "\"Virtual Address\": \"%s\",", print_in_addr_t(mi->reporting_addr, IA_EMPTY_IF_UNDEF, &gc));
+                    status_printf(so, "\"Virtual IPv6 Address\": \"%s\",", print_in6_addr(mi->reporting_addr_ipv6, IA_EMPTY_IF_UNDEF, &gc));
+                    status_printf(so, "\"Bytes Received\": " counter_format ",", mi->context.c2.link_read_bytes);
+                    status_printf(so, "\"Bytes Sent\": " counter_format ",", mi->context.c2.link_write_bytes);
+                    status_printf(so, "\"Connected Since\": \"%s\",", time_string(mi->created, 0, false, &gc));
+                    status_printf(so, "\"Connected Since (time_t)\": %u,", (unsigned int)mi->created);
+                    status_printf(so, "\"Username\": \"%s\",", tls_username(mi->context.c2.tls_multi, false));
+#ifdef MANAGEMENT_DEF_AUTH
+                    status_printf(so, "\"Client ID\": %lu,", mi->context.c2.mda_context.cid);
+#endif
+                    status_printf(so, "\"Peer ID\": %" PRIu32, mi->context.c2.tls_multi ? mi->context.c2.tls_multi->peer_id : UINT32_MAX);
+                    status_printf(so, "}");
+                }
+                gc_free(&gc);
+            }
+            hash_iterator_free(&hi);
+
+            status_printf(so, "],");
+            status_printf(so, "\"ROUTING_TABLE\": [");
+
+            is_first = 1;
+            hash_iterator_init(m->vhash, &hi);
+            while ((he = hash_iterator_next(&hi)))
+            {
+                struct gc_arena gc = gc_new();
+                const struct multi_route *route = (struct multi_route *) he->value;
+
+                if (multi_route_defined(m, route))
+                {
+                    const struct multi_instance *mi = route->instance;
+                    const struct mroute_addr *ma = &route->addr;
+                    char flags[2] = {0, 0};
+
+                    if (route->flags & MULTI_ROUTE_CACHE)
+                    {
+                        flags[0] = 'C';
+                    }
+                        if(0 == is_first) {
+                            status_printf(so, ",");
+                        } else {
+                            is_first = 0;
+                        }
+                        status_printf(so, "{\"Virtual Address\": \"%s%s\",", mroute_addr_print(ma, &gc), flags);
+                        status_printf(so, "\"Common Name\": \"%s\",", tls_common_name(mi->context.c2.tls_multi, false));
+                        status_printf(so, "\"Real Address\": \"%s\",", mroute_addr_print(&mi->real, &gc));
+                        status_printf(so, "\"Last Ref\": \"%s\",", time_string(route->last_reference, 0, false, &gc));
+                        status_printf(so, "\"Last Ref (time_t)\": %u}", (unsigned int)route->last_reference);
+                }
+                gc_free(&gc);
+            }
+            hash_iterator_free(&hi);
+            status_printf(so, "]");
+
+            if (m->mbuf)
+            {
+                status_printf(so, ",\"GLOBAL_STATS\": {");
+                status_printf(so, "\"Max bcast/mcast queue length\": %d", mbuf_maximum_queued(m->mbuf));
+                status_printf(so, "}");
+            }
+            status_printf(so, "}");
+            status_printf(so, "END");
+        }
         else
         {
             status_printf(so, "ERROR: bad status format version number");