Patchwork [OE-devel] ntp: Resolve some abnormal behaviors

login
register
mail settings
Submitter Xufeng Zhang
Date May 31, 2013, 6:18 a.m.
Message ID <1369981125-9597-1-git-send-email-xufeng.zhang@windriver.com>
Download mbox | patch
Permalink /patch/50919/
State New, archived
Headers show

Comments

Xufeng Zhang - May 31, 2013, 6:18 a.m.
The main changes include:
1). Add ntp:ntp(user:group) to system.
2). Running ntpd dameon as ntp:ntp.
3). Move relevant files from /usr/bin to /usr/sbin.
4). Add crypto support.

[YOCTO #4567]
[ CQID: WIND00417282 ]

Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
---
 meta-networking/recipes-support/ntp/files/ntpd    |    8 ++++----
 meta-networking/recipes-support/ntp/files/ntpdate |    6 +++---
 meta-networking/recipes-support/ntp/ntp.inc       |   20 ++++++++++++--------
 3 files changed, 19 insertions(+), 15 deletions(-)
Paul Eggleton - May 31, 2013, 8:34 a.m.
On Friday 31 May 2013 14:18:45 Xufeng Zhang wrote:
> The main changes include:
> 1). Add ntp:ntp(user:group) to system.
> 2). Running ntpd dameon as ntp:ntp.

> 3). Move relevant files from /usr/bin to /usr/sbin.

I'm not sure but I think the way this has been done (--with-binsubdir=sbin) is 
going to bypass the ability to set sbindir in the distro config. If that's the 
case then I don't think this is a good change, although I appreciate having 
ntpd in /usr/sbin by default would be more appropriate.

> 4). Add crypto support.

The support is already there, it's just off by default and that was 
intentional. Those that need this can easily enable it in their distro config.

Cheers,
Paul
Xufeng Zhang - May 31, 2013, 9:08 a.m.
On 05/31/2013 04:34 PM, Paul Eggleton wrote:
> On Friday 31 May 2013 14:18:45 Xufeng Zhang wrote:
>    
>> The main changes include:
>> 1). Add ntp:ntp(user:group) to system.
>> 2). Running ntpd dameon as ntp:ntp.
>>      
>    
>> 3). Move relevant files from /usr/bin to /usr/sbin.
>>      
>    
Thanks for your review, Paul!

> I'm not sure but I think the way this has been done (--with-binsubdir=sbin) is
> going to bypass the ability to set sbindir in the distro config. If that's the
> case then I don't think this is a good change, although I appreciate having
> ntpd in /usr/sbin by default would be more appropriate.
>    
But currently it's the proper and easy way to do that, right?
I can think out any better way to do the same job.

>    
>> 4). Add crypto support.
>>      
> The support is already there, it's just off by default and that was
> intentional. Those that need this can easily enable it in their distro config.
>    
I'm not sure why we do that intentional, but shouldn't the package run
as secure as possible by default?


Thanks,
Xufeng

> Cheers,
> Paul
>
>
Xufeng Zhang - May 31, 2013, 9:14 a.m.
On 05/31/2013 05:08 PM, Xufeng Zhang wrote:
> On 05/31/2013 04:34 PM, Paul Eggleton wrote:
>> On Friday 31 May 2013 14:18:45 Xufeng Zhang wrote:
>>> The main changes include:
>>> 1). Add ntp:ntp(user:group) to system.
>>> 2). Running ntpd dameon as ntp:ntp.
>>> 3). Move relevant files from /usr/bin to /usr/sbin.
> Thanks for your review, Paul!
>
>> I'm not sure but I think the way this has been done 
>> (--with-binsubdir=sbin) is
>> going to bypass the ability to set sbindir in the distro config. If 
>> that's the
>> case then I don't think this is a good change, although I appreciate 
>> having
>> ntpd in /usr/sbin by default would be more appropriate.
> But currently it's the proper and easy way to do that, right?
> I can think out any better way to do the same job.
Sorry, a typo, s/can/can't/


Thanks,
Xufeng

>
>>> 4). Add crypto support.
>> The support is already there, it's just off by default and that was
>> intentional. Those that need this can easily enable it in their 
>> distro config.
> I'm not sure why we do that intentional, but shouldn't the package run
> as secure as possible by default?
>
>
> Thanks,
> Xufeng
>
>> Cheers,
>> Paul
>>
>
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>
Paul Eggleton - May 31, 2013, 11:14 a.m.
On Friday 31 May 2013 17:08:23 Xufeng Zhang wrote:
> On 05/31/2013 04:34 PM, Paul Eggleton wrote:
> > On Friday 31 May 2013 14:18:45 Xufeng Zhang wrote:
> >> The main changes include:
> >> 1). Add ntp:ntp(user:group) to system.
> >> 2). Running ntpd dameon as ntp:ntp.
> >> 
> >> 
> >> 3). Move relevant files from /usr/bin to /usr/sbin.
> 
> Thanks for your review, Paul!
> 
> > I'm not sure but I think the way this has been done
> > (--with-binsubdir=sbin) is going to bypass the ability to set sbindir in
> > the distro config. If that's the case then I don't think this is a good
> > change, although I appreciate having ntpd in /usr/sbin by default would
> > be more appropriate.
> 
> But currently it's the proper and easy way to do that, right?
> I can think out any better way to do the same job.

