Re: Simplify OpenVPN blob handling

2016-01-26 Thread Simon Geard
On Tue, 2016-01-26 at 11:51 +, David Woodhouse wrote:
> It does even make a little bit of sense, if the most sensitive item
> on the computer in question *is* the VPN certificate

That would certainly be the case for my VPN setup... it's just there so
I can access the work network from my personal machine at home. All the
sensitive stuff is being accessed through remote-desktop or ssh, so
there's nothing confidential on the local machine.

Simon.

signature.asc
Description: This is a digitally signed message part
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH v3] settings: Resolve path if hostname is a sym-link

2016-01-26 Thread Joel Holdsworth
If the hostname file is a symbolic link, follow it to find where the
real file is located, otherwise g_file_set_contents will attempt to
replace the link with a plain file.
---
 src/settings/nm-settings.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index f6f8c37..688da2f 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -1532,11 +1532,11 @@ write_hostname (NMSettingsPrivate *priv, const char 
*hostname)
char *hostname_eol;
gboolean ret;
gs_free_error GError *error = NULL;
-   const char *file = priv->hostname.file;
+   char *file = priv->hostname.file, *link_path = NULL;
gs_unref_variant GVariant *var = NULL;
+   struct stat file_stat = { .st_mode = 0 };
 #if HAVE_SELINUX
security_context_t se_ctx_prev = NULL, se_ctx = NULL;
-   struct stat file_stat = { .st_mode = 0 };
mode_t st_mode = 0;
 #endif
 
@@ -1554,6 +1554,14 @@ write_hostname (NMSettingsPrivate *priv, const char 
*hostname)
return !error;
}
 
+   /* If the hostname file is a symbolic link, follow it to find where the
+* real file is located, otherwise g_file_set_contents will attempt to
+* replace the link with a plain file.
+*/
+   if (lstat (file, &file_stat) == 0 && S_ISLNK (file_stat.st_mode) &&
+   (link_path = g_file_read_link (file, NULL)))
+   file = link_path;
+
 #if HAVE_SELINUX
/* Get default context for hostname file and set it for fscreate */
if (stat (file, &file_stat) == 0)
@@ -1584,6 +1592,7 @@ write_hostname (NMSettingsPrivate *priv, const char 
*hostname)
 #endif
 
g_free (hostname_eol);
+   g_free (link_path);
 
if (!ret) {
nm_log_warn (LOGD_SETTINGS, "Could not save hostname to %s: 
%s", file, error->message);
-- 
1.9.1

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH] simplify blob handling

2016-01-26 Thread Matthias Berndt
Hi,

here's the patch to simplify blob handling.

Cheers,
Matthias
>From 469b5974cb539a7cfc2d79b489f58e98e79424dc Mon Sep 17 00:00:00 2001
From: Matthias Berndt 
Date: Tue, 26 Jan 2016 22:55:24 +0100
Subject: [PATCH] simplify blob handling

---
 properties/import-export.c | 29 +
 1 file changed, 1 insertion(+), 28 deletions(-)

diff --git a/properties/import-export.c b/properties/import-export.c
index 3adcf3e..0107fb6 100644
--- a/properties/import-export.c
+++ b/properties/import-export.c
@@ -246,8 +246,6 @@ handle_blob_item (const char ***line,
   GError **error)
 {
 	gboolean success = FALSE;
-	const char *blob_mark_start, *blob_mark_end;
-	const char *blob_mark_start2 = NULL, *blob_mark_end2 = NULL;
 	const char *start_tag, *end_tag;
 	char *filename = NULL;
 	char *dirname = NULL;
@@ -267,25 +265,15 @@ handle_blob_item (const char ***line,
 	if (!strcmp (key, NM_OPENVPN_KEY_CA)) {
 		start_tag = CA_BLOB_START_TAG;
 		end_tag = CA_BLOB_END_TAG;
-		blob_mark_start = CERT_BEGIN;
-		blob_mark_end = CERT_END;
 	} else if (!strcmp (key, NM_OPENVPN_KEY_CERT)) {
 		start_tag = CERT_BLOB_START_TAG;
 		end_tag = CERT_BLOB_END_TAG;
-		blob_mark_start = CERT_BEGIN;
-		blob_mark_end = CERT_END;
 	} else if (!strcmp (key, NM_OPENVPN_KEY_TA)) {
 		start_tag = TLS_AUTH_BLOB_START_TAG;
 		end_tag = TLS_AUTH_BLOB_END_TAG;
-		blob_mark_start = STATIC_KEY_BEGIN;
-		blob_mark_end = STATIC_KEY_END;
 	} else if (!strcmp (key, NM_OPENVPN_KEY_KEY)) {
 		start_tag = KEY_BLOB_START_TAG;
 		end_tag = KEY_BLOB_END_TAG;
-		blob_mark_start = PRIV_KEY_BEGIN;
-		blob_mark_end = PRIV_KEY_END;
-		blob_mark_start2 = RSA_PRIV_KEY_BEGIN;
-		blob_mark_end2 = RSA_PRIV_KEY_END;
 	} else
 		g_return_val_if_reached (FALSE);
 	p = *line;
@@ -294,25 +282,14 @@ handle_blob_item (const char ***line,
 
 	NEXT_LINE;
 
-	if (blob_mark_start2 && !strcmp (*p, blob_mark_start2)) {
-		blob_mark_start = blob_mark_start2;
-		blob_mark_end = blob_mark_end2;
-	} else if (strcmp (*p, blob_mark_start))
-		goto finish;
-
-	NEXT_LINE;
 	in_file = g_string_new (NULL);
 
-	while (*p && strcmp (*p, blob_mark_end)) {
+	while (*p && strcmp (*p, end_tag)) {
 		g_string_append (in_file, *p);
 		g_string_append_c (in_file, '\n');
 		NEXT_LINE;
 	}
 
-	NEXT_LINE;
-	if (strncmp (*p, end_tag, strlen (end_tag)))
-		goto finish;
-
 	/* Construct file name to write the data in */
 	filename = g_strdup_printf ("%s-%s.pem", name, key);
 	dirname = g_build_filename (g_get_home_dir (), ".cert", NULL);
@@ -328,15 +305,11 @@ handle_blob_item (const char ***line,
 	}
 
 	/* Write the new file */
-	g_string_prepend_c (in_file, '\n');
-	g_string_prepend (in_file, blob_mark_start);
-	g_string_append_printf (in_file, "%s\n", blob_mark_end);
 	success = g_file_set_contents (path, in_file->str, -1, error);
 	if (!success)
 		goto finish;
 
 	nm_setting_vpn_add_data_item (s_vpn, key, path);
-
 finish:
 	*line = p;
 	g_free (filename);
-- 
2.5.0

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH v2] platform: ignore permanent MAC addresses of all ones (FF:FF:FF:FF:FF:FF)

2016-01-26 Thread Dan Williams
On Tue, 2016-01-26 at 13:41 -0500, Chuck Anderson wrote:
> While we're on this topic, what about other types of invalid MAC
> addresses?  Like ones with the Multicast bit set (odd first octet),
> or
> ones with the LAA (Locallay Administered Address) bit set (2nd least
> significant bit of first octet, e,g, x2: x6: xa: xe:).
> 
> For Multicast Ethernet addresses, I don't see how that could ever
> work
> and it would screw up any network it gets attached to because normal
> switches can't learn them.
> 
> For LAA, they aren't supposed to be permanently programmed into a
> device, but there are unfortunately cheap devices out there that use
> non-IEEE-registered OUIs, some of them LAA, so it may be
> unfortunately
> unworkable to ban those.  Network bridge devices and other software
> devices may also use them.

Yes, Multicast should be rejected too.  There are also some older
drivers that used special addresses to indicate some states that we
used to ignore as well.

But I think we need to allow LAA if the device reports it :(

Dan

> On Tue, Jan 26, 2016 at 11:33:49AM -0600, Dan Williams wrote:
> > Drivers are stupid, and just like the platform ignores an all zeros
> > permanent address, so should it ignore all ones.
> > 
> > NetworkManager[509]:  [1453743778.854919] [devices/nm
> > -device.c:8885] nm_device_update_hw_address(): [0x190370] (eth0):
> > hardware address now 86:18:52:xx:xx:xx
> > NetworkManager[509]:  [1453743778.855438] [devices/nm
> > -device.c:9138] constructed(): [0x190370] (eth0): read initial MAC
> > address 86:18:52:xx:xx:xx
> > NetworkManager[509]:  [1453743778.861602] [devices/nm
> > -device.c:9148] constructed(): [0x190370] (eth0): read permanent
> > MAC address FF:FF:FF:FF:FF:FF
> > ---
> >  src/platform/nm-platform-utils.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm
> > -platform-utils.c
> > index 953ac8c..b0b0b56 100644
> > --- a/src/platform/nm-platform-utils.c
> > +++ b/src/platform/nm-platform-utils.c
> > @@ -142,7 +142,8 @@ nmp_utils_ethtool_get_permanent_address (const
> > char *ifname,
> > struct ethtool_perm_addr e;
> > guint8 _extra_data[NM_UTILS_HWADDR_LEN_MAX + 1];
> > } edata;
> > -   guint zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
> > +   static const guint8 zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0
> > };
> > +   static guint8 ones[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
> >  
> > if (!ifname)
> > return FALSE;
> > @@ -161,6 +162,12 @@ nmp_utils_ethtool_get_permanent_address (const
> > char *ifname,
> > if (memcmp (edata.e.data, zeros, edata.e.size) == 0)
> > return FALSE;
> >  
> > +   /* Some drivers return a permanent address of all ones.
> > Reject that too */
> > +   if (G_UNLIKELY (ones[0] != 0xFF))
> > +   memset (ones, 0xFF, sizeof (ones));
> > +   if (memcmp (edata.e.data, ones, edata.e.size) == 0)
> > +   return FALSE;
> > +
> > memcpy (buf, edata.e.data, edata.e.size);
> > *length = edata.e.size;
> > return TRUE;
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH] wwan: retry connect on some errors and save them for log messages

2016-01-26 Thread Dan Williams
First, cb751012a2f4b8ef236eab2a7c65687c99205806 mistakenly converted the
act_stage_context_step() in connect_ready() to connect_context_clear()
instead of connect_context_step().  This would cause the IP Type retry
logic to fail and no further types to be tried.  It also throws
away the ctx->first_error and causes all errors that MM returns on the
connect attempt to be dropped on the floor.

Second, not all errors should cause an advance to the next IP Type,
since some errors aren't related to it.  Specifically, MM_CORE_ERROR_RETRY
when using Simple.Connect() means that a timeout was reached
in the internal connect logic, not a modem or network error.  In
that case, try the connect again with the same IP Type before advancing
to the next type.

Fixes: cb751012a2f4b8ef236eab2a7c65687c99205806
---
 src/devices/wwan/nm-modem-broadband.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/src/devices/wwan/nm-modem-broadband.c 
b/src/devices/wwan/nm-modem-broadband.c
index 427f3ed..44ac1021 100644
--- a/src/devices/wwan/nm-modem-broadband.c
+++ b/src/devices/wwan/nm-modem-broadband.c
@@ -52,6 +52,7 @@ typedef struct {
MMSimpleConnectProperties *connect_properties;
GArray *ip_types;
guint ip_types_i;
+   guint ip_type_tries;
GError *first_error;
 } ConnectContext;
 
@@ -331,11 +332,17 @@ connect_ready (MMModemSimple *simple_iface,
} else
g_error_free (error);
 
-   /* If the modem/provider lies and the IP type we tried isn't 
supported,
-* retry with the next one, if any.
-*/
-   ctx->ip_types_i++;
-   connect_context_clear (self);
+   if (ctx->ip_type_tries == 0 && g_error_matches (error, 
MM_CORE_ERROR, MM_CORE_ERROR_RETRY)) {
+   /* Try one more time */
+   ctx->ip_type_tries++;
+   } else {
+   /* If the modem/provider lies and the IP type we tried 
isn't supported,
+* retry with the next one, if any.
+*/
+   ctx->ip_types_i++;
+   ctx->ip_type_tries = 0;
+   }
+   connect_context_step (self);
return;
}
 
@@ -485,9 +492,10 @@ connect_context_step (NMModemBroadband *self)
else
g_assert_not_reached ();
 
-   nm_log_dbg (LOGD_MB, "(%s): launching connection with 
ip type '%s'",
+   nm_log_dbg (LOGD_MB, "(%s): launching connection with 
ip type '%s' (try %d)",
nm_modem_get_uid (NM_MODEM (self)),
-   nm_modem_ip_type_to_string (current));
+   nm_modem_ip_type_to_string (current),
+   ctx->ip_type_tries + 1);
 
mm_modem_simple_connect (self->priv->simple_iface,
 ctx->connect_properties,
-- 
2.4.3
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH v2] platform: ignore permanent MAC addresses of all ones (FF:FF:FF:FF:FF:FF)

2016-01-26 Thread Chuck Anderson
While we're on this topic, what about other types of invalid MAC
addresses?  Like ones with the Multicast bit set (odd first octet), or
ones with the LAA (Locallay Administered Address) bit set (2nd least
significant bit of first octet, e,g, x2: x6: xa: xe:).

For Multicast Ethernet addresses, I don't see how that could ever work
and it would screw up any network it gets attached to because normal
switches can't learn them.

For LAA, they aren't supposed to be permanently programmed into a
device, but there are unfortunately cheap devices out there that use
non-IEEE-registered OUIs, some of them LAA, so it may be unfortunately
unworkable to ban those.  Network bridge devices and other software
devices may also use them.

On Tue, Jan 26, 2016 at 11:33:49AM -0600, Dan Williams wrote:
> Drivers are stupid, and just like the platform ignores an all zeros
> permanent address, so should it ignore all ones.
> 
> NetworkManager[509]:  [1453743778.854919] [devices/nm-device.c:8885] 
> nm_device_update_hw_address(): [0x190370] (eth0): hardware address now 
> 86:18:52:xx:xx:xx
> NetworkManager[509]:  [1453743778.855438] [devices/nm-device.c:9138] 
> constructed(): [0x190370] (eth0): read initial MAC address 86:18:52:xx:xx:xx
> NetworkManager[509]:  [1453743778.861602] [devices/nm-device.c:9148] 
> constructed(): [0x190370] (eth0): read permanent MAC address FF:FF:FF:FF:FF:FF
> ---
>  src/platform/nm-platform-utils.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/platform/nm-platform-utils.c 
> b/src/platform/nm-platform-utils.c
> index 953ac8c..b0b0b56 100644
> --- a/src/platform/nm-platform-utils.c
> +++ b/src/platform/nm-platform-utils.c
> @@ -142,7 +142,8 @@ nmp_utils_ethtool_get_permanent_address (const char 
> *ifname,
>   struct ethtool_perm_addr e;
>   guint8 _extra_data[NM_UTILS_HWADDR_LEN_MAX + 1];
>   } edata;
> - guint zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
> + static const guint8 zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
> + static guint8 ones[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
>  
>   if (!ifname)
>   return FALSE;
> @@ -161,6 +162,12 @@ nmp_utils_ethtool_get_permanent_address (const char 
> *ifname,
>   if (memcmp (edata.e.data, zeros, edata.e.size) == 0)
>   return FALSE;
>  
> + /* Some drivers return a permanent address of all ones. Reject that too 
> */
> + if (G_UNLIKELY (ones[0] != 0xFF))
> + memset (ones, 0xFF, sizeof (ones));
> + if (memcmp (edata.e.data, ones, edata.e.size) == 0)
> + return FALSE;
> +
>   memcpy (buf, edata.e.data, edata.e.size);
>   *length = edata.e.size;
>   return TRUE;
> -- 
> 2.4.3
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


[PATCH v2] platform: ignore permanent MAC addresses of all ones (FF:FF:FF:FF:FF:FF)

2016-01-26 Thread Dan Williams
Drivers are stupid, and just like the platform ignores an all zeros
permanent address, so should it ignore all ones.

NetworkManager[509]:  [1453743778.854919] [devices/nm-device.c:8885] 
nm_device_update_hw_address(): [0x190370] (eth0): hardware address now 
86:18:52:xx:xx:xx
NetworkManager[509]:  [1453743778.855438] [devices/nm-device.c:9138] 
constructed(): [0x190370] (eth0): read initial MAC address 86:18:52:xx:xx:xx
NetworkManager[509]:  [1453743778.861602] [devices/nm-device.c:9148] 
constructed(): [0x190370] (eth0): read permanent MAC address FF:FF:FF:FF:FF:FF
---
 src/platform/nm-platform-utils.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm-platform-utils.c
index 953ac8c..b0b0b56 100644
--- a/src/platform/nm-platform-utils.c
+++ b/src/platform/nm-platform-utils.c
@@ -142,7 +142,8 @@ nmp_utils_ethtool_get_permanent_address (const char *ifname,
struct ethtool_perm_addr e;
guint8 _extra_data[NM_UTILS_HWADDR_LEN_MAX + 1];
} edata;
-   guint zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
+   static const guint8 zeros[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
+   static guint8 ones[NM_UTILS_HWADDR_LEN_MAX] = { 0 };
 
if (!ifname)
return FALSE;
@@ -161,6 +162,12 @@ nmp_utils_ethtool_get_permanent_address (const char 
*ifname,
if (memcmp (edata.e.data, zeros, edata.e.size) == 0)
return FALSE;
 
+   /* Some drivers return a permanent address of all ones. Reject that too 
*/
+   if (G_UNLIKELY (ones[0] != 0xFF))
+   memset (ones, 0xFF, sizeof (ones));
+   if (memcmp (edata.e.data, ones, edata.e.size) == 0)
+   return FALSE;
+
memcpy (buf, edata.e.data, edata.e.size);
*length = edata.e.size;
return TRUE;
-- 
2.4.3
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH v2] dns: clean up error paths in dns-manager

2016-01-26 Thread Dan Williams
On Mon, 2016-01-25 at 22:59 +0100, Beniamino Galvani wrote:
> On Mon, Jan 25, 2016 at 12:59:22PM -0600, Dan Williams wrote:
> > From 2370f508df8400b424fec032f8314cea97fdbaef Mon Sep 17 00:00:00
> > 2001
> > From: Dan Williams 
> > Date: Wed, 20 Jan 2016 13:52:59 -0600
> > Subject: [PATCH] dns: clean up error paths in dns-manager
> > 
> > Specifically for resolvconf, if the write succeeded, but the
> > pclose()
> > failed error would be left NULL and SR_ERROR would be returned,
> > which
> > caused a crash in nm_dns_manager_end_updates().
> > ---
> > v2: fixed thaller's comments except for the '\n' which I'll do in
> > another patch
> 
> Looks good to me.
> 

Pushed along with the newline cleanup Thomas requested.

Dan
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH v2] dns: clean up error paths in dns-manager

2016-01-26 Thread Thomas Haller
On Mon, 2016-01-25 at 12:59 -0600, Dan Williams wrote:
> From 2370f508df8400b424fec032f8314cea97fdbaef Mon Sep 17 00:00:00
> 2001
> From: Dan Williams 
> Date: Wed, 20 Jan 2016 13:52:59 -0600
> Subject: [PATCH] dns: clean up error paths in dns-manager
> 
> Specifically for resolvconf, if the write succeeded, but the pclose()
> failed error would be left NULL and SR_ERROR would be returned, which
> caused a crash in nm_dns_manager_end_updates().
> ---
> v2: fixed thaller's comments except for the '\n' which I'll do in
> another patch
> 

lgtm

Thomas

signature.asc
Description: This is a digitally signed message part
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: Simplify OpenVPN blob handling

2016-01-26 Thread David Woodhouse
On Tue, 2016-01-26 at 10:01 +0100, Matthias Berndt wrote:
> 
> 
> > OTOH if she is keeping her cert deliberately secure on an encrypted USB
> > storage device, and it gets copied to the unencrypted hard drive, she
> > might not be able to connect tomorrow because she's been *fired* for
> > this breach of security policy.

> What kind of security policy requires you to encrypt your USB drives
> but not your hard drive? That seems contrived to me. Besides, we
> already copy certificates if they are stored as blobs inside the
> .ovpn file - I think it's better to be consistent here.

Arguably a daft one. But I've seen stupider :)

It does even make a little bit of sense, if the most sensitive item on
the computer in question *is* the VPN certificate (perhaps it's used
*purely* for accessing web apps and there's no local storage of
anything sensitive at all — or local storage *is* carefully managed by
other means, and you blindly copying a cert to where *you* choose does
not fit in with that solution.

Fundamentally though, the principle is simple: You *don't* copy
sensitive data from where it originally resides.

Whoever created the OpenVPN blob already had a choice of whether to
pass the certificate by value (embedded in the blob) or by reference (a
filename or PKCS#11 URI).

In the latter case, where they have chosen to pass it by reference, it
seems entirely wrong to take a *copy* instead of just abiding by their
choice.

> > And if her cert expires and she renews it, even if she is still
> > employed, she's going to get very confused when NM is still using the
> > *old* certificate that she's *deleted* from the USB stick and replaced
> > with a new one.

> Either she is technical enough to generate her own keys and
> certificates, in that case it'll be trivial for her to update her
> NetworkManager settings accordingly. Or she's not, in which case her
> administrator will give her a USB stick with the new configuration
> and she'll import it just as she did before. I think that from a
> "normal user" pov, copying is definitely what I'd expect. I certainly
> did.

In my case, we have scripts which talk to the corporate PKI
infrastructure and obtain a certificate; placing it in a known
location.

We configure our VPN to use the certificate from there, and definitely
*not* to use a copy of it. Then when the certificate is renewed, the
VPN keeps working. Unless NetworkManager did something daft and took a
*copy*...

> > If you do this, make it *optional* and make it clear that you're doing
> > it.
> How to do that?

Actually, I take it back. It's *already* optional, as I noted above. If
the blob contains it by value, then by all means store it where you
like. If the blob has a *reference* to an existing file or PKCS#11 URI,
then do not copy it ever.

> > And in fact, do *not* import it to a file elsewhere; import it into
> > gnome-keyring and refer to it by its PKCS#11 URI.
>
> Yeah, except she may well not be using gnome. We might be able to
> come up with something based on the freedesktop secret service api,
> I'll look into it.

Yeah. Import it into "the default writeable token". Which might even be
an NSS database ~/.pki/nssdb. I don't think the secret service API is
relevant here; this is all through PKCS#11 and managed by p11-kit.

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH v2] settings: Resolve path if hostname is a sym-link

2016-01-26 Thread Thomas Haller
On Mon, 2016-01-25 at 22:57 +, Joel Holdsworth wrote:
> If the hostname file is a symbolic link, follow it to find where the
> real file is located, otherwise g_file_set_contents will attempt to
> replace the link with a plain file.
> ---
>  src/settings/nm-settings.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)


Hi Joel,



> diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
> index f6f8c37..fc2eb29 100644
> --- a/src/settings/nm-settings.c
> +++ b/src/settings/nm-settings.c
> @@ -1532,11 +1532,11 @@ write_hostname (NMSettingsPrivate *priv,
> const char *hostname)
>   char *hostname_eol;
>   gboolean ret;
>   gs_free_error GError *error = NULL;
> - const char *file = priv->hostname.file;
> + char *file = g_strdup(priv->hostname.file), *link_path;

Whitespace between "g_strdup" and "(".


This now leaks @file.

Better do:

        const char *file = priv->hostname.file;
        gs_free char *link_path = NULL;


and later:

if (   lstat (file, &file_stat) == 0
&& S_ISLNK  (file_stat.st_mode)
&& (link_path = g_file_read_link (file, NULL)))
file = link_path;

which also avoid the additional copy.


>   gs_unref_variant GVariant *var = NULL;
> + struct stat file_stat = { .st_mode = 0 };
>  #if HAVE_SELINUX
>   security_context_t se_ctx_prev = NULL, se_ctx = NULL;
> - struct stat file_stat = { .st_mode = 0 };
>   mode_t st_mode = 0;
>  #endif
>  
> @@ -1554,6 +1554,16 @@ write_hostname (NMSettingsPrivate *priv, const
> char *hostname)
>   return !error;
>   }
>  



Anyway, why do we event want this? It's not clear to me that this is
desired behavior.

If NM is configured to overwrite /etc/hostname, with /etc/hostname
symlinking to somewhere else (e.g. /var/run/some-daemon/hostname), then
I don't think that NM should overwrite the file owned by "some-daemon"
but always rewrite /etc/hostname.

Note that for /etc/resolv.conf, NM only writes to
/var/run/NetworkManager/resolv.conf without redirecting the symlink in
/etc.
Maybe we should do something similar with /etc/hostname, but then it
seems to me that hostnamed is the future to manage the hostname.


ciao
Thomas


signature.asc
Description: This is a digitally signed message part
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [PATCH] platform: ignore permanent MAC addresses of all ones (FF:FF:FF:FF:FF:FF)

2016-01-26 Thread Bjørn Mork
Dan Williams  writes:

> Drivers are stupid,

They don't *have* to be, you know :)

I did wonder if we should make the usbnet framework reject such
addresses after seeing this a while ago:

Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 2 Communications
  bInterfaceSubClass 13 
  bInterfaceProtocol  0 
  iInterface  5 Sierra Wireless EM7345 4G LTE (NCM)
  CDC Header:
bcdCDC   1.20
  CDC Union:
bMasterInterface0
bSlaveInterface 1 
  CDC NCM:
bcdNcmVersion1.00
bmNetworkCapabilities 0x00
  CDC Ethernet:
iMacAddress  6 
bmEthernetStatistics0x
wMaxSegmentSize   1514
wNumberMCFilters0x
bNumberPowerFilters  0
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0040  1x 64 bytes
bInterval   4


but I ended up letting it be, considering that if the vendor meant this
to work then they would probably have provided a more useful address...

Note that this address is an USB string descriptor served to us by the
firmware, so it's not like we're reading some uninitialized register
here.  Directly at least - the problem can of course be that the
firmware translates some uninitialized register into a string with no
sanity checking.

Anyway, just thought I'd mention this example in case you are in favour
of letting the driver fix this up.  We can do that, if smarter drivers
are on the wishlist :)


Bjørn
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: Simplify OpenVPN blob handling

2016-01-26 Thread Matthias Berndt



>OTOH if she is keeping her cert deliberately secure on an encrypted USB
>storage device, and it gets copied to the unencrypted hard drive, she
>might not be able to connect tomorrow because she's been *fired* for
>this breach of security policy.
What kind of security policy requires you to encrypt your USB drives but not 
your hard drive? That seems contrived to me. Besides, we already copy 
certificates if they are stored as blobs inside the .ovpn file - I think it's 
better to be consistent here.

>And if her cert expires and she renews it, even if she is still
>employed, she's going to get very confused when NM is still using the
>*old* certificate that she's *deleted* from the USB stick and replaced
>with a new one.
Either she is technical enough to generate her own keys and certificates, in 
that case it'll be trivial for her to update her NetworkManager settings 
accordingly. Or she's not, in which case her administrator will give her a USB 
stick with the new configuration and she'll import it just as she did before. I 
think that from a "normal user" pov, copying is definitely what I'd expect. I 
certainly did.

>If you do this, make it *optional* and make it clear that you're doing
>it.
How to do that?

>And in fact, do *not* import it to a file elsewhere; import it into
>gnome-keyring and refer to it by its PKCS#11 URI.
Yeah, except she may well not be using gnome. We might be able to come up with 
something based on the freedesktop secret service api, I'll look into it.
>cf. https://bugzilla.gnome.org/show_bug.cgi?id=679860

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list