[MIR] adcli

Bug #1868159 reported by Jason Edgecombe
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
adcli (Ubuntu)
Fix Released
Medium
Unassigned

Bug Description

I request that the adcli package be included in main.

adcli is a key support tool for the many organizations that use Linux machines with Active Directory. Many enterprises are requiring that Linux machine join their AD domains and use AD authentication.

adcli is a necessity for using kerberized NFS4 in an Active Directory environment. NFS4 with kerberos (sec=krb5 mount option). adcli is used to add the nfs/hostname service name to the machine's keytab and AD computer account, which is required on the NFS server and any NFS4 clients using leases.

Note this MIR is related to MIR in bug 1868154

Dan Streetman (ddstreet)
Changed in adcli (Ubuntu):
assignee: nobody → Dan Streetman (ddstreet)
Revision history for this message
Dan Streetman (ddstreet) wrote :
Download full text (3.3 KiB)

[Summary]
This package is acceptable for MIR, with my only concern being the very
long period between releases upstream, and the lack of upstream commits
pulled into Debian and/or Ubuntu between upstream releases.

This does need a security review, so I'll assign ubuntu-security after
the next MIR team mtg, if the team agrees with my review.

Notes/TODOs:
As I'm new to the MIR team, I am making this approval conditional on
MIR team review of my review at the next MIR team mtg.

[Duplication]
- There is no other package in main providing the same functionality.
  - Note: as with my review for realmd, it is possible perform manual
    configuration/steps for similar functionality; this package
    automates and simplifies much of the manual work.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  - includes build dep from universe, but all binary deps are from main
- no -dev/-debug/-doc packages that need exclusion

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking

[Security]
OK:
- history of CVEs does not look concerning
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop

Problems:
- does parse data formats
- does deal with system authentication (eg, pam), etc)

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
    - added forced error to src pkg to verify
- The package has a team bug subscriber
  - MIR requestor is subscribed to all realmd bugs in Ubuntu
- not a python package
- not a Go package

Problems:
- does have a test suite that runs as autopkgtest
  - this is probably ok, since there are build-time tests run, and this
    is a relatively simple package
- no translation present
  - translation would be good to have, but probably is not a requirement
    as this package provides only a single tool, and that tool is at least
    partially used by end-users indirectly through realmd.

[Packaging red flags]
OK:
- Ubuntu does not carry a delta
- symbols tracking not applicable for this kind of code.
- d/watch is present and looks ok
- Upstream update history is good
- the current release is packaged
- promoting this does not seem to cause issues for MOTUs
  - rarely, if ever, patched in Ubuntu
- no massive Lintian warnings
- d/rules is rather clean
- Does not have Built-Using
- Not Go Package

Problems:
- Debian update history is slow
  - only updates are new upstream releases
  - while upstream does commit to git often, upstream releases are sparse
    last upstream release was 6 months ago (good) but before that
    ~3.5 years ago (bad)
- Ubuntu update history is nonexistent
  - no Ubuntu patches to package for any currently supported release

[Upstream red flags]
OK:
- no Errors/warnings during the build
- no incautious use of malloc/sprintf (as far as I can check it)
- no use of sudo, gksu, pkexec, or LD_LIBRARY_PATH
- no use of user nobody
- no use of setuid
- no important open bugs...

Read more...

Changed in adcli (Ubuntu):
importance: Undecided → Medium
Revision history for this message
Dan Streetman (ddstreet) wrote :

Follow up:

After discussion with MIR team, the item I identified (long upstream release cycles) does need attention, similar to the comments for the related MIR bug 1868154, however since upstream adcli does have a relatively recent release, and Debian/Ubuntu are up to date with that release, I don't feel that any immediate action is needed there.

But, MIR does require that a reliable team does need to subscribe to all the package's bug reports. I would expect the owning team to be proactive about pulling upstream bugfixes more regularly.

Revision history for this message
Dan Streetman (ddstreet) wrote :

And to clarify, after the above is addressed, this still does need a security review.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Having used realmd (and adcli indirectly) while updating the Ubuntu Server Guide, I must say it's a very useful tool, and I wouldn't want to join an Active Directory domain without it (and adcli).

Revision history for this message
Dan Streetman (ddstreet) wrote :

This still needs server team agreement for ownership (per comment 2), but while that's being evaluated I'll put this on the security team, to review in parallel.