I'm not sure of the details of ntp's build system, but perhaps we could patch 
it to use sbindir instead of bindir for the appropriate binaries?
 
> >> 4). Add crypto support.
> > 
> > The support is already there, it's just off by default and that was
> > intentional. Those that need this can easily enable it in their distro
> > config.
>
> I'm not sure why we do that intentional, but shouldn't the package run
> as secure as possible by default?

The main concern is the dependency on OpenSSL that this introduces which many 
users do not want. I suspect it also depends on whether the NTP servers you 
are using support SSL.

Cheers,
Paul
Joe MacDonald - June 10, 2013, 3:29 p.m.
Hey Xufeng,

[[oe] [OE-devel] [PATCH] ntp: Resolve some abnormal behaviors] On 13.05.31 (Fri 14:18) Xufeng Zhang wrote:

> The main changes include:
> 1). Add ntp:ntp(user:group) to system.
> 2). Running ntpd dameon as ntp:ntp.
> 3). Move relevant files from /usr/bin to /usr/sbin.
> 4). Add crypto support.

This one seems to have trailed off.  Sorry if you guys were waiting on
my input as well.

First, I agree with Paul on both #3 and #4.  I would rather see a patch
that updates NTP to use sbindir instead of bindir in the options (or at
least a follow-up indicating that it's infeasible for some reason, I
also don't know what NTP's build system looks like, so maybe that's not
an option).  I also agree that my preferred scenario is for the system
to be as secure as possible by default, but crypto support is available
and not everyone wants or needs it.  We (relatively) recently when
through an extended discussion about ntp versus ntp-ssl and the current
situation seems to be the best compromise for everyone.

As a more general comment, you have four bullet-points below.  That's
normally an indication (to me, at least) that four patches are
appropriate.  Looking a bit closer, it looks like two related changes
and two unrelated ones, so I'd want to see three patches for this unless
there's a good reason why all of them are tied together.

#1 and #2 aren't likely to be contentious, so feel free to send out a
single patch doing both of those any time and we can revisit #3 and #4
at your convenience.

Thanks,
-J.

> 
> [YOCTO #4567]
> [ CQID: WIND00417282 ]
> 
> Signed-off-by: Xufeng Zhang <xufeng.zhang@windriver.com>
> ---
>  meta-networking/recipes-support/ntp/files/ntpd    |    8 ++++----
>  meta-networking/recipes-support/ntp/files/ntpdate |    6 +++---
>  meta-networking/recipes-support/ntp/ntp.inc       |   20 ++++++++++++--------
>  3 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/meta-networking/recipes-support/ntp/files/ntpd b/meta-networking/recipes-support/ntp/files/ntpd
> index ae50f13..285f5c0 100755
> --- a/meta-networking/recipes-support/ntp/files/ntpd
> +++ b/meta-networking/recipes-support/ntp/files/ntpd
> @@ -1,7 +1,7 @@
>  #! /bin/sh
>  #
>  # ntpd	init.d script for ntpdc from ntp.isc.org
> -test -x /usr/bin/ntpd -a -r /etc/ntp.conf || exit 0
> +test -x /usr/sbin/ntpd -a -r /etc/ntp.conf || exit 0
>  # rcS contains TICKADJ
>  test -r /etc/default/rcS && . /etc/default/rcS
>  
> @@ -9,9 +9,9 @@ test -r /etc/default/rcS && . /etc/default/rcS
>  settick(){
>    	# If TICKADJ is set we *must* adjust it before we start, because the
>  	# driftfile relies on the correct setting
> -	test -n "$TICKADJ" -a -x /usr/bin/tickadj && {
> +	test -n "$TICKADJ" -a -x /usr/sbin/tickadj && {
>  		echo -n "Setting tick to $TICKADJ: "
> -		/usr/bin/tickadj "$TICKADJ"
> +		/usr/sbin/tickadj "$TICKADJ"
>  		echo "done"
>  	}
>  }
> @@ -21,7 +21,7 @@ startdaemon(){
>  	# this.  If ntpd seems to disappear after a while assume TICKADJ
>  	# above is set to a totally incorrect value.
>  	echo -n "Starting ntpd: "
> -	start-stop-daemon --start -x /usr/bin/ntpd -- -p /var/run/ntp.pid "$@"
> +	start-stop-daemon --start -x /usr/sbin/ntpd -- -u ntp:ntp -p /var/run/ntp.pid "$@"
>  	echo "done"
>  }
>  stopdaemon(){
> diff --git a/meta-networking/recipes-support/ntp/files/ntpdate b/meta-networking/recipes-support/ntp/files/ntpdate
> index ab0551c..17b64d1 100755
> --- a/meta-networking/recipes-support/ntp/files/ntpdate
> +++ b/meta-networking/recipes-support/ntp/files/ntpdate
> @@ -1,8 +1,8 @@
>  #!/bin/sh
>  
> -PATH=/sbin:/bin:/usr/bin
> +PATH=/sbin:/bin:/usr/bin:/usr/sbin
>  
> -test -x /usr/bin/ntpdate || exit 0
> +test -x /usr/sbin/ntpdate || exit 0
>  
>  if test -f /etc/default/ntpdate ; then
>  . /etc/default/ntpdate
> @@ -40,7 +40,7 @@ if [ -x /usr/bin/lockfile-create ]; then
>  	LOCKTOUCHPID="$!"
>  fi
>  
> -if /usr/bin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
> +if /usr/sbin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
>  	if [ "$UPDATE_HWCLOCK" = "yes" ]; then
>  		hwclock --systohc || :
>  	fi
> diff --git a/meta-networking/recipes-support/ntp/ntp.inc b/meta-networking/recipes-support/ntp/ntp.inc
> index 79e7401..b52a7d6 100644
> --- a/meta-networking/recipes-support/ntp/ntp.inc
> +++ b/meta-networking/recipes-support/ntp/ntp.inc
> @@ -24,14 +24,19 @@ SRC_URI = "http://www.eecis.udel.edu/~ntp/ntp_spool/ntp4/ntp-4.2/ntp-${PV}.tar.g
>             file://sntp \
>  "
>  
> -inherit autotools update-rc.d systemd
> +inherit autotools update-rc.d systemd useradd
>  
>  # The ac_cv_header_readline_history is to stop ntpdc depending on either
>  # readline or curses
> -EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd ac_cv_header_readline_history_h=no"
> +EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd ac_cv_header_readline_history_h=no --with-binsubdir=sbin"
>  CFLAGS_append = " -DPTYS_ARE_GETPT -DPTYS_ARE_SEARCHED"
>  
> -PACKAGECONFIG ??= ""
> +USERADD_PACKAGES = "${PN}"
> +USERADD_PARAM_${PN} = "--system --home /etc/ntp \
> +		       --no-create-home --shell /bin/false \
> +		       --user-group ntp"
> +
> +PACKAGECONFIG ??= "openssl"
>  PACKAGECONFIG[openssl] = "--with-openssl-libdir=${STAGING_LIBDIR} \
>                            --with-openssl-incdir=${STAGING_INCDIR} \
>                            --with-crypto, \
> @@ -91,10 +96,10 @@ RCONFLICTS_ntpdate += "ntpdate-systemd"
>  
>  RSUGGESTS_${PN} = "iana-etc"
>  
> -FILES_${PN} = "${bindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${sbindir} ${libdir}"
> -FILES_${PN}-tickadj = "${bindir}/tickadj"
> -FILES_${PN}-utils = "${bindir}"
> -FILES_ntpdate = "${bindir}/ntpdate \
> +FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${libdir}"
> +FILES_${PN}-tickadj = "${sbindir}/tickadj"
> +FILES_${PN}-utils = "${sbindir}"
> +FILES_ntpdate = "${sbindir}/ntpdate \
>      ${sysconfdir}/network/if-up.d/ntpdate-sync \
>      ${bindir}/ntpdate-sync \
>      ${sysconfdir}/default/ntpdate \
> @@ -122,4 +127,3 @@ else
>      fi
>  fi
>  }
> -
Xufeng Zhang - Feb. 28, 2014, 7:41 a.m.
On 06/10/2013 11:29 PM, Joe MacDonald wrote:
> Hey Xufeng,
>
> [[oe] [OE-devel] [PATCH] ntp: Resolve some abnormal behaviors] On 13.05.31 (Fri 14:18) Xufeng Zhang wrote:
>
>    
>> The main changes include:
>> 1). Add ntp:ntp(user:group) to system.
>> 2). Running ntpd dameon as ntp:ntp.
>> 3). Move relevant files from /usr/bin to /usr/sbin.
>> 4). Add crypto support.
>>      
> This one seems to have trailed off.  Sorry if you guys were waiting on
> my input as well.
>    

Sorry for late response, I have missed this email.

> First, I agree with Paul on both #3 and #4.

Now I also agree that I should drop #4.

> I would rather see a patch
> that updates NTP to use sbindir instead of bindir in the options

I'm not quite understand what's the meaning of "in the options".

I'll explain how "--with-binsubdir" works for ntp:
"--with-binsubdir" controls whether we use bin_PROGRAMS or sbin_PROGRAMS
for built binaries in Makefile, in others words, it controls where we 
install the
binaries. If "--with-binsubdir" is not set or if "--with-binsubdir=bin", 
then we use
bindir, otherwise, if "--with-binsubdir=sbin", we use sbindir, so if we 
want to
install the binaries into sbindir, we must specify "--with-binsubdir=sbin".


> (or at
> least a follow-up indicating that it's infeasible for some reason, I
> also don't know what NTP's build system looks like, so maybe that's not
> an option).  I also agree that my preferred scenario is for the system
> to be as secure as possible by default, but crypto support is available
> and not everyone wants or needs it.  We (relatively) recently when
> through an extended discussion about ntp versus ntp-ssl and the current
> situation seems to be the best compromise for everyone.
>
> As a more general comment, you have four bullet-points below.  That's
> normally an indication (to me, at least) that four patches are
> appropriate.  Looking a bit closer, it looks like two related changes
> and two unrelated ones, so I'd want to see three patches for this unless
> there's a good reason why all of them are tied together.
>
> #1 and #2 aren't likely to be contentious, so feel free to send out a
> single patch doing both of those any time and we can revisit #3 and #4
> at your convenience.
>    

Thank you very much for the detail suggestions and explanations!
I'll send V2 patch until we come to a agreement on #3.


