Open Bug 1696958 Opened 3 years ago Updated 2 years ago

[regression] File downloads failing with sandboxing

Categories

(Core :: Security: Process Sandboxing, defect, P5)

Product:
Core
Component:
Security: Process Sandboxing
Version:
Firefox 87
Platform:
All
OpenBSD
Type:
defect

Tracking

()

Tracking Flags:
Tracking Status
firefox-esr78 --- unaffected
firefox86 --- unaffected
firefox87 - fix-optional
firefox88 --- fix-optional

People

(Reporter: gaston, Assigned: gaston)

References

(Regression)

Details

(Keywords: regression, regressionwindow-wanted)

Firefox 86.0 on OpenBSD allows to download files, storing them in ~/Downloads (according to browser.download.dir.

On OpenBSD, unveil syscall (cf bug #1580271) allows to only present a subset of the filesystem to processes, thus firefox doesnt know about , but sees '/Downloads' (among other dirs).

In 87.0 betas, trying to save a file badly fails (ie no blue arrow with downloads in the bar), and in the browser console i have this:

NotFoundError: Could not create directory at /home/landry/Téléchargements because the path has missing ancestor components

The file is saved (as a fallback) in /tmp/mozilla_landry0/something.part.

somehow my guess is that something changed in 87 that tries to internationalize the download path, tries to create it and fails, but :
xdg-user-dir config isnt respected either:

$grep DOWN .config/user-dirs.dirs  
XDG_DOWNLOAD_DIR="$HOME/"

browser.download.dir isnt respected either ?

my locale is globally set to LC_ALL=fr_FR.UTF-8, and about:support shows:

Paramètres d’application
Langues demandées 	["fr","und"]
Langues disponibles 	["en-US","de","fr"]
Langues de l’application 	["fr","en-US"]
Préférences régionales 	["fr-FR"]
Langue par défaut 	"en-US"
Système d’exploitation
Langues du système 	["fr-FR"]
Préférences régionales 	["fr-FR"]

setting LC_ALL=C.UTF-8 LC_CTYPE=C.UTF-8 in the env i get this in about:support

Paramètres d’application
Langues demandées 	["fr","und"]
Langues disponibles 	["en-US","de","fr"]
Langues de l’application 	["fr","en-US"]
Préférences régionales 	["fr","en-US"]
Langue par défaut 	"en-US"
Système d’exploitation
Langues du système 	["en-US-posix"]
Préférences régionales 	["en-US-posix"]

but that doesnt help downloads at all, e.g browser console still shows the same error:

NotFoundError: Could not create directory at /home/landry/Téléchargements because the path has missing ancestor components

Tried removing the two local langpacks i had installed in the profile to see if this was related, that fails with this error in the browser console:

Exception { name: "NS_ERROR_FAILURE", message: "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFile.create]", result: 2147500037, filename: "resource://gre/modules/addons/XPIInstall.jsm", lineNumber: 4579, columnNumber: 0, data: null, stack: "uninstallAddon@resource://gre/modules/addons/XPIInstall.jsm:4579:17\nuninstall@resource://gre/modules/addons/XPIDatabase.jsm:1243:23\nhandleEvent@chrome://mozapps/content/extensions/aboutaddons.js:3092:27\n", location: XPCWrappedNative_NoHelper }

manually removed the two langpacks i had under extensions/ in the profile, now trying to download a file yields:

NotFoundError: Could not create directory at /home/landry/Downloads because the path has missing ancestor components

The directory exists and is unveiled to the main process, with rwc mode, and to the content process as r mode.

Unless in 87 there's a new standalone distinct process type handling file downloads ?

Disabling unveil for the main process works around the issue, but of course this isnt acceptable.

Hello Landry,

Since you said that downloading worked on previous firefox, could you try to use mozregression to find out the problem?

Thanks.

Flags: needinfo?(landry)

[Tracking Requested - why for this release]:

status-firefox86: --- → unaffected
status-firefox87: --- → affected
tracking-firefox87: --- → ?
Flags: needinfo?(landry)

using mozregression or trying to bisect manually wont be possible unfortunately, i'm not able to build m-c for various reasons (last i tried using mach it wasnt working on openbsd) and i dont have much time to spend on this issue.

sandboxing seems to be the proper component for this bug.

Component: Networking → Security: Process Sandboxing

tier3 platform, not tracking

status-firefox87: affected → fix-optional
tracking-firefox87: ? → -
Priority: -- → P5

not 100% sure but looking at what changed recently in this area, bug #1649611 seems related.. that's a total shot in the dark, but since the browser console error message seems to come from IOUtils, will try with the calls to IOUtils.makeDirectory in toolkit/components/downloads/DownloadIntegration.jsm replaced by OS.File.makeDir that were previously here.

Regressed by: 1649611
Has Regression Range: --- → yes

what's puzzling is that those two calls in toolkit/components/downloads/DownloadIntegration.jsm have createAncestors: false so the codepath leading to the error msg NotFoundError: Could not create directory at /home/landry/Downloads because the path has missing ancestor components shouldnt be hit...

hah ! i think i got the logic wrong, passing false will check if the parent dirs exist and then fail, as the process doesnt see/knows the parent dir, only ~/Downloads.

after testing with the two IOUtils.makeDirectory calls reverted it works. After looking a bit more, i'm fighting with two issues:

  • it should be okay within IOUtils.makeDirectory to avoid erroring out if the parent dir doesnt exist, sandboxing might just prevent the process to check that. That's our case
  • the fact that now the downloads folder can be translated in toolkit/components/downloads/DownloadUIHelper.jsm breaks our assumption where by default sandboxing only give access to ~/Downloads which is 'predictable' - if the user has installed a langpack, firefox will try to create a localized folder (and fail).

While it doesn't help with the localization issue, if bug #1697721 gets fixed and createAncestors gets changed to true, downloads to ~/Downloads should start working again. It looks like not creating ancestors was done to preserve the old behavior, but there wasn't a specific reason for keeping it that way.

nah, sadly combining the various patches doesnt fix this issue.

If i reset browser.downloads.dir setting, trying to save a pdf seems to call the right API to create ~/Downloads, but the dir isnt actually created and the downloads ends up in /tmp/mozilla_landry0/XBoZJixU.pdf.part (DEBUG_NSIFILE #ifdefs removed + your patch from bug #1697721)

MakeDirectorySync: /home/landry/Downloads, createAncestors: 0, ignoreExisting: 1
nsIFile: before: /home/landry/Downloads
nsIFile: mkdir("/home")
nsIFile: mkdir("/home/landry")
nsIFile: after: /home/landry/Downloads

if i set again browser.downloads.dir to the full path and manually create ~/Downloads, saving a pdf ends up in the right dir.

My testing disagrees with yours. Fixing bug #1697721 is necessary but not sufficient to fix this one. There is a subtle interaction with unveil which may have led you astray.

The first issue is that, initially, the ~/Downloads directory does not exist, and it is not created until a file is downloaded (at least on 86 and above). On 86, directory creation succeeds; on 87, it fails. However, this is not the only reason why downloads fail. On 86, where the directory was successfully created, the download will still fail because the newly-created ~/Downloads directory is inaccessible due to unveil. Quoting the man page, "Directories are remembered at the time of a call to unveil(). This means that a directory that is removed and recreated after a call to unveil() will appear to not exist." Since the ~/Downloads directory did not exist at the time it was unveiled, it remains inaccessible. Firefox must be launched with an existing ~/Downloads directory before downloads will work. This has not changed between 86 and 87.

The second issue is IOUtils::makeDirectory being called with createAncestors set to false [1] [2] when creating ~/Downloads. This causes IOUtils::makeDirectory to perform its own directory existence check instead of relying on nsLocalFile::CreateAllAncestors. Since ~ is veiled to Firefox, this check fails, and the ~/Downloads directory is never created. I'm not sure why you saw different behavior here. I didn't mess with browser.downloads.dir. Your log says "createAncestors: 0". Is this a mistake, or did you not change createAncestors to true?

The other way to fix the second issue is to skip the parent directory existence check in IOUtils::makeDirectory and just try to create the directory. There is already code in place to handle the directory creation failing, so there should be no issues with just removing the existence check while keeping the special case for root directories.

createAncestors: 0 was with a chunk adding a similar workaround to make sure the error wasnt triggered:

   if (!aCreateAncestors) {
     nsCOMPtr<nsIFile> parent;
     MOZ_TRY(aFile->GetParent(getter_AddRefs(parent)));
+#ifdef __OpenBSD__
+    parent = nullptr;
+#endif
     if (parent) {

i've also tried with your patch + setting createAncestors to true but so far i havent been able to make it fully work.

I'm also unsure what happens if browser.downloads.dir is unset, not sure it takes a valid default value.

It seems setting the pref and starting firefox will create the dir (but it wont be available for downloads in the same process session), and after restarting firefox it is able to save files in the directory created by the previous process - as you analyzed.

so far i have two patches (on top of yours from #1697721), one to ensure we dont use a localized string:

Index: toolkit/components/downloads/DownloadUIHelper.jsm
--- toolkit/components/downloads/DownloadUIHelper.jsm.orig
+++ toolkit/components/downloads/DownloadUIHelper.jsm
@@ -154,6 +154,7 @@ XPCOMUtils.defineLazyGetter(DownloadUIHelper, "strings
       strings[stringName] = string.value;
     }
   }
+  strings["downloadsFolder"] = "Downloads";
   return strings;
 });

and one to set createAncestors to true in the two callers:

Index: toolkit/components/downloads/DownloadIntegration.jsm
--- toolkit/components/downloads/DownloadIntegration.jsm.orig
+++ toolkit/components/downloads/DownloadIntegration.jsm
@@ -366,7 +366,7 @@ var DownloadIntegration = {
           );
           directoryPath = directory.path;
           await IOUtils.makeDirectory(directoryPath, {
-            createAncestors: false,
+            createAncestors: true,
           });
         } catch (ex) {
           // Either the preference isn't set or the directory cannot be created.
@@ -944,7 +944,7 @@ var DownloadIntegration = {
 
     // Create the Downloads folder and ignore if it already exists.
     return IOUtils.makeDirectory(directoryPath, {
-      createAncestors: false,
+      createAncestors: true,
     }).then(() => directoryPath);
   },
Severity: -- → S4
Hardware: Unspecified → All

Anything I can do for you here? Might the second part of your patch be upstreamable?

Assignee: nobody → landry
Flags: needinfo?(landry)

well, in the end when i updated our package to 87 i ended up with 3 patches:

with all that, it's still a bit non-functional with new/empty profiles, as (from my testing) you need to start firefox once so that it creates ~/Downloads, but in the same session you can't use it iirc (as it isnt present when unveiling it ?), you need to kill/restart the browser so that it can properly save files to/upload files from ~/Downloads.

in my testing, iirc using IOUtils.makeDirectory with createAncestors: true didnt work for some reason, but i dont remember if this was with the patch from bug #1697721.

Ppl tell me i should use a wrapper script to create ~/Downloads before starting firefox but i've always found that ugly. Or create ~/Downloads in the code before calling unveil, but that would also be a local patch not upstreamable.

Given the situation, i'm really open to better ideas.. that unveil integration isnt my thing, and it's a maze i dont want to be responsible for. jcs might have an idea ?

Flags: needinfo?(landry) → needinfo?(jcs)

StartOpenBSDSandbox already does a mkdir for some dconf directory, can't we just do the same thing for localized Downloads?

    case GeckoProcessType_Default: {
      OpenBSDFindPledgeUnveilFilePath("pledge.main", pledgeFile);
      OpenBSDFindPledgeUnveilFilePath("unveil.main", unveilFile);

      // Ensure dconf dir exists before we veil the filesystem
      nsAutoCString dConf("$XDG_CACHE_HOME/dconf");
      ExpandUnveilPath(dConf);
      MkdirP(dConf);
      break;
    }

We can replace the hard-coded ~/Downloads rwc in our *.unveil files with something dynamic that gets expanded in ExpandUnveilPath. I'm still not sure if Firefox is supposed to be honoring $XDG_DOWNLOAD_DIR, but if so it can just use that.

Flags: needinfo?(jcs)

I did have a concern during the initial unveil implementation: the existing dynamically-expanded things in the *.unveil files append something to the dynamic path, so at worse if a child process can start with a corrupted environment, an attacker can't make an environment variable point to just / and then get / rwc. At worse a variable like $XDG_CONFIG_HOME set to / would add paths like //dconf rwc which isn't very useful. For the download path, if it's just $XDG_DOWNLOAD_DIR rwc, a similar scenario would get expanded to / rwc.

Since the Downloads path will be the full path, this will make it slightly less secure because corrupting $XDG_DOWNLOAD_DIR in the environment or some about:config variable that determines the localized path could lead to effectively disabling unveil. Corrupting the environment would probably be harder because it would have to be done in the parent process, but I assume being able to write to a config variable can be done anywhere and it propagates to new child processes later.

But either way, it still raises the bar for an attack.

per https://searchfox.org/mozilla-central/search?q=XDG_DOWNLOAD_DIR&path= it seems firefox doesnt honour XDG_DOWNLOAD_DIR anyway, so i guess it should rather create (if not existing) the browser.download.dir path (in that case that's the responsability of the user to unveil it), or if unset ~/Downloads

fwiw, it seems what you did for dconf back in the days is now broken here, as firefox constantly complains for not being able to access/create another dir..

(firefox:52194): dconf-CRITICAL **: 19:02:56.747: unable to create file '/var/run/user/1000/dconf/user': Permission denied.  dconf will not work properly.

oh, var/run/user/1000 is apparently $XDG_RUNTIME_DIR ...

that dconf thing will be taken care of in bug #1714018

Uh, so... are downloads still completely broken on openBSD? Is there something we can do to help?

Flags: needinfo?(landry)

(In reply to :Gijs (back Thu 8 Sep; he/him) from comment #26)

Uh, so... are downloads still completely broken on openBSD? Is there something we can do to help?

There were several distinct issues, if my memory is still correct:

  • as i said in comment 20 i had a local revert of a part of aadba76932ea because createAncestors had a wrong behaviour - this was apparently solved by https://hg.mozilla.org/mozilla-central/rev/c41a7b705276 in bug #1697721 (and i hadnt recently rechecked), so i've removed the local revert when updating our package to 104.
  • i still have a local patch to ensure we don't use a localized name for ~/Downloads (eg override downloadsFolder), because we unveil only ~/Downloads and don't want to deal with localizations. I think at some point i tried to enforce it by setting browser.download.dir but that uses the full path with ~ expanded and cant be globally setup for all users.
  • as it is now with 104, downloads properly work iff ~/Downloads exists. If it doesn't exist (new user/install/profile), the first initial run of firefox will create it - From that first running session, downloads do fail, but if i close and reopen firefox downloads start working.

as for unveil behaviour, i'll have to reread http://man.openbsd.org/unveil, but in the current unveil config we use:

/etc/firefox/unveil.content:~/Downloads r
/etc/firefox/unveil.main:~/Downloads rwc

which means the main process can create, read and write the directory, and the content process can read it (eg i suppose for file uploads usecase), but i'll have to check with ktrace what is done first (unveiling the paths, or try creating ~/Downloads) and what are the consequences.

what can i try in terms of MOZ_LOG classes to try understanding what might fail ?Ideally having downloads working straight away with a new profile would be nicer :)

Flags: needinfo?(landry)

i've built a simple reproducer that shows that if a dir doesnt exist, and is unveiled, it can be created but it cant be used ?

[23:34] c64:~/ $cat unveil.c                                                                                                                                                                                        
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#define XXX "/tmp/X1"
const char *str = "Temporary string to be written to file!";
int main() {
int ret = 0;
  struct stat st = {0};
  ret = unveil(XXX,"rwc");
  printf("unveiled /tmp/X1 with rwc:%d\n", ret);
  unveil(NULL, NULL);
  if (stat(XXX, &st) == -1) {
    ret = mkdir(XXX, 0700);
    printf("created /tmp/X1:%d\n", ret);
  } else {
    printf("/tmp/X1 exists\n");
  }
  FILE* output_file = fopen("/tmp/X1/bla", "w+");
  if (!output_file) {
    perror("fopen");
    exit(EXIT_FAILURE);
  }

  fwrite(str, 1, strlen(str), output_file);
  printf("Done Writing!\n");
  fclose(output_file);
}
[23:33] c64:~/ $./unveil                                                                                                                                                                                            
unveiled /tmp/X1 with rwc:0
created /tmp/X1:0
fopen: No such file or directory
[23:33] c64:~/ $./unveil       
unveiled /tmp/X1 with rwc:0
/tmp/X1 exists
Done Writing!

same thing if unveiled with rwxc mode.

ktrace'ing firefox when ~/Downloads doesnt exist, i get this:

# ~/Downloads is unveiled
 49378 firefox  CALL  unveil(0x7f7ffffed92c,0x7f7ffffed994)
 49378 firefox  STRU  flags="rwc"
 49378 firefox  NAMI  "/home/landry/Downloads"
# ~/Downloads is created
 49378 firefox  CALL  mkdir(0x530bb065a8,0755<S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH>)
 49378 firefox  NAMI  "/home/landry/Downloads"
 49378 firefox  RET   mkdir 0
# file is downloaded to /tmp
 49378 firefox  CALL  open(0x525318cd08,0xe01<O_WRONLY|O_CREAT|O_TRUNC|O_EXCL>,0600<S_IRUSR|S_IWUSR>)
 49378 firefox  NAMI  "/tmp/KOmKpafU.tgz"
# firefox tries to move it to ~/Downloads, and fails
 49378 firefox  CALL  open(0x53010a7108,0xe01<O_WRONLY|O_CREAT|O_TRUNC|O_EXCL>,0600<S_IRUSR|S_IWUSR>)
 49378 firefox  NAMI  "/home/landry/Downloads/KOmKpafU.tgz.part"
 49378 firefox  RET   open -1 errno 2 No such file or directory
 49378 firefox  CALL  mkdir(0x52458ba1c8,0700<S_IRUSR|S_IWUSR|S_IXUSR>)
 49378 firefox  NAMI  "/home"
 49378 firefox  RET   mkdir -1 errno 2 No such file or directory
 49378 firefox  CALL  access(0x52458ba1c8,0<F_OK>)
 49378 firefox  NAMI  "/home"
 49378 firefox  RET   access -1 errno 2 No such file or directory
 49378 firefox  CALL  mkdir(0x52458ba1c8,0700<S_IRUSR|S_IWUSR|S_IXUSR>)
 49378 firefox  NAMI  "/home/landry"
 49378 firefox  RET   mkdir -1 errno 2 No such file or directory
 49378 firefox  CALL  access(0x52458ba1c8,0<F_OK>)
 49378 firefox  NAMI  "/home/landry"
 49378 firefox  RET   access -1 errno 2 No such file or directory
 49378 firefox  CALL  mkdir(0x52458ba1c8,0700<S_IRUSR|S_IWUSR|S_IXUSR>)
 49378 firefox  NAMI  "/home/landry/Downloads"
 49378 firefox  RET   mkdir -1 errno 2 No such file or directory
 49378 firefox  CALL  access(0x52458ba1c8,0<F_OK>)
 49378 firefox  NAMI  "/home/landry/Downloads"
 49378 firefox  RET   access -1 errno 2 No such file or directory

closing firefox, reopening it:

# ~/Downloads is unveiled
 72114 firefox  CALL  unveil(0x7f7ffffed81c,0x7f7ffffed884)
 72114 firefox  STRU  flags="rwc"
 72114 firefox  NAMI  "/home/landry/Downloads"
# firefox tries to recreate it anyway
 72114 firefox  CALL  mkdir(0x3acb590fde8,0755<S_IRUSR|S_IWUSR|S_IXUSR|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH>)
 72114 firefox  NAMI  "/home/landry/Downloads"
 72114 firefox  RET   mkdir -1 errno 17 File exists
# file is downloaded to /tmp
 72114 firefox  CALL  open(0x3ad48099fe8,0xe01<O_WRONLY|O_CREAT|O_TRUNC|O_EXCL>,0600<S_IRUSR|S_IWUSR>)
 72114 firefox  NAMI  "/tmp/raYsrZP5.tgz"
 72114 firefox  RET   open 135/0x87
# then moved to ~/Downloads
 72114 firefox  CALL  unlink(0x3ad48099fe8)
 72114 firefox  NAMI  "/tmp/raYsrZP5.tgz"
 72114 firefox  RET   unlink 0
 72114 firefox  CALL  open(0x3ad480cf2c8,0xe01<O_WRONLY|O_CREAT|O_TRUNC|O_EXCL>,0600<S_IRUSR|S_IWUSR>)
 72114 firefox  NAMI  "/home/landry/Downloads/raYsrZP5.tgz.part"
 72114 firefox  RET   open 135/0x87

(In reply to Landry Breuil (:gaston) from comment #28)

i've built a simple reproducer that shows that if a dir doesnt exist, and is unveiled, it can be created but it cant be used ?

I can only apologize for my ignorance of all things OpenBSD - but does this imply that there's an issue with unveil, ie outside of Firefox? Or am I misunderstanding this comment? Is this issue tracked somewhere upstream?

Flags: needinfo?(landry)

(In reply to :Gijs (he/him) from comment #30)

(In reply to Landry Breuil (:gaston) from comment #28)

i've built a simple reproducer that shows that if a dir doesnt exist, and is unveiled, it can be created but it cant be used ?

I can only apologize for my ignorance of all things OpenBSD - but does this imply that there's an issue with unveil, ie outside of Firefox? Or am I misunderstanding this comment? Is this issue tracked somewhere upstream?

it's technically a known "quirk" in the unveil() implementation, not a bug, but we have to live with that behaviour. The manpage specifies This means that a directory that is removed and recreated after a call to unveil() will appear to not exist.

I'm reluctant to add an mkdirP("~/Downloads") call in https://searchfox.org/mozilla-central/source/dom/ipc/ContentChild.cpp#4878 but i guess we have no way around that.. and that's more or less what our chromium port does in its wrapper script, cf https://github.com/openbsd/ports/blob/master/www/chromium/files/chrome#L51

Flags: needinfo?(landry)

(In reply to Landry Breuil (:gaston) from comment #31)

(In reply to :Gijs (he/him) from comment #30)

(In reply to Landry Breuil (:gaston) from comment #28)

i've built a simple reproducer that shows that if a dir doesnt exist, and is unveiled, it can be created but it cant be used ?

I can only apologize for my ignorance of all things OpenBSD - but does this imply that there's an issue with unveil, ie outside of Firefox? Or am I misunderstanding this comment? Is this issue tracked somewhere upstream?

it's technically a known "quirk" in the unveil() implementation, not a bug, but we have to live with that behaviour. The manpage specifies This means that a directory that is removed and recreated after a call to unveil() will appear to not exist.

I'm reluctant to add an mkdirP("~/Downloads") call in https://searchfox.org/mozilla-central/source/dom/ipc/ContentChild.cpp#4878 but i guess we have no way around that.. and that's more or less what our chromium port does in its wrapper script, cf https://github.com/openbsd/ports/blob/master/www/chromium/files/chrome#L51

Hm, sounds like that would be the way around it, yes - though would we need to do this for the user-configured download dir rather than hardcoding ~/Downloads ? And if so, does that mean changing that default directory at runtime wouldn't work (would require a restart)?

Flags: needinfo?(landry)

(In reply to :Gijs (he/him) from comment #32)

I'm reluctant to add an mkdirP("~/Downloads") call in https://searchfox.org/mozilla-central/source/dom/ipc/ContentChild.cpp#4878 but i guess we have no way around that.. and that's more or less what our chromium port does in its wrapper script, cf https://github.com/openbsd/ports/blob/master/www/chromium/files/chrome#L51

Hm, sounds like that would be the way around it, yes - though would we need to do this for the user-configured download dir rather than hardcoding ~/Downloads ? And if so, does that mean changing that default directory at runtime wouldn't work (would require a restart)?

Yes, i think changing the Downloads directory at runtime via the UI would only work if 1) the directory already exists before starting firefox 2) it is already listed in unveil config.

Right now, i preferred hardcoding ~/Downloads (and clearly documenting it in the package readme, cf http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/mozilla-firefox/pkg/README?rev=1.33&content-type=text/x-cvsweb-markup), not taking care of localizations, and im not sure we want to support all crazy customizations possible - eg XDG_DOWNLOAD_DIR, vs browser.download.dir ?

or we can default to mkdirP("~/Downloads") only if browser.download.dir & XDG_DOWNLOAD_DIR is unset, preferring one then the other and fallback to hardcoded ? if ppl set those/change them from the defaults, might aswell assume they know what they're doing ?

Flags: needinfo?(landry)
You need to log in before you can comment on or make changes to this bug.