Changed in adcli (Ubuntu):
assignee: Dan Streetman (ddstreet) → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Ubuntu server is subscribed.

Revision history for this message
Alex Murray (alexmurray) wrote : security audit

I reviewed adcli 0.9.0-1 as checked into groovy. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

adcli is a command-line utility to join computers to Active Directory
domains, integrating with kerberos as well.

- CVE History:
  - No CVE history
- Build-Depends
  - libsasl2-dev, libkrb5-dev, libldap2-dev
- No pre/post inst/rm scripts
- No init scripts
- No systemd units
- No dbus services
- No setuid binaries
- binaries in PATH
  - /usr/sbin/adcli
- No sudo fragments
- No polkit files
- No udev rules
- No cron jobs
- Library unit tests are run during the build - these test both data
  structure implementations as well as utility functions like calling out
  to external programs, and more featured tests like kerberos
  authentication etc.
- No autopkgtests
- Build logs are pretty clean - only thing of note is that during linking
  of adcli itself (and some of the unit tests) the following is emitted:

/usr/bin/ld: warning: libkrb5.so.26, needed by /usr/lib/x86_64-linux-gnu/libgssapi.so.3, may conflict with libkrb5.so.3

- Spawns an external process to call out to samba to get samba data - but
  this needs to be specified as a command-line argument so cannot be used
  to cross a security boundary.
- Involves a large amount of dynamic memory management via malloc() /
  calloc() / asprintf() etc - this appears to be quite defensive and
  appropriately checks bounds and errors etc.
- File IO is used when setting up a temporary kerberos configuration - file
  paths are generated sanely and using mkdtemp(). umask is used to restrict
  access to only the owner as well.
- Logging is done via warnx() etc - no obvious instances of format string
  vulnerabilities or similar.
- Environment variables used:
  - KRB5_CONFIG - used to specify an existing kerberos configuration when
    generating a local temporary kerberos configuration
  - TMPDIR - used to determine where to locate the temporary kerberos
    configuration
  - ADCLI_STRICT - used to abort() early during failures of precondition
    checks - this is not set by default but is used during
    development.
- No use of privileged functions
- Uses kerberos for cryptography / random numbers etc
- Uses of temp files via TMPDIR and mkdtemp() to ensure names are not
  predictable etc.
- No direct use of networking - is done via libkrb / libldap2 etc
- No use of WebKit
- No use of PolicyKit

- No significant cppcheck results
  - 3 issues found but all in the unit tests
- No significant Coverity results
  - There are a few minor memory leaks but for a cli util which is not
    long-lived this is neglibigle. There are a few use-after-free issues
    but these are all in the unit tests and would occur only during test
    failure so again no need to worry about.

adcli appears well written and contains no glaring issues from a security
perspective. The maintenance history is perhaps a bit concerning but
assuming upstream are responsive to any possible security issues this
should be ok.

Security team ACK for promoting adcli to main.

Changed in adcli (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
tags: added: security-review-done
Dan Streetman (ddstreet)
Changed in adcli (Ubuntu):
status: New → In Progress
Dan Streetman (ddstreet)
Changed in adcli (Ubuntu):
assignee: nobody → Dan Streetman (ddstreet)
Dan Streetman (ddstreet)
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thank you all, MIR Ack, Security Ack and bug subscription is present - this is ready for promotion as soon as the seed was modified to pull it in.
The same applies to the sibling bug in 1868154

Changed in adcli (Ubuntu):
assignee: Dan Streetman (ddstreet) → nobody
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

$ ./change-override -c main -S adcli
Override component to main
adcli 0.9.0-1 in groovy: universe/misc -> main
adcli 0.9.0-1 in groovy amd64: universe/admin/optional/100% -> main
adcli 0.9.0-1 in groovy arm64: universe/admin/optional/100% -> main
adcli 0.9.0-1 in groovy armhf: universe/admin/optional/100% -> main
adcli 0.9.0-1 in groovy ppc64el: universe/admin/optional/100% -> main
adcli 0.9.0-1 in groovy riscv64: universe/admin/optional/100% -> main
adcli 0.9.0-1 in groovy s390x: universe/admin/optional/100% -> main
Override [y|N]? y
7 publications overridden.

Changed in adcli (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.