adsys pam issues

Yeah, I don’t really think those are security issues, let’s open it, but fix some of them:

-> the 2 arggv leak in fork()
This is only if a failure and just before exiting the child process. Anyway, let’s free it :)

-> status != 0
Yes, we could have done something like the man page:
               do {
                   w = waitpid(cpid, &wstatus, WUNTRACED | WCONTINUED);
                   if (w == -1) {
                       perror("waitpid");
                       exit(EXIT_FAILURE);
                   }

                   if (WIFEXITED(wstatus)) {
                       printf("exited, status=%d\n", WEXITSTATUS(wstatus));
                   } else if (WIFSIGNALED(wstatus)) {
                       printf("killed by signal %d\n", WTERMSIG(wstatus));
                   } else if (WIFSTOPPED(wstatus)) {
                       printf("stopped by signal %d\n", WSTOPSIG(wstatus));
                   } else if (WIFCONTINUED(wstatus)) {
                       printf("continued\n");
                   }
               } while (!WIFEXITED(wstatus) && !WIFSIGNALED(wstatus));

However, this seems equivalent to me. We took our inspiration from pam_exec() with the different kinds of errors and this is the way they are using it, so we did something similar as we are running in the same context. It ought to work as we tested different kind of failures manually and used scenarios worked. Note that pam_exec don’t even free argvv for the parent process :p
So, depending on how strongly you feel about it, tell us!

-> gethostname() indentation
typical case of "this was looking fine in my editor" :p This is why I like Go with autoformatting :)

arggv leak and gethostname identation fixed in https://github.com/ubuntu/adsys/pull/285.

Hah so I've got another bug to report then? :) Thanks!

Oh yes, the waitpid() stuff.. if it works in testing, leaving it alone is probably fine then. Thanks.

Heh, so Dmitry might actually remove the other 'free' calls in the error paths :) -- and he spotted that the strdup() calls are unchecked:

https://github.com/linux-pam/linux-pam/issues/444

It might be worth mirroring whatever he decides to do.

Thanks

Ah, thanks! Lurking there and will shamelessly copy what they decide to do! :)

This bug was fixed in the package adsys - 0.8.1

---------------
adsys (0.8.1) jammy; urgency=medium

  * Change chown logic on script directory and parents to avoid potential
    vulnerability. (LP: #1961458)
  * Separate readiness from session running to avoid unrefreshed user script
    directories after a logout without any new logins.
  * pam_adsys: Fix memory leak and identation. (LP: #1961459)
  * Adapt to newer samba, while keeping backward compatilibity for CI.
    Thanks Michael. (LP: #1962170)
  * Try to stabilize configuration detection change test by calling sync() to
    sync FHS to disk, and then, hoping we get the inotify update. Seems to fix
    flakyness on armhf. (LP: #1962510)
  * Enforce closing stderr on ppcel64 in tests with new samba to avoid hangs
    in race.
  * Fix linting issues discovered by new golangci-lint.
  * Misc syntax polish.
  * Dependencies update:
    - github.com/godbus/dbus/v5
    - github.com/golangci/golangci-lint
    - gopkg.in/ini.v1

 -- Didier Roche <email address hidden> Tue, 08 Mar 2022 09:49:08 +0100

Hello Seth, or anyone else affected,

Accepted adsys into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/adsys/0.9.2~20.04 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Verified successfully on Focal with adsys 0.9.2~20.04.

Marking as verification-done

The verification of the Stable Release Update for adsys has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package adsys - 0.9.2~20.04

---------------
adsys (0.9.2~20.04) focal; urgency=medium

  * Backport to focal
    - Build with Go 1.16
    - Move debhelper compat to 12
    - Do not recommends ubuntu-advantage-desktop-daemon as it’s not available
      on focal yet.

adsys (0.9.2) kinetic; urgency=medium

  * Update generators to fix FTBFS
    - shell out to mkdir instead of go's os.Mkdir which can bypass fakeroot's
      filesystem hijacking and cause unexpected behavior
  * Update dependencies to latest:
    - github.com/golangci/golangci-lint
    - google.golang.org/protobuf

adsys (0.9.1) kinetic; urgency=medium

  [ Didier Roche ]
  [ Gabriel Nagy ]
  * Fix loading policy content from uppercase folders (LP: #1982330)
  * Add GSettings power management keys (LP: #1982349)
  * Allow parsing policy entries with empty values (LP: #1982342)
  * Allow parsing policies with unsupported types (LP: #1982343)
  * Allow parsing policy entries with no data (LP: #1982345)
  * Lowercase target name when normalizing (LP: #1982347)
  * Annotate policies that require Ubuntu Pro (LP: #1982348)
  * Update dependencies to latest:
    - github.com/spf13/cobra
    - github.com/spf13/viper
    - github.com/stretchr/testify
    - github.com/charmbracelet/bubbletea
    - github.com/charmbracelet/bubbles
    - google.golang.org/grpc
    - github.com/golangci/golangci-lint
    - github.com/sirupsen/logrus

adsys (0.9.0) kinetic; urgency=medium

  [ Jean-Baptiste Lallement ]
  [ Didier Roche ]
  [ Gabriel Nagy ]
  * Add Active Directory Watch Daemon - adwatchd: (LP: #1982351)
    - Implement a Windows daemon that watches a list of configured directories
      for changes and bumps the relevant GPT.INI files.
    - Add adsys-windows binary package which includes the Windows daemon
      executable and the admx/adml policies.
  * Config detection now includes current executable directory
  * Fixes in generator build race
  * Update dependencies to latest:
    - github.com/spf13/cobra
    - github.com/stretchr/testify
  * CI updates:
    - switch to Go setup v3
    - bump to really build with Golang 1.18

adsys (0.8.6) kinetic; urgency=medium

  * Fix new build failures on 32 bits due to libsmbclient-dev no longer sets
    the large file support cflags in libsmbclient.h.
    Update to latest libsmbclient-go.
  * Update dependencies to latest:
    - google.golang.org/grpc
    - gopkg.in/ini.v1
    - github.com/golangci/golangci-lint
    - github.com/spf13/viper
    - github.com/stretchr/testify

adsys (0.8.5) kinetic; urgency=medium

  [ Jean-Baptiste Lallement ]
  [ Didier Roche ]
  * Rename chapters to be in correct ascii order when viewed online.
    Thanks to Anton Drastrup-Fjordbak.
  * Include 22.04 in admx/adml for lts only releases. (LP: #1973745)
  * Bump embedeed dependencies minor versions for both bug fixes and minor
    security enhancements.
  * Fix dconf keys not being readable by user after applying policy.
    (LP: #1973748)
  * Ensure we can execute machine and user scripts:
    /run is now noexec on Ubuntu. Ensure that we can execute the scripts in
    /run/adsys subdirectories. The scripts mechanism has been reviewed by the...