Re: [PATCH 1/1] import: ignore file encoding for ovpn configuration file

[Dan Williams <dcbw redhat com>]

On Wed, 2016-03-02 at 12:53 +0100, Thomas Haller wrote:
> Openvpn treats ovpn files as ASCII configuration file and
> does not care about a specific certain encoding. As such,
> only encodings that are an extension of ASCII can work at
> all (like iso8859-* or utf8).
>
> We should not try to handle configuration files that cannot even
> be handled by openvpn itself.
>
> As regular options must be ASCII-compatbile, the encoding only
> matters for filenames and inline-blobs.
>
> Openvpn itself doesn't care about encoding of filenames and passes
> them directly to the system functions (open, access). The same is
> true
> for glib, which expects paths in "GLib file encoding".
> Nowaways, most Linux filesystems use utf8 encoding for paths.
> Therefore,
> if we would know the encoding of the file, we probably would want to
> convert the paths to utf8. However, how do we guess the right
> encoding?
> And what if the user *really* meant what is written in the
> configuration
> file? Note, that openvpn doesn't support escape sequences like
> "\344",
> thus, if the user really wanted to specify such a character, he is
> only
> able to do so if we don't mess with the encoding.
>
> Inline blobs usually are ASCII/base64 encoded. If they happen to be
> in a
> different encoding, we still want to preserve the original blob and
> not guess and convert encodings.
>
> The only sane option is ignoring the encoding and pretend it is
> ASCII compatible. Who writes non-utf8 configuration files anyway?

We have to make sure string data we pass through D-Bus (like in the
connection properties) is UTF-8 though.  So it doesn't need to be
converted or validated at some point, or dbus will kick the
editor/whatever off the bus when it tries to send the invalid data to
NM.

Dan

> ---
>  properties/import-export.c            | 17 -----------------
>  properties/tests/test-import-export.c |  3 +--
>  2 files changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/properties/import-export.c b/properties/import-export.c
> index 8fe0364..6e8159d 100644
> --- a/properties/import-export.c
> +++ b/properties/import-export.c
> @@ -647,7 +647,6 @@ do_import (const char *path, const char
> *contents, gsize contents_len, GError **
>       gs_free char *basename = NULL;
>       gs_free char *default_path = NULL;
>       char *tmp, *tmp2;
> -     gs_free char *new_contents = NULL;
>       const char *last_seen_key_direction = NULL;
>       gboolean have_certs, have_ca;
>       GSList *inline_blobs = NULL, *sl_iter;
> @@ -683,22 +682,6 @@ do_import (const char *path, const char
> *contents, gsize contents_len, GError **
>               *tmp = '\0';
>       g_object_set (s_con, NM_SETTING_CONNECTION_ID, basename,
> NULL);
>  
> -     if (!g_utf8_validate (contents, contents_len, NULL)) {
> -             GError *conv_error = NULL;
> -             gsize bytes_written;
> -
> -             new_contents = g_locale_to_utf8 (contents,
> contents_len, NULL, &bytes_written, &conv_error);
> -             if (conv_error) {
> -                     /* ignore the error, we tried at least. */
> -                     g_error_free (conv_error);
> -                     g_free (new_contents);
> -             } else {
> -                     g_assert (new_contents);
> -                     contents = new_contents;  /* update contents
> with the UTF-8 safe text */
> -                     contents_len = bytes_written + 1;
> -             }
> -     }
> -
>       if (strncmp (contents, "\xEF\xBB\xBF", 3) == 0) {
>               /* skip over UTF-8 BOM */
>               contents += 3;
> diff --git a/properties/tests/test-import-export.c
> b/properties/tests/test-import-export.c
> index b2a8f0f..96ca13b 100644
> --- a/properties/tests/test-import-export.c
> +++ b/properties/tests/test-import-export.c
> @@ -494,7 +494,6 @@ test_non_utf8_import (void)
>       NMConnection *connection;
>       NMSettingConnection *s_con;
>       NMSettingVpn *s_vpn;
> -     const char *expected_cacert = "Attätaenko.pem";
>       char *expected_path;
>       const char *charset = NULL;
>  
> @@ -515,7 +514,7 @@ test_non_utf8_import (void)
>       s_vpn = nm_connection_get_setting_vpn (connection);
>       g_assert (s_vpn);
>  
> -     expected_path = g_strdup_printf ("%s/%s", SRCDIR,
> expected_cacert);
> +     expected_path = g_strdup_printf ("%s/%s", SRCDIR,
> "Att\344taenko.pem");
>       _check_item (s_vpn, NM_OPENVPN_KEY_CA, expected_path);
>       g_free (expected_path);
>