Xufeng

> Thanks,
> -J.
>
>    
>> [YOCTO #4567]
>> [ CQID: WIND00417282 ]
>>
>> Signed-off-by: Xufeng Zhang<xufeng.zhang@windriver.com>
>> ---
>>   meta-networking/recipes-support/ntp/files/ntpd    |    8 ++++----
>>   meta-networking/recipes-support/ntp/files/ntpdate |    6 +++---
>>   meta-networking/recipes-support/ntp/ntp.inc       |   20 ++++++++++++--------
>>   3 files changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/meta-networking/recipes-support/ntp/files/ntpd b/meta-networking/recipes-support/ntp/files/ntpd
>> index ae50f13..285f5c0 100755
>> --- a/meta-networking/recipes-support/ntp/files/ntpd
>> +++ b/meta-networking/recipes-support/ntp/files/ntpd
>> @@ -1,7 +1,7 @@
>>   #! /bin/sh
>>   #
>>   # ntpd	init.d script for ntpdc from ntp.isc.org
>> -test -x /usr/bin/ntpd -a -r /etc/ntp.conf || exit 0
>> +test -x /usr/sbin/ntpd -a -r /etc/ntp.conf || exit 0
>>   # rcS contains TICKADJ
>>   test -r /etc/default/rcS&&  . /etc/default/rcS
>>
>> @@ -9,9 +9,9 @@ test -r /etc/default/rcS&&  . /etc/default/rcS
>>   settick(){
>>     	# If TICKADJ is set we *must* adjust it before we start, because the
>>   	# driftfile relies on the correct setting
>> -	test -n "$TICKADJ" -a -x /usr/bin/tickadj&&  {
>> +	test -n "$TICKADJ" -a -x /usr/sbin/tickadj&&  {
>>   		echo -n "Setting tick to $TICKADJ: "
>> -		/usr/bin/tickadj "$TICKADJ"
>> +		/usr/sbin/tickadj "$TICKADJ"
>>   		echo "done"
>>   	}
>>   }
>> @@ -21,7 +21,7 @@ startdaemon(){
>>   	# this.  If ntpd seems to disappear after a while assume TICKADJ
>>   	# above is set to a totally incorrect value.
>>   	echo -n "Starting ntpd: "
>> -	start-stop-daemon --start -x /usr/bin/ntpd -- -p /var/run/ntp.pid "$@"
>> +	start-stop-daemon --start -x /usr/sbin/ntpd -- -u ntp:ntp -p /var/run/ntp.pid "$@"
>>   	echo "done"
>>   }
>>   stopdaemon(){
>> diff --git a/meta-networking/recipes-support/ntp/files/ntpdate b/meta-networking/recipes-support/ntp/files/ntpdate
>> index ab0551c..17b64d1 100755
>> --- a/meta-networking/recipes-support/ntp/files/ntpdate
>> +++ b/meta-networking/recipes-support/ntp/files/ntpdate
>> @@ -1,8 +1,8 @@
>>   #!/bin/sh
>>
>> -PATH=/sbin:/bin:/usr/bin
>> +PATH=/sbin:/bin:/usr/bin:/usr/sbin
>>
>> -test -x /usr/bin/ntpdate || exit 0
>> +test -x /usr/sbin/ntpdate || exit 0
>>
>>   if test -f /etc/default/ntpdate ; then
>>   . /etc/default/ntpdate
>> @@ -40,7 +40,7 @@ if [ -x /usr/bin/lockfile-create ]; then
>>   	LOCKTOUCHPID="$!"
>>   fi
>>
>> -if /usr/bin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
>> +if /usr/sbin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
>>   	if [ "$UPDATE_HWCLOCK" = "yes" ]; then
>>   		hwclock --systohc || :
>>   	fi
>> diff --git a/meta-networking/recipes-support/ntp/ntp.inc b/meta-networking/recipes-support/ntp/ntp.inc
>> index 79e7401..b52a7d6 100644
>> --- a/meta-networking/recipes-support/ntp/ntp.inc
>> +++ b/meta-networking/recipes-support/ntp/ntp.inc
>> @@ -24,14 +24,19 @@ SRC_URI = "http://www.eecis.udel.edu/~ntp/ntp_spool/ntp4/ntp-4.2/ntp-${PV}.tar.g
>>              file://sntp \
>>   "
>>
>> -inherit autotools update-rc.d systemd
>> +inherit autotools update-rc.d systemd useradd
>>
>>   # The ac_cv_header_readline_history is to stop ntpdc depending on either
>>   # readline or curses
>> -EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd ac_cv_header_readline_history_h=no"
>> +EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd ac_cv_header_readline_history_h=no --with-binsubdir=sbin"
>>   CFLAGS_append = " -DPTYS_ARE_GETPT -DPTYS_ARE_SEARCHED"
>>
>> -PACKAGECONFIG ??= ""
>> +USERADD_PACKAGES = "${PN}"
>> +USERADD_PARAM_${PN} = "--system --home /etc/ntp \
>> +		       --no-create-home --shell /bin/false \
>> +		       --user-group ntp"
>> +
>> +PACKAGECONFIG ??= "openssl"
>>   PACKAGECONFIG[openssl] = "--with-openssl-libdir=${STAGING_LIBDIR} \
>>                             --with-openssl-incdir=${STAGING_INCDIR} \
>>                             --with-crypto, \
>> @@ -91,10 +96,10 @@ RCONFLICTS_ntpdate += "ntpdate-systemd"
>>
>>   RSUGGESTS_${PN} = "iana-etc"
>>
>> -FILES_${PN} = "${bindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${sbindir} ${libdir}"
>> -FILES_${PN}-tickadj = "${bindir}/tickadj"
>> -FILES_${PN}-utils = "${bindir}"
>> -FILES_ntpdate = "${bindir}/ntpdate \
>> +FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${libdir}"
>> +FILES_${PN}-tickadj = "${sbindir}/tickadj"
>> +FILES_${PN}-utils = "${sbindir}"
>> +FILES_ntpdate = "${sbindir}/ntpdate \
>>       ${sysconfdir}/network/if-up.d/ntpdate-sync \
>>       ${bindir}/ntpdate-sync \
>>       ${sysconfdir}/default/ntpdate \
>> @@ -122,4 +127,3 @@ else
>>       fi
>>   fi
>>   }
>> -
>>
stephen.arnold42 - March 1, 2014, 4:33 a.m.
Sorry I missed the previous traffic, but does it make more sense to set the
ntp user's home to something like /var/lib/ntp instead?  For me it's always
been that way as the typical place for the drift file (written to by the
ntpd process running as ntp) as well as a *.keys file it needed.  IMHO the
above with a suitable line in the default ntp.conf would be a Good Thing.

Lastly, what about using pool.ntp.org as the default server for both ntpd
and ntpdate?  That way everything works out of the box (as long as there's
a net connection) and users can customize from there.  I have a bbappend
for ntp right now, so maybe I should look at the default config again...


On Thu, Feb 27, 2014 at 11:41 PM, Xufeng Zhang
<xufeng.zhang@windriver.com>wrote:

> On 06/10/2013 11:29 PM, Joe MacDonald wrote:
>
>> Hey Xufeng,
>>
>> [[oe] [OE-devel] [PATCH] ntp: Resolve some abnormal behaviors] On
>> 13.05.31 (Fri 14:18) Xufeng Zhang wrote:
>>
>>
>>
>>> The main changes include:
>>> 1). Add ntp:ntp(user:group) to system.
>>> 2). Running ntpd dameon as ntp:ntp.
>>> 3). Move relevant files from /usr/bin to /usr/sbin.
>>> 4). Add crypto support.
>>>
>>>
>> This one seems to have trailed off.  Sorry if you guys were waiting on
>> my input as well.
>>
>>
>
> Sorry for late response, I have missed this email.
>
>  First, I agree with Paul on both #3 and #4.
>>
>
> Now I also agree that I should drop #4.
>
>  I would rather see a patch
>> that updates NTP to use sbindir instead of bindir in the options
>>
>
> I'm not quite understand what's the meaning of "in the options".
>
> I'll explain how "--with-binsubdir" works for ntp:
> "--with-binsubdir" controls whether we use bin_PROGRAMS or sbin_PROGRAMS
> for built binaries in Makefile, in others words, it controls where we
> install the
> binaries. If "--with-binsubdir" is not set or if "--with-binsubdir=bin",
> then we use
> bindir, otherwise, if "--with-binsubdir=sbin", we use sbindir, so if we
> want to
> install the binaries into sbindir, we must specify "--with-binsubdir=sbin".
>
>
>  (or at
>> least a follow-up indicating that it's infeasible for some reason, I
>> also don't know what NTP's build system looks like, so maybe that's not
>> an option).  I also agree that my preferred scenario is for the system
>> to be as secure as possible by default, but crypto support is available
>> and not everyone wants or needs it.  We (relatively) recently when
>> through an extended discussion about ntp versus ntp-ssl and the current
>> situation seems to be the best compromise for everyone.
>>
>> As a more general comment, you have four bullet-points below.  That's
>> normally an indication (to me, at least) that four patches are
>> appropriate.  Looking a bit closer, it looks like two related changes
>> and two unrelated ones, so I'd want to see three patches for this unless
>> there's a good reason why all of them are tied together.
>>
>> #1 and #2 aren't likely to be contentious, so feel free to send out a
>> single patch doing both of those any time and we can revisit #3 and #4
>> at your convenience.
>>
>>
>
> Thank you very much for the detail suggestions and explanations!
> I'll send V2 patch until we come to a agreement on #3.
>
>
> Xufeng
>
>  Thanks,
>> -J.
>>
>>
>>
>>> [YOCTO #4567]
>>> [ CQID: WIND00417282 ]
>>>
>>> Signed-off-by: Xufeng Zhang<xufeng.zhang@windriver.com>
>>> ---
>>>   meta-networking/recipes-support/ntp/files/ntpd    |    8 ++++----
>>>   meta-networking/recipes-support/ntp/files/ntpdate |    6 +++---
>>>   meta-networking/recipes-support/ntp/ntp.inc       |   20
>>> ++++++++++++--------
>>>   3 files changed, 19 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/meta-networking/recipes-support/ntp/files/ntpd
>>> b/meta-networking/recipes-support/ntp/files/ntpd
>>> index ae50f13..285f5c0 100755
>>> --- a/meta-networking/recipes-support/ntp/files/ntpd
>>> +++ b/meta-networking/recipes-support/ntp/files/ntpd
>>> @@ -1,7 +1,7 @@
>>>   #! /bin/sh
>>>   #
>>>   # ntpd        init.d script for ntpdc from ntp.isc.org
>>> -test -x /usr/bin/ntpd -a -r /etc/ntp.conf || exit 0
>>> +test -x /usr/sbin/ntpd -a -r /etc/ntp.conf || exit 0
>>>   # rcS contains TICKADJ
>>>   test -r /etc/default/rcS&&  . /etc/default/rcS
>>>
>>> @@ -9,9 +9,9 @@ test -r /etc/default/rcS&&  . /etc/default/rcS
>>>   settick(){
>>>         # If TICKADJ is set we *must* adjust it before we start, because
>>> the
>>>         # driftfile relies on the correct setting
>>> -       test -n "$TICKADJ" -a -x /usr/bin/tickadj&&  {
>>> +       test -n "$TICKADJ" -a -x /usr/sbin/tickadj&&  {
>>>                 echo -n "Setting tick to $TICKADJ: "
>>> -               /usr/bin/tickadj "$TICKADJ"
>>> +               /usr/sbin/tickadj "$TICKADJ"
>>>                 echo "done"
>>>         }
>>>   }
>>> @@ -21,7 +21,7 @@ startdaemon(){
>>>         # this.  If ntpd seems to disappear after a while assume TICKADJ
>>>         # above is set to a totally incorrect value.
>>>         echo -n "Starting ntpd: "
>>> -       start-stop-daemon --start -x /usr/bin/ntpd -- -p
>>> /var/run/ntp.pid "$@"
>>> +       start-stop-daemon --start -x /usr/sbin/ntpd -- -u ntp:ntp -p
>>> /var/run/ntp.pid "$@"
>>>         echo "done"
>>>   }
>>>   stopdaemon(){
>>> diff --git a/meta-networking/recipes-support/ntp/files/ntpdate
>>> b/meta-networking/recipes-support/ntp/files/ntpdate
>>> index ab0551c..17b64d1 100755
>>> --- a/meta-networking/recipes-support/ntp/files/ntpdate
>>> +++ b/meta-networking/recipes-support/ntp/files/ntpdate
>>> @@ -1,8 +1,8 @@
>>>   #!/bin/sh
>>>
>>> -PATH=/sbin:/bin:/usr/bin
>>> +PATH=/sbin:/bin:/usr/bin:/usr/sbin
>>>
>>> -test -x /usr/bin/ntpdate || exit 0
>>> +test -x /usr/sbin/ntpdate || exit 0
>>>
>>>   if test -f /etc/default/ntpdate ; then
>>>   . /etc/default/ntpdate
>>> @@ -40,7 +40,7 @@ if [ -x /usr/bin/lockfile-create ]; then
>>>         LOCKTOUCHPID="$!"
>>>   fi
>>>
>>> -if /usr/bin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
>>> +if /usr/sbin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
>>>         if [ "$UPDATE_HWCLOCK" = "yes" ]; then
>>>                 hwclock --systohc || :
>>>         fi
>>> diff --git a/meta-networking/recipes-support/ntp/ntp.inc
>>> b/meta-networking/recipes-support/ntp/ntp.inc
>>> index 79e7401..b52a7d6 100644
>>> --- a/meta-networking/recipes-support/ntp/ntp.inc
>>> +++ b/meta-networking/recipes-support/ntp/ntp.inc
>>> @@ -24,14 +24,19 @@ SRC_URI = "http://www.eecis.udel.edu/~
>>> ntp/ntp_spool/ntp4/ntp-4.2/ntp-${PV}.tar.g
>>>              file://sntp \
>>>   "
>>>
>>> -inherit autotools update-rc.d systemd
>>> +inherit autotools update-rc.d systemd useradd
>>>
>>>   # The ac_cv_header_readline_history is to stop ntpdc depending on
>>> either
>>>   # readline or curses
>>> -EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd
>>> ac_cv_header_readline_history_h=no"
>>> +EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd
>>> ac_cv_header_readline_history_h=no --with-binsubdir=sbin"
>>>   CFLAGS_append = " -DPTYS_ARE_GETPT -DPTYS_ARE_SEARCHED"
>>>
>>> -PACKAGECONFIG ??= ""
>>> +USERADD_PACKAGES = "${PN}"
>>> +USERADD_PARAM_${PN} = "--system --home /etc/ntp \
>>> +                      --no-create-home --shell /bin/false \
>>> +                      --user-group ntp"
>>> +
>>> +PACKAGECONFIG ??= "openssl"
>>>   PACKAGECONFIG[openssl] = "--with-openssl-libdir=${STAGING_LIBDIR} \
>>>                             --with-openssl-incdir=${STAGING_INCDIR} \
>>>                             --with-crypto, \
>>> @@ -91,10 +96,10 @@ RCONFLICTS_ntpdate += "ntpdate-systemd"
>>>
>>>   RSUGGESTS_${PN} = "iana-etc"
>>>
>>> -FILES_${PN} = "${bindir}/ntpd ${sysconfdir}/ntp.conf
>>> ${sysconfdir}/init.d/ntpd ${sbindir} ${libdir}"
>>> -FILES_${PN}-tickadj = "${bindir}/tickadj"
>>> -FILES_${PN}-utils = "${bindir}"
>>> -FILES_ntpdate = "${bindir}/ntpdate \
>>> +FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf
>>> ${sysconfdir}/init.d/ntpd ${libdir}"
>>> +FILES_${PN}-tickadj = "${sbindir}/tickadj"
>>> +FILES_${PN}-utils = "${sbindir}"
>>> +FILES_ntpdate = "${sbindir}/ntpdate \
>>>       ${sysconfdir}/network/if-up.d/ntpdate-sync \
>>>       ${bindir}/ntpdate-sync \
>>>       ${sysconfdir}/default/ntpdate \
>>> @@ -122,4 +127,3 @@ else
>>>       fi
>>>   fi
>>>   }
>>> -
>>>
>>>
>>
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
>
Paul Eggleton - March 1, 2014, 9:59 a.m.
Hi Stephen,

On Friday 28 February 2014 20:33:55 Stephen Arnold wrote:
> Lastly, what about using pool.ntp.org as the default server for both ntpd
> and ntpdate?  That way everything works out of the box (as long as there's
> a net connection) and users can customize from there.  I have a bbappend
> for ntp right now, so maybe I should look at the default config again...

That would be nice, but since you can't actually distribute a product that 
connects to the main pool.ntp.org server (you need to register your own NTP 
pool [1]) we've elected not to set any default so that you are encouraged to 
do the right thing and explicitly set your own.

Cheers,
Paul

[1] http://www.pool.ntp.org/en/vendors.html#basic-guidelines
Ross Burton - March 1, 2014, 8:18 p.m.
On 1 March 2014 09:59, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
> That would be nice, but since you can't actually distribute a product that
> connects to the main pool.ntp.org server (you need to register your own NTP
> pool [1]) we've elected not to set any default so that you are encouraged to
> do the right thing and explicitly set your own.

We should probably put this in the documentation.  I'll file a bug.

Ross
Paul Eggleton - March 1, 2014, 10:48 p.m.
On Saturday 01 March 2014 20:18:44 Burton, Ross wrote:
> On 1 March 2014 09:59, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
> > That would be nice, but since you can't actually distribute a product that
> > connects to the main pool.ntp.org server (you need to register your own
> > NTP
> > pool [1]) we've elected not to set any default so that you are encouraged
> > to do the right thing and explicitly set your own.
> 
> We should probably put this in the documentation.  I'll file a bug.

Whose documentation though? ntp is part of meta-networking rather than OE-
Core.

Cheers,
Paul
Khem Raj - March 1, 2014, 10:57 p.m.
On Mar 1, 2014, at 2:48 PM, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:

> On Saturday 01 March 2014 20:18:44 Burton, Ross wrote:
>> On 1 March 2014 09:59, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
>>> That would be nice, but since you can't actually distribute a product that
>>> connects to the main pool.ntp.org server (you need to register your own
>>> NTP
>>> pool [1]) we've elected not to set any default so that you are encouraged
>>> to do the right thing and explicitly set your own.
>> 
>> We should probably put this in the documentation.  I'll file a bug.
> 
> Whose documentation though? ntp is part of meta-networking rather than OE-
> Core.
> 

README for meta-networking is best place. 

> Cheers,
> Paul
> 
> -- 
> 
> Paul Eggleton
> Intel Open Source Technology Centre
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel
Joe MacDonald - March 14, 2014, 12:37 p.m.
[Re: [oe] [OE-devel] [PATCH] ntp: Resolve some abnormal behaviors] On 14.03.01 (Sat 14:57) Khem Raj wrote:

> 
> On Mar 1, 2014, at 2:48 PM, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
> 
> > On Saturday 01 March 2014 20:18:44 Burton, Ross wrote:
> >> On 1 March 2014 09:59, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
> >>> That would be nice, but since you can't actually distribute a product that
> >>> connects to the main pool.ntp.org server (you need to register your own
> >>> NTP
> >>> pool [1]) we've elected not to set any default so that you are encouraged
> >>> to do the right thing and explicitly set your own.
> >> 
> >> We should probably put this in the documentation.  I'll file a bug.
> > 
> > Whose documentation though? ntp is part of meta-networking rather than OE-
> > Core.
> > 
> 
> README for meta-networking is best place. 

I agree.  I'm working through my backlog of merges right now but I've
not seen a readme patch.  If I missed it, please let me know, otherwise
when I get a chance I'll propose a readme documenting this myself.

-J.

> 
> > Cheers,
> > Paul
> > 
> > -- 
> > 
> > Paul Eggleton
> > Intel Open Source Technology Centre
> > _______________________________________________
> > Openembedded-devel mailing list
> > Openembedded-devel@lists.openembedded.org
> > http://lists.openembedded.org/mailman/listinfo/openembedded-devel
> 



> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-devel

Patch

diff --git a/meta-networking/recipes-support/ntp/files/ntpd b/meta-networking/recipes-support/ntp/files/ntpd
index ae50f13..285f5c0 100755
--- a/meta-networking/recipes-support/ntp/files/ntpd
+++ b/meta-networking/recipes-support/ntp/files/ntpd
@@ -1,7 +1,7 @@ 
 #! /bin/sh
 #
 # ntpd	init.d script for ntpdc from ntp.isc.org
-test -x /usr/bin/ntpd -a -r /etc/ntp.conf || exit 0
+test -x /usr/sbin/ntpd -a -r /etc/ntp.conf || exit 0
 # rcS contains TICKADJ
 test -r /etc/default/rcS && . /etc/default/rcS
 
@@ -9,9 +9,9 @@  test -r /etc/default/rcS && . /etc/default/rcS
 settick(){
   	# If TICKADJ is set we *must* adjust it before we start, because the
 	# driftfile relies on the correct setting
-	test -n "$TICKADJ" -a -x /usr/bin/tickadj && {
+	test -n "$TICKADJ" -a -x /usr/sbin/tickadj && {
 		echo -n "Setting tick to $TICKADJ: "
-		/usr/bin/tickadj "$TICKADJ"
+		/usr/sbin/tickadj "$TICKADJ"
 		echo "done"
 	}
 }
@@ -21,7 +21,7 @@  startdaemon(){
 	# this.  If ntpd seems to disappear after a while assume TICKADJ
 	# above is set to a totally incorrect value.
 	echo -n "Starting ntpd: "
-	start-stop-daemon --start -x /usr/bin/ntpd -- -p /var/run/ntp.pid "$@"
+	start-stop-daemon --start -x /usr/sbin/ntpd -- -u ntp:ntp -p /var/run/ntp.pid "$@"
 	echo "done"
 }
 stopdaemon(){
diff --git a/meta-networking/recipes-support/ntp/files/ntpdate b/meta-networking/recipes-support/ntp/files/ntpdate
index ab0551c..17b64d1 100755
--- a/meta-networking/recipes-support/ntp/files/ntpdate
+++ b/meta-networking/recipes-support/ntp/files/ntpdate
@@ -1,8 +1,8 @@ 
 #!/bin/sh
 
-PATH=/sbin:/bin:/usr/bin
+PATH=/sbin:/bin:/usr/bin:/usr/sbin
 
-test -x /usr/bin/ntpdate || exit 0
+test -x /usr/sbin/ntpdate || exit 0
 
 if test -f /etc/default/ntpdate ; then
 . /etc/default/ntpdate
@@ -40,7 +40,7 @@  if [ -x /usr/bin/lockfile-create ]; then
 	LOCKTOUCHPID="$!"
 fi
 
-if /usr/bin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
+if /usr/sbin/ntpdate -s $OPTS $NTPSERVERS 2>/dev/null; then
 	if [ "$UPDATE_HWCLOCK" = "yes" ]; then
 		hwclock --systohc || :
 	fi
diff --git a/meta-networking/recipes-support/ntp/ntp.inc b/meta-networking/recipes-support/ntp/ntp.inc
index 79e7401..b52a7d6 100644
--- a/meta-networking/recipes-support/ntp/ntp.inc
+++ b/meta-networking/recipes-support/ntp/ntp.inc
@@ -24,14 +24,19 @@  SRC_URI = "http://www.eecis.udel.edu/~ntp/ntp_spool/ntp4/ntp-4.2/ntp-${PV}.tar.g
            file://sntp \
 "
 
-inherit autotools update-rc.d systemd
+inherit autotools update-rc.d systemd useradd
 
 # The ac_cv_header_readline_history is to stop ntpdc depending on either
 # readline or curses
-EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd ac_cv_header_readline_history_h=no"
+EXTRA_OECONF += "--with-net-snmp-config=no --without-ntpsnmpd ac_cv_header_readline_history_h=no --with-binsubdir=sbin"
 CFLAGS_append = " -DPTYS_ARE_GETPT -DPTYS_ARE_SEARCHED"
 
-PACKAGECONFIG ??= ""
+USERADD_PACKAGES = "${PN}"
+USERADD_PARAM_${PN} = "--system --home /etc/ntp \
+		       --no-create-home --shell /bin/false \
+		       --user-group ntp"
+
+PACKAGECONFIG ??= "openssl"
 PACKAGECONFIG[openssl] = "--with-openssl-libdir=${STAGING_LIBDIR} \
                           --with-openssl-incdir=${STAGING_INCDIR} \
                           --with-crypto, \
@@ -91,10 +96,10 @@  RCONFLICTS_ntpdate += "ntpdate-systemd"
 
 RSUGGESTS_${PN} = "iana-etc"
 
-FILES_${PN} = "${bindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${sbindir} ${libdir}"
-FILES_${PN}-tickadj = "${bindir}/tickadj"
-FILES_${PN}-utils = "${bindir}"
-FILES_ntpdate = "${bindir}/ntpdate \
+FILES_${PN} = "${sbindir}/ntpd ${sysconfdir}/ntp.conf ${sysconfdir}/init.d/ntpd ${libdir}"
+FILES_${PN}-tickadj = "${sbindir}/tickadj"
+FILES_${PN}-utils = "${sbindir}"
+FILES_ntpdate = "${sbindir}/ntpdate \
     ${sysconfdir}/network/if-up.d/ntpdate-sync \
     ${bindir}/ntpdate-sync \
     ${sysconfdir}/default/ntpdate \
@@ -122,4 +127,3 @@  else
     fi
 fi
 }
-