bug#35996: User account password got locked when booting old generation

[pelzflorian (Florian Pelz)]

Please add more logging and/or locking.  Note that the elogind has the
following comment in its locking implementation in
/gnu/store/dm2ri0qvjirl0iq2ndfk5z9lq9dyk4jf-elogind-241.3-checkout/src/basic/user-util.c:

/* This is roughly the same as lckpwdf(), but not as awful. We
 * don't want to use alarm() and signals, hence we implement
 * our own trivial version of this.
 *
 * Note that shadow-utils also takes per-database locks in
 * addition to lckpwdf(). However, we don't given that they
 * are redundant as they invoke lckpwdf() first and keep
 * it during everything they do. The per-database locks are
 * awfully racy, and thus we just won't do them. */

Regards,
Florian

bug#35785: ‘string->uri’ is locale-dependent and breaks in ‘sv_SE’

[Ludovic Courtès]

Hi Timothy,

Timothy Sample  skribis:

> Here’s a patch for Guile that uses explicit lists of characters in the
> ‘(web uri)’ module instead of character ranges.  It includes two tests
> that are pretty verbose, but seem to do the trick.
>
> I have a bit more background on the problem, mostly coming from a Glibc
> bug report: .
>
> It turns out that it is well-known upstream, and avoiding character
> ranges is the recommended approach for know.  Some other GNU tools have
> adopted what is being called the “Rational Range Interpretation”
> .
> AIUI, this means they use the underlying encoding numbers for ranges (I
> checked the source, but I’m only mostly sure I read it right).  It looks
> like the Glibc folks are unsure how to proceed on this (but are maybe
> slightly leaning towards the “rational” approach).

Great that you gleaned good references on this topic!

> It’s all a pretty big mess, really.  I was hoping there would be some
> obvious thing that would fix the problem more generally.  Short of
> pulling in the Gnulib regex code or writing something in Scheme, it
> looks like Guile is stuck where it is now.

Yeah.  The alternative would be to not use regexps in this context, I
guess.

> I’m unsure if the changes are considered “trivial” from a copyright
> perspective.  It’s pretty close, but I think programmers tend to
> underestimate here.  I’ve started the FSF copyright assignment process
> either way, since is likely not my last Guile patch.  :)

If the process is already underway, I think it’s fine to commit this
patch (I would rather wait if it were longer and/or if we didn’t know
each other already).

> From 7b02be4c050c7b17a0e2685e8e453295f798c360 Mon Sep 17 00:00:00 2001
> From: Timothy Sample 
> Date: Sun, 2 Jun 2019 14:41:20 -0400
> Subject: [PATCH] Make URI handling locale independent.
>
> Fixes .
>
> * module/web/uri.scm (digits, hex-digits, letters): New variables.
> (ipv4-regexp, ipv6-regexp, domain-label-regexp, top-label-regexp,
> userinfo-pat, host-pat, ipv6-host-pat, port-pat, scheme-pat): Explicitly
> list each character instead of using character ranges.
> * test-suite/tests/web-uri.test: Add corresponding tests.

[...]

> +  (pass-if "http://www.example.com (sv_SE)"
> +(dynamic-wind
> +  (lambda () #t)
> +  (lambda ()
> +(with-locale "sv_SE.utf8"
> +  (reload-module (resolve-module '(web uri)))
> +  (uri=? (string->uri "http://www.example.com";)
> + #:scheme 'http #:host "www.example.com" #:path "")))

Aren’t ‘reload-module’ calls a leftover that can now be removed (also in
the other test)?

For the sv_SE test, what about taking a host name with a ‘w’, since
that’s the use case that allowed us to uncover this bug?

Apart from that it LGTM, thank you!

Ludo’.

bug#35996: User account password got locked when booting old generation

[Ludovic Courtès]

Hi Florian,

"pelzflorian (Florian Pelz)"  skribis:

> After I booted to a Guix install USB, chrooted as described on the
> Arch wiki and started a Guix daemon, I could reconfigure as before.
> There was no need to fiddle with grub-install.
>
> After multiple reconfigures, it happened again, my /etc/shadow has !
> again in the password field.  My recently changed root password became
> empty as well, like 35902.  I did not even run sudo concurrently.  The
> password just got locked.

What were the differences between your config files when you
reconfigured?

Did the config files describe the exact same set of user accounts?

Did the user accounts in the config files differ in any way?

Were the user accounts altered in any way in between reconfigures
(‘passwd’, ‘usermod’, GNOME, etc.)?


Looking at ‘user+group-databases’ in (gnu build accounts), which takes a
list of  and a list of  to produce
/etc/{passwd,shadow,group}, the only way this could happen is if
/etc/shadow does not exist at the time of reconfigure.

In that case, ‘user+group-databases’ assumes we’re starting anew, so it
creates /etc/shadow with the initial passwords specified in the OS
config.  At that point, the other passwords are lost forever.

Does Shadow or gnome-control-center or something remove /etc/shadow
altogether while it’s accessing it?

At the very least, adding locking like you suggested should avoid this
class of problems; I’ll look into that.

> From 1eb7699d5036062993a080393bfb4a46d2dc1bea Mon Sep 17 00:00:00 2001
> From: Florian Pelz 
> Date: Mon, 3 Jun 2019 07:19:20 +0200
> Subject: [PATCH 1/2] =?UTF-8?q?Add=20cracklib=E2=80=99s=20password=20dicti?=
>  =?UTF-8?q?onary=20to=20cracklib=E2=80=99s=20default=20output.?=
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> * gnu/packages/password-utils.scm (cracklib): Use `make dict`.

[...]

> From c7c016adc34c591febd0d3630f32dbecdd20ad7c Mon Sep 17 00:00:00 2001
> From: Florian Pelz 
> Date: Sun, 2 Jun 2019 20:01:23 +0200
> Subject: [PATCH 2/2] Make gnome-control-center find passwd binary.
>
> * gnu/packages/gnome.scm (gnome-control-center): Substitute correct path to
>   passwd.

Great, applied both.

Thank you!

Ludo’.

bug#35785: ‘string->uri’ is locale-dependent and breaks in ‘sv_SE’

[Timothy Sample]

Hi Ludo,

Ludovic Courtès  writes:

> Hi Timothy,
>
> Timothy Sample  skribis:
>
>> Here’s a patch for Guile that uses explicit lists of characters in the
>> ‘(web uri)’ module instead of character ranges.  It includes two tests
>> that are pretty verbose, but seem to do the trick.
>>
>> I have a bit more background on the problem, mostly coming from a Glibc
>> bug report: .
>>
>> It turns out that it is well-known upstream, and avoiding character
>> ranges is the recommended approach for know.  Some other GNU tools have
>> adopted what is being called the “Rational Range Interpretation”
>> .
>> AIUI, this means they use the underlying encoding numbers for ranges (I
>> checked the source, but I’m only mostly sure I read it right).  It looks
>> like the Glibc folks are unsure how to proceed on this (but are maybe
>> slightly leaning towards the “rational” approach).
>
> Great that you gleaned good references on this topic!
>
>> It’s all a pretty big mess, really.  I was hoping there would be some
>> obvious thing that would fix the problem more generally.  Short of
>> pulling in the Gnulib regex code or writing something in Scheme, it
>> looks like Guile is stuck where it is now.
>
> Yeah.  The alternative would be to not use regexps in this context, I
> guess.

I meant fixing regexes in other contexts, since I’m sure the URI module
is not the only Guile code ever that assumed “[a-z]” would only match
ASCII lowercase letters.

>> I’m unsure if the changes are considered “trivial” from a copyright
>> perspective.  It’s pretty close, but I think programmers tend to
>> underestimate here.  I’ve started the FSF copyright assignment process
>> either way, since is likely not my last Guile patch.  :)
>
> If the process is already underway, I think it’s fine to commit this
> patch (I would rather wait if it were longer and/or if we didn’t know
> each other already).

Sounds good!

>> From 7b02be4c050c7b17a0e2685e8e453295f798c360 Mon Sep 17 00:00:00 2001
>> From: Timothy Sample 
>> Date: Sun, 2 Jun 2019 14:41:20 -0400
>> Subject: [PATCH] Make URI handling locale independent.
>>
>> Fixes .
>>
>> * module/web/uri.scm (digits, hex-digits, letters): New variables.
>> (ipv4-regexp, ipv6-regexp, domain-label-regexp, top-label-regexp,
>> userinfo-pat, host-pat, ipv6-host-pat, port-pat, scheme-pat): Explicitly
>> list each character instead of using character ranges.
>> * test-suite/tests/web-uri.test: Add corresponding tests.
>
> [...]
>
>> +  (pass-if "http://www.example.com (sv_SE)"
>> +(dynamic-wind
>> +  (lambda () #t)
>> +  (lambda ()
>> +(with-locale "sv_SE.utf8"
>> +  (reload-module (resolve-module '(web uri)))
>> +  (uri=? (string->uri "http://www.example.com";)
>> + #:scheme 'http #:host "www.example.com" #:path "")))
>
> Aren’t ‘reload-module’ calls a leftover that can now be removed (also in
> the other test)?

I needed to reload the modules like that to make the tests fail without
the patch and pass with it.  My understanding is that the bug happens
at regex compile time, which happens when the module is loaded.  If I
don’t reload the module, the old URI code passes the tests, since the
regexes were compiled with a locale that does not trigger the bug.  It’s
a little wacky, sure, but it was the best idea I could come up with.

> For the sv_SE test, what about taking a host name with a ‘w’, since
> that’s the use case that allowed us to uncover this bug?

I thought I was being clever by using a “www” hostname, but apparently
it’s so normalized as to be invisible!  Feel free to change it to
something more obvious like “w.com” or whatever.


-- Tim

bug#35996: User account password got locked when booting old generation

[pelzflorian (Florian Pelz)]

On Mon, Jun 03, 2019 at 03:22:51PM +0200, Ludovic Courtès wrote:
> > After multiple reconfigures, it happened again, my /etc/shadow has !
> > again in the password field.  My recently changed root password became
> > empty as well, like 35902.  I did not even run sudo concurrently.  The
> > password just got locked.
> 
> What were the differences between your config files when you
> reconfigured?
>

For the last reconfigure, there were no differences, although I had
rebooted into an unbootable, older generation with a different
syslog.conf and broken Udevd arguments before booting the new
generation.  I suppose the other victims of this bug have not booted
to unbootable generations?


> Did the config files describe the exact same set of user accounts?
>

Yes, they’re the same.

> Did the user accounts in the config files differ in any way?
>

No, they do not differ.

> Were the user accounts altered in any way in between reconfigures
> (‘passwd’, ‘usermod’, GNOME, etc.)?
> 
>
> Looking at ‘user+group-databases’ in (gnu build accounts), which takes a
> list of  and a list of  to produce
> /etc/{passwd,shadow,group}, the only way this could happen is if
> /etc/shadow does not exist at the time of reconfigure.
>
> In that case, ‘user+group-databases’ assumes we’re starting anew, so it
> creates /etc/shadow with the initial passwords specified in the OS
> config.  At that point, the other passwords are lost forever.
> 
> Does Shadow or gnome-control-center or something remove /etc/shadow
> altogether while it’s accessing it?
>

I did not use gnome-control-center or shadow or sudo during the last
reconfigure, except sudo for starting the reconfigure.

> At the very least, adding locking like you suggested should avoid this
> class of problems; I’ll look into that.
> 

I do not know if something somehow accessed /etc/shadow in the
background without my knowledge.  I believe locks are important anyway
to have more guarantees passwords are not lost when Guix is run on a
sensitive multi-user setup.  Thank you for looking into it.

If locks do not stop these issues, it would be nice to have more
detailed logs of shadow changes written to syslog on reconfigure
and/or on reboot.

Regards,
Florian

bug#35996: User account password got locked when booting old generation

[Ludovic Courtès]

Hello,

"pelzflorian (Florian Pelz)"  skribis:

> Please add more logging and/or locking.  Note that the elogind has the
> following comment in its locking implementation in
> /gnu/store/dm2ri0qvjirl0iq2ndfk5z9lq9dyk4jf-elogind-241.3-checkout/src/basic/user-util.c:
>
> /* This is roughly the same as lckpwdf(), but not as awful. We
>  * don't want to use alarm() and signals, hence we implement
>  * our own trivial version of this.

Attach is a set of patches to lock /etc/.pwd.lock when we access
/etc/{passwd,shadow,group} from the activation snippets.  It should
ensure that, when running ‘guix system reconfigure’, we’re honoring the
locking protocol that Shadow & co. follow.

It would be great if you could test again in this context.

Thanks!

Ludo’.

>From e26d9759ebe58c93e7eb449ea6bd3317b2840e99 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= 
Date: Mon, 3 Jun 2019 16:23:01 +0200
Subject: [PATCH 1/5] syscalls: Add 'with-file-lock' macro.

* guix/scripts/offload.scm (lock-file, unlock-file, with-file-lock):
Move to...
* guix/build/syscalls.scm: ... here.
---
 .dir-locals.el   |  2 ++
 guix/build/syscalls.scm  | 27 +++
 guix/scripts/offload.scm | 25 -
 3 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index f1196fd781..228685a69f 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -34,6 +34,8 @@
 
(eval . (put 'modify-services 'scheme-indent-function 1))
(eval . (put 'with-directory-excursion 'scheme-indent-function 1))
+   (eval . (put 'with-file-lock 'scheme-indent-function 1))
+
(eval . (put 'package 'scheme-indent-function 0))
(eval . (put 'origin 'scheme-indent-function 0))
(eval . (put 'build-system 'scheme-indent-function 0))
diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index 3abe65bc4f..04fbebb8a2 100644
--- a/guix/build/syscalls.scm
+++ b/guix/build/syscalls.scm
@@ -81,7 +81,11 @@
 fdatasync
 pivot-root
 scandir*
+
 fcntl-flock
+lock-file
+unlock-file
+with-file-lock
 
 set-thread-name
 thread-name
@@ -1067,6 +1071,29 @@ exception if it's already taken."
   ;; Presumably we got EAGAIN or so.
   (throw 'flock-error err))
 
+(define (lock-file file)
+  "Wait and acquire an exclusive lock on FILE.  Return an open port."
+  (let ((port (open-file file "w0")))
+(fcntl-flock port 'write-lock)
+port))
+
+(define (unlock-file port)
+  "Unlock PORT, a port returned by 'lock-file'."
+  (fcntl-flock port 'unlock)
+  (close-port port)
+  #t)
+
+(define-syntax-rule (with-file-lock file exp ...)
+  "Wait to acquire a lock on FILE and evaluate EXP in that context."
+  (let ((port (lock-file file)))
+(dynamic-wind
+  (lambda ()
+#t)
+  (lambda ()
+exp ...)
+  (lambda ()
+(unlock-file port)
+
 
 ;;;
 ;;; Miscellaneous, aka. 'prctl'.
diff --git a/guix/scripts/offload.scm b/guix/scripts/offload.scm
index eb02672dbf..0c0dd9d516 100644
--- a/guix/scripts/offload.scm
+++ b/guix/scripts/offload.scm
@@ -236,30 +236,6 @@ instead of '~a' of type '~a'~%")
 ;;; Synchronization.
 ;;;
 
-(define (lock-file file)
-  "Wait and acquire an exclusive lock on FILE.  Return an open port."
-  (mkdir-p (dirname file))
-  (let ((port (open-file file "w0")))
-(fcntl-flock port 'write-lock)
-port))
-
-(define (unlock-file lock)
-  "Unlock LOCK."
-  (fcntl-flock lock 'unlock)
-  (close-port lock)
-  #t)
-
-(define-syntax-rule (with-file-lock file exp ...)
-  "Wait to acquire a lock on FILE and evaluate EXP in that context."
-  (let ((port (lock-file file)))
-(dynamic-wind
-  (lambda ()
-#t)
-  (lambda ()
-exp ...)
-  (lambda ()
-(unlock-file port)
-
 (define (machine-slot-file machine slot)
   "Return the file name of MACHINE's file for SLOT."
   ;; For each machine we have a bunch of files representing each build slot.
@@ -829,7 +805,6 @@ This tool is meant to be used internally by 'guix-daemon'.\n"))
  (leave (G_ "invalid arguments: ~{~s ~}~%") x
 
 ;;; Local Variables:
-;;; eval: (put 'with-file-lock 'scheme-indent-function 1)
 ;;; eval: (put 'with-error-to-port 'scheme-indent-function 1)
 ;;; eval: (put 'with-timeout 'scheme-indent-function 2)
 ;;; End:
-- 
2.21.0

>From cf802dcd04425ce783714d70444b023902b36947 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= 
Date: Mon, 3 Jun 2019 16:24:31 +0200
Subject: [PATCH 2/5] syscalls: 'with-file-lock' expands to a call to
 'call-with-file-lock'.

* guix/build/syscalls.scm (call-with-file-lock): New procedure.
(with-file-lock): Expand to a call to 'call-with-file-lock'.
---
 guix/build/syscalls.scm | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index 04fbebb8a2..3af41f2cf5 100644
--

bug#35996: User account password got locked when booting old generation

[Danny Milosavljevic]

For debugging, I've used "chattr +i" in the past in order to make such files
(i.e. /etc/shadow) immutable.  This would hopefully expose the mutator (other
than Guix)--it should log some error somewhere.



pgpKrPhEw9hmR.pgp
Description: OpenPGP digital signature

bug#35996: User account password got locked when booting old generation

[pelzflorian (Florian Pelz)]

All this is not a problem with name service cache daemon, is it?

On Mon, Jun 03, 2019 at 05:22:55PM +0200, Ludovic Courtès wrote:
> Attach is a set of patches to lock /etc/.pwd.lock when we access
> /etc/{passwd,shadow,group} from the activation snippets.  It should
> ensure that, when running ‘guix system reconfigure’, we’re honoring the
> locking protocol that Shadow & co. follow.
> 
> It would be great if you could test again in this context.
> 

Thank you!  Will do.

Regards,
Florian

bug#36074: Incorrect quoting of GUIX_LOCPATH environment variable in guix-daemon.service

[Jack Hill]

I wrote more about this on help-guix@

https://lists.gnu.org/archive/html/help-guix/2019-06/msg00024.html

bug#36074: Incorrect quoting of GUIX_LOCPATH environment variable in guix-daemon.service

[Jack Hill]

Hi Guix,

Based on my experience setting up Guix on a CentOS 7 foreign distribution, 
if quoted, the variable name in a systemd unit Environment definition 
should be inside the quotes. I didn't not find definitive documentation 
stating this, but all the examples show it this way. If the value is left 
like


```
Environment=GUIX_LOCPATH='/path/to/profile/lib/locale'
```

the guix-daemon process is not able to find the right locale even when it 
is installed.


Best,
Jack

bug#36074: [PATCH] etc: guix-daemon.service.in: fix GUIX_LOCPATH quoting

[Jack Hill]

etc/guix-daemon.service.in: Move the GUIX_LOCPATH environment varialbe name
inside the quotes are required in systemd unit files.
---
 etc/guix-daemon.service.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/etc/guix-daemon.service.in b/etc/guix-daemon.service.in
index 7b20a91931..407cdd199c 100644
--- a/etc/guix-daemon.service.in
+++ b/etc/guix-daemon.service.in
@@ -7,7 +7,7 @@ Description=Build daemon for GNU Guix

 [Service]
 
ExecStart=@localstatedir@/guix/profiles/per-user/root/current-guix/bin/guix-daemon
 --build-users-group=guixbuild
-Environment=GUIX_LOCPATH='@localstatedir@/guix/profiles/per-user/root/guix-profile/lib/locale'
 LC_ALL=en_US.utf8
+Environment='GUIX_LOCPATH=@localstatedir@/guix/profiles/per-user/root/guix-profile/lib/locale'
 LC_ALL=en_US.utf8
 RemainAfterExit=yes
 StandardOutput=syslog
 StandardError=syslog
--
2.21.0

bug#36076: Manual should clarify that glibc-utf8-locales is needed by default on foreign distros

[Jack Hill]

Hi Guix,

While setting up Guix on a foreign distribution (CentOS 7), I elected to 
use the full glibc-locales while following section 2.6.1 of the manual for 
application setup. I installed the glibc-locales package in both my user's 
profile and root's so that the locales would be available to guix-daemon.


However, I was surprised that even though I had the full set of locales 
available guix-daemon couldn't find the locale it was looking for. This is 
because the default systemd unit for guix-daemon configures it to use the 
en_US.utf8 locale which is provided by glibc-utf8-locales (en_US.UTF-8 is 
in glibc-locales).


I think it would be good to clarify in the manual which locale package is 
needed when using the default daemon configuration on systemd foreign 
distros.


I also talked about my experience on help-guix: 
https://lists.gnu.org/archive/html/help-guix/2019-06/msg00024.html

Best,
Jack

bug#36040: Guix Installation Bug Report

[Danny Milosavljevic]

Hi,

yeah, you have a hidden Wifi network in range and we had a bug with
them (see bug# 35941).

It has since been fixed--but there was no new Guix release yet.

If possible, can you move out of range of the hidden Wifi to install
(just to install) Guix?

If not, I can build a new installation iso for you to use.


pgpwzsCf74tRF.pgp
Description: OpenPGP digital signature

bug#36077: [staging] Mesa build failure on i686

[Marius Bakke]

Hello,

On the 'staging' branch, Mesa fails a test for i686.  From
:

--8<---cut here---start->8---
Ok:   64
Expected Fail: 0
Fail:  1
Unexpected Pass:   0
Skipped:   0
Timeout:   0


The output from the failed tests:

62/65 mesa:gallium / u_format_testFAIL 0.04 s (exit status 1)
--8<---cut here---end--->8---


signature.asc
Description: PGP signature

bug#36078: [staging] AArch64 missing substitute for 'libdrm'

[Marius Bakke]

Hello,

There is no substitute for 
/gnu/store/9b5y8vr3q67cx7gm41k5snkk9g4gmvdn-libdrm-2.4.98 on
.

Unfortunately I don't know what the problem is.  There is no build log:
.

Does Cuirass provide more information about the failed derivation
somewhere?


signature.asc
Description: PGP signature

bug#36084: ghc-tasty/ghc-clock circular dependency breaking is broken

[Robert Vollmert]

ghc-tasty depends via “inputs” on ghc-clock-bootstrap (v0.5) which is built 
without tests,
while ghc-clock (v0.7) depends via “native-inputs” on ghc-tasty for tests.

This means that any package which depends on ghc-tasty and ghc-clock is 
potentially broken,
e.g.:

Warning:
This package indirectly depends on multiple versions of the same package. 
This is very likely to cause a compile failure.
  package tasty (tasty-1.1.0.3-98rSghuW4l26JGzzQLN3ha) requires 
clock-0.5.1-KAiVgaxjUlIIuEB7RoOIe6
  package hspec-core (hspec-core-2.5.5-BSfG8Pnx1al9rTpu1j0PaW) requires 
clock-0.7.2-JStwYFlKVmNHl0K1ll1Mx5

I’d suggest breaking the cycle instead by

1. introducing tasty-bootstrap which builds against the `time` module via the 
cabal flags:

flag clock
  description:
Depend on the clock package for more accurate time measurement
  default: True

This could be done via
  (arguments `(#:configure-flags (“—flag=-clock”)))
as far as I understand.

2. changing ghc-clock to test via tasty-bootstrap (and possibly some more 
variant testing
packages that would be changed to depend on tasty-bootstrap)

3. changing tasty to test via ghc-clock.


(I gave this approach a shot myself, but I’m running into mysterious silently 
hanging guild and guix build
processes — possibly some cyclic dependencies which are noticed?)