Bug 1308974

Summary: NetworkManager does not correctly set hostname obtained from DNS
Product: [Fedora] Fedora Reporter: Petr Spacek <pspacek>
Component: NetworkManagerAssignee: Beniamino Galvani <bgalvani>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 24CC: bgalvani, dcbw, lkundrak, lrintel, psimerda, thaller
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: NetworkManager-1.2.0-0.7.beta2.fc24 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-03-26 18:01:52 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Full output from "journalctl -b"
none
[PATCH] core: use hostnamed to set the transient hostname none

Description Petr Spacek 2016-02-16 15:45:02 UTC
Created attachment 1127619 [details]
Full output from "journalctl -b"

Description of problem:
After a upgrade to Fedora 24, machine is no longer getting its hostname set from PTR record in DNS.

Version-Release number of selected component (if applicable):
NetworkManager-1.2.0-0.6.beta1.fc24.x86_64

How reproducible:
100 %

Steps to Reproduce:
1. do not configure any hostname in the machine
2. boot the machine in a network where DNS contains a hostname in the reverse record for IPv4 address

Actual results:
NetworkManager[892]: <warn>  couldn't set the system hostname to 'vm-058-011.abc.idm.lab.eng.brq.redhat.com': (1) Operation not permitted
NetworkManager[892]: <warn>  You should use hostnamed when systemd hardening is in effect!


Expected results:
The hostname is set.


Additional info:
Audit log does not show any SELinux AVC. Switching SELinux to permissive mode does not help.

Full output from "journalctl -b" is attached.

Comment 1 Beniamino Galvani 2016-02-19 10:16:25 UTC
Created attachment 1128501 [details]
[PATCH] core: use hostnamed to set the transient hostname

Comment 2 Dan Williams 2016-02-20 16:08:29 UTC
In nm_settings_set_transient_hostname() the variable 'var' is unused.

Might as well use gs_free_error for 'error' in set_transient_hostname_done() too, and you could use gs_free for 'info', and gs_unref_variant for 'result' too.

Also, since set_system_hostname() only gets called from one place now, and your patch reduces its size, maybe we should just move that code into _set_hostname()?

Comment 3 Beniamino Galvani 2016-02-22 09:52:58 UTC
(In reply to Dan Williams from comment #2)
> In nm_settings_set_transient_hostname() the variable 'var' is unused.
> 
> Might as well use gs_free_error for 'error' in set_transient_hostname_done()
> too, and you could use gs_free for 'info', and gs_unref_variant for 'result'
> too.
> 
> Also, since set_system_hostname() only gets called from one place now, and
> your patch reduces its size, maybe we should just move that code into
> _set_hostname()?

Fixed and pushed 2 additional commits to bg/sethostname-hostnamed-rh1308974.

Comment 4 Dan Williams 2016-02-22 22:40:41 UTC
> core: use hostnamed to set the transient hostname

set_system_hostname() doesn't need a return; at the end of the function anymore since it's not returning anything.

Also, instead of strdup-ing the hostname in set_system_hostname(), I'd do that in nm_settings_set_transient_hostname() when creating the info structure, and then free it in set_transient_hostname_done().  That way the callback itself doesn't have to care, and the thing that might do the asynchronous operation is responsible for alloc/free.

rest looks good to me!

Comment 5 Beniamino Galvani 2016-02-23 22:17:17 UTC
(In reply to Dan Williams from comment #4)
> Also, instead of strdup-ing the hostname in set_system_hostname(), I'd do
> that in nm_settings_set_transient_hostname() when creating the info
> structure, and then free it in set_transient_hostname_done().  That way the
> callback itself doesn't have to care, and the thing that might do the
> asynchronous operation is responsible for alloc/free.

Makes sense, thanks. Branch updated.

Comment 6 Jan Kurik 2016-02-24 15:22:33 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 24 development cycle.
Changing version to '24'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora24#Rawhide_Rebase

Comment 7 Thomas Haller 2016-03-01 15:55:35 UTC
+typedef void (*NMSettingsSetHostnameCb) (const char *name, gboolean result);

when having a callback, it requires a user-data argument.

Comment 8 Beniamino Galvani 2016-03-02 09:50:38 UTC
(In reply to Thomas Haller from comment #7)
> +typedef void (*NMSettingsSetHostnameCb) (const char *name, gboolean result);
> 
> when having a callback, it requires a user-data argument.

Fixed and repushed.

Comment 9 Dan Williams 2016-03-03 21:05:35 UTC
LGTM now.

Comment 10 Thomas Haller 2016-03-04 07:26:18 UTC
Maybe use g_slice allocator for SetHostnameInfo and replace "!strlen (new_hostname)" by "!new_hostname[0]" 


Otherwise, lgtm.

Comment 11 Beniamino Galvani 2016-03-04 13:41:02 UTC
(In reply to Thomas Haller from comment #10)
> Maybe use g_slice allocator for SetHostnameInfo

Probably it doesn't make much difference since the function is called rarely.

> and replace "!strlen (new_hostname)" by "!new_hostname[0]" 

Changed and merged to master as:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=0fe8a557f80ddcebbf637c585d02cebd0bd735a2

Comment 12 Fedora Update System 2016-03-22 10:32:40 UTC
NetworkManager-1.2.0-0.7.beta2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-e4d2697a0b

Comment 13 Fedora Update System 2016-03-22 10:32:54 UTC
NetworkManager-1.2.0-0.7.beta2.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-e4d2697a0b

Comment 14 Fedora Update System 2016-03-22 16:56:06 UTC
NetworkManager-1.2.0-0.7.beta2.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-e4d2697a0b

Comment 15 Fedora Update System 2016-03-26 18:01:45 UTC
NetworkManager-1.2.0-0.7.beta2.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.