1592922 - AutoConfig (MCD): getLDAPAttributes() > processLDAPValues() ("user supplied method") fails in TB 68

Status: RESOLVED FIXED

(Core :: AutoConfig (Mission Control Desktop), defect)

Description

[mojmir21]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:70.0) Gecko/20100101 Firefox/70.0

Steps to reproduce:

Mozilla.cfg:
getLDAPAttributes("ldap.***.***.cz","ou=people,dc=fio,dc=cz","uid=" + env_user,"uid,cn,mail,labeledURI");

Actual results:

Error: getLDAPAttibutes fail: ReferenceError: processLDAPValues is not defined

Expected results:

It should work with "getLDAPAttributes", not "getLDAPAttibutes". Last version not afected is 60.9.0

Comment 1

[Jorg K (CEST = GMT+2)]

Mike, looks like a typo here in Mozilla core code:
https://searchfox.org/comm-central/rev/a1c52b2c68bc40266e42e193a785106e767db946/mozilla/extensions/pref/autoconfig/src/prefcalls.js#158

I'll do a patch and you can find the right component for it.

Comment 2

[Jorg K (CEST = GMT+2)]

Attachment: Bug 1592922 - Fix typo in autoconfig's prefcalls.js. r=mkaply DONTBUILD

Comment 3

[Jorg K (CEST = GMT+2)]

So what's the issue here? That there is a typo in the error message or that the function doesn't work?

Comment 4

[mojmir21]

(In reply to Jorg K (GMT+2) from comment #3)

So what's the issue here? That there is a typo in the error message or that the function doesn't work?

The function doesn't work.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eecadaaac3e5
Fix typo in autoconfig's prefcalls.js. r=mkaply DONTBUILD

Comment 6

[Jorg K (CEST = GMT+2)]

Mike, thanks for the review. I fully agree about the DONTBUILD, although I've see M-C developers push comment changes of a few lines causing an entire build.

Now that we fixed the typo, what can be said about the malfunction? What is that LDAP stuff residing in M-C? Does that call into TB's LDAP handling? It does, incredible:
https://searchfox.org/comm-central/rev/a1c52b2c68bc40266e42e193a785106e767db946/mozilla/extensions/pref/autoconfig/src/prefcalls.js#146
https://searchfox.org/comm-central/search?q=nsILDAPURL&case=false&regexp=false&path=
https://searchfox.org/comm-central/search?q=nsILDAPSyncQuery&case=false&regexp=false&path=

Note that TB's LDAP handling with SSL is broken in TB 68, see bug 1576364.

Ben, more LDAP chagrin.

Comment 7

[Ben Campbell]

Hmm.

1. The offending function, processLDAPValues doesn't appear to exist anywhere:
   https://searchfox.org/comm-central/search?q=processLDAPValues&path=
   Comment just above where it's called says "user supplied method".
   I had to look it what the autoconfig module actually is - it's MCD, Mission Control Desktop, AKA AutoConfig). The linked page has an example processLDAPValues function.

2. I really don't know what the status of the "autoconfig" module is. Even supplying a processLDAPValues function, I suspect there might be issues...

3. If it's supposed to work under both TB and Firefox, presumably its LDAP support should be disabled under Firefox, given that the LDAP support won't even be available? If it's TB-specific, then presumably the whole module should be moved out into comm-central.

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/eecadaaac3e5

Comment 9

[github]

I have used autoconfig for thunderbird (on Ubuntu) pretty much the way it is described in MCD and it has worked just fine until now. I use it to set up default email and address book for a few hundred users.

Comment 10

[yr]

I reported a bug which might be related to the issues described here bug 1628580

IMO the processLDAPValues function, which has to be self-defined in autoconfiguration, seem to not process values returned by LDAP.
This is blocking us to update 1600 Users :-/

Comment 11

[pierre.marie.ubm]

Hello, any update on this problem ?
How could I know if the function getLDAPAttributes is returning something ?
can I debug it ?

Comment 12

[Wayne Mery (:wsmwk)]

Thomas, geoff, note comment 10. also, need a component.

Comment 13

[marinar]

This bug and lack of support for Enterprise configurations for Thunderbird holding us from update and maybe looking at different mail client.

Comment 14

[Thomas D. (:thomas8)]

(In reply to Wayne Mery (:wsmwk) from comment #12)

Thomas, geoff, note comment 10. also, need a component.

The original product/component for this is Core : AutoConfig (Mission Control Desktop).
Which currently has 7 open bugs, all assigned to Mike Kaply.
We'll have to check with him about the status of that component and if it's still advisable to move our bugs there.

AutoConfig (MCD) lives here as an mozilla extension:
https://searchfox.org/comm-central/source/mozilla/extensions/pref/autoconfig/src/prefcalls.js

getLDAPAttributes() is found in prefcalls.js and is part of the API:
https://searchfox.org/comm-central/rev/48425df835d9ba232b7db39fcd72f93051e3e870/mozilla/extensions/pref/autoconfig/src/prefcalls.js#216-232

var APIs = {
  pref,
  defaultPref,
  lockPref,
  unlockPref,
  getPref,
  clearPref,
  setLDAPVersion,
  getLDAPAttributes,
  getLDAPValue,
  displayError,
  getenv,
};

for (let [defineAs, func] of Object.entries(APIs)) {
  Cu.exportFunction(func, gSandbox, { defineAs });
}

processLDAPValues (user provided function) gets called inside API function getLDAPAttributes():
https://searchfox.org/comm-central/rev/48425df835d9ba232b7db39fcd72f93051e3e870/mozilla/extensions/pref/autoconfig/src/prefcalls.js#128-160

function getLDAPAttributes(host, base, filter, attribs, isSecure) {
  [snip]
  processLDAPValues(ldapquery.getQueryResults(url, gVersion));
}

processLDAPValues(ldapquery.getQueryResults(url, gVersion));
https://searchfox.org/comm-central/rev/48425df835d9ba232b7db39fcd72f93051e3e870/mozilla/extensions/pref/autoconfig/src/prefcalls.js#156

It would be good to know this from affected users:

• After using API function getLDAPAttributes(), does your own function, processLDAPValues(), get called at all?
• Can we have an updated error message now that the spelling error has been fixed?

Comment 15

[Mike Kaply [:mkaply]]

Kris:

When we implemented the Sandboxing:

https://hg.mozilla.org/mozreview/gecko/rev/ad42d9c4fc178abff2730a8e0d6f63ef01264e35#index_header

We didn't take into account that getLDAPAttributes needs to call a local function in the autoconfig file called "processLDAPValues". Basically user created it and LDAP code calls it:

https://searchfox.org/mozilla-central/source/extensions/pref/autoconfig/src/prefcalls.js#156

Do you think itis just a matter of adding that function to the list of APIs? Or is there something else we need to do to enable calling that function in the sandbox?

Comment 16

[Kris Maglione [:kmag]]

We did consider it, we just decided not to worry about it, since it wasn't used or tested.

The fix would be to call gSandbox.processLDAPValues, or change the API to have the caller pass a callback or expect a promise instead (which would be preferable, especially the promise variant).

That said, I seem to recall there having been other issues with that API, where it wanted to accept or return XPCOM objects which the sandbox couldn't have access to. Looking at the code now, though, it seems to just deal with strings, so maybe it would be fine.

Comment 17

[Mike Kaply [:mkaply]]

So to preface this, what happens in practice is that when prefcalls.js calls processLDAPValues, startup hangs. Thunderbird never comes up, and you have to Ctrl+C.

The fix would be to call gSandbox.processLDAPValues, or change the API to have the caller pass a callback or expect a promise instead (which would be preferable, especially the promise variant).

I tried gSandbox.processLDAPValues, but I got the same results, hang at startup.

Going to get a debug build going so I can take a closer look.

Comment 18

[Mike Kaply [:mkaply]]

debug build provides no extra info on the console. Just a straight up hang at startup.

Looking in the debugger, we hang doing the actual evaluate:

https://searchfox.org/mozilla-central/rev/e0eb861a187f0bb6d994228f2e0e49b2c9ee455e/js/xpconnect/src/Sandbox.cpp#2017

Comment 19

[Mike Kaply [:mkaply]]

so looking into this more, the hang was because of the failed ldap query in my testing.

I ran some tests and the call to processLDAPValues should be working (and is working in my faked tests).

prefcalls.js is able to call function in firefox.cfg with no problem. You can test this yourself by adding a new function in a CFG file and adding a new API to prefcalls.js

And FYI, all this can be tested by modifying dist/bin/defaults/autoconfig/prefcalls.js directly. No need to do a build or anything.

So what I need is a test LDAP environment so I can debug what is actually happening.

Comment 20

[Mike Kaply [:mkaply]]

It works if you disable the sandbox

pref("general.config.sandbox_enabled", false);

I'm looking into how to fix with sandbox enabled.

Comment 21

[Mike Kaply [:mkaply]]

I just ran some quick checks againt 52/68/78 and processLDAPValues is properly called in all of them.

The problems I'm seeing seem to only happen with local builds. I have a very simple autoconfig:

// 
var ldapHost = "ldap.adams.edu";
var ldapBase = "dc=example,dc=com";
 
var ldapFilter = "uid=" + "bcdavis";

var ldapAttrs = new Array( "cn", "email" );
 
// Define how to process LDAP results before we make the call
function processLDAPValues(queryResults)
{  
    Components.utils.reportError(queryResults);
}
 
// Call upon LDAP for the values in ldapAttrs array
// Uses the previous processLDAPValues()
getLDAPAttributes( ldapHost, ldapBase, ldapFilter, ldapAttrs.join(",") );

And while the LDAP connection fails, it properly called processLDAPValues.

I can't move any further on this unless I'm connected with someone experiencing this problem.

Everything I've tested works.

Comment 22

[yr]

Hi, as the reporter of the issue I could test your fix, if you give me instructions how to apply it to an installation (preferably v78).

regards

Comment 23

[Mike Kaply [:mkaply]]

Hi, as the reporter of the issue I could test your fix, if you give me instructions how to apply it to an installation (preferably v78).

What's confusing me here is that in my testing, I don't get this error (and I tested 78).

So what I'd like to know is if this is still a problem.

I did a simple config:

pref("general.config.obscure_value", 0);
pref("general.config.filename", "firefox.cfg");
pref("general.config.sandbox_enabled", false);

//
var ldapHost = "ldap.adams.edu";
var ldapBase = "dc=example,dc=com";

var ldapFilter = "uid=" + "bcdavis";

var ldapAttrs = new Array( "cn", "email" );

// Define how to process LDAP results before we make the call
function processLDAPValues(queryResults)
{
    Components.utils.reportError(queryResults);
}

// Call upon LDAP for the values in ldapAttrs array
// Uses the previous processLDAPValues()
getLDAPAttributes( ldapHost, ldapBase, ldapFilter, ldapAttrs.join(",") );

In particular, does adding this line:

pref("general.config.sandbox_enabled", false);

to your autoconfig.js file make things work.

Comment 24

[yr]

I just tried with the last version (78.3.3, x64), the issue persists.
Sandbox is disabled in autoconfig.js
Here my processLDAPValues function:

function processLDAPValues(queryResults){
if( queryResults ) {
// Build the userInfo object for later use
for( var attr in ldapAttrs ) {
userInfo[ ldapAttrs[attr] ] = getLDAPValue( queryResults, ldapAttrs[attr] );
}
} else {
throw( "No LDAP results" );
}
}

Behaviour: at Thunderbird start: I first get the thrown "No LDAP results" message, then autoconfig errors because of missing values.
Using wireshark I can see that the requested values are returned from our LDAP server.

I also tried using your function but doesn't work either

regards

Comment 25

[Mike Kaply [:mkaply]]

So we're definitely getting farther because previously processLDAPValues wasn't defined. Looking into why no values were passed to processLDAPValues

Comment 26

[Mike Kaply [:mkaply]]

Attachment: Bug 1592922 - Account for possibility of sandbox when calling processLDAPValues. r?kmag!

Comment 27

[Mike Kaply [:mkaply]]

So the patch I put up does cause us to call processLDAPValues properly in some cases, but ldapquery.getQueryResults is returning no results.

As with yr, I verified that Wireshark shows we are making the LDAP connection and getting the results.

But here:

https://dxr.mozilla.org/comm-release/rev/6ff411885e6f02b6a5b41a64606ee38802a2231b/ldap/xpcom/src/nsLDAPSyncQuery.cpp#350

we are getting no results.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6ba460ddb06
Account for possibility of sandbox when calling processLDAPValues. r=kmag

Comment 29

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e6ba460ddb06

Comment 30

[Thomas D. (:thomas8)]

(In reply to Mike Kaply [:mkaply] from comment #27)

Mike, thank you so much for following up on this after our email conversation!!!

So the patch I put up does cause us to call processLDAPValues properly in some cases, but ldapquery.getQueryResults is returning no results.

I'm not sure if I'm understanding you correctly here, or maybe there's a typo?
Does this mean that even with your patch applied, there are still (other?) cases that fail?

But here:
https://dxr.mozilla.org/comm-release/rev/6ff411885e6f02b6a5b41a64606ee38802a2231b/ldap/xpcom/src/nsLDAPSyncQuery.cpp#350
we are getting no results.

Comment 31

[Mike Kaply [:mkaply]]

I'm not sure if I'm understanding you correctly here, or maybe there's a typo?
Does this mean that even with your patch applied, there are still (other?) cases that fail?

Yes. My patch fixes the autoconfig calling issues, but ldapquery.getQueryResults is returning no results, even though I see the LDAP query via wireshark and it returns results.

My next plan is to completely back out the sandbox fix in a local build to see if it causes this, or if there was some other change that broke LDAP querying in Autoconfig.

LDAP logging isn't working for me and there appear to no LDAP experts around anymore.

Comment 32

[Thomas D. (:thomas8)]

(In reply to Mike Kaply [:mkaply] from comment #31)

I'm not sure if I'm understanding you correctly here, or maybe there's a typo?
Does this mean that even with your patch applied, there are still (other?) cases that fail?

Yes. My patch fixes the autoconfig calling issues

That's a great step forward!

but ldapquery.getQueryResults is returning no results, even though I see the LDAP query via wireshark and it returns results.
My next plan is to completely back out the sandbox fix in a local build to see if it causes this, or if there was some other change that broke LDAP querying in Autoconfig.

Awesome. You rock!

Comment 33

[Mike Kaply [:mkaply]]

So in bug 1662433, they said things worked in 68 with

pref("general.config.sandbox_enabled", false);

but started failing in 78.

I'm even more confused now.

Comment 34

[marinar]

The last version I can make MCD work is in 68.2.2

Comment 35

[Mike Kaply [:mkaply]]

(In reply to marinar from comment #34)

The last version I can make MCD work is in 68.2.2

So you're saying it doesn't work in 68.3? This is starting to sound like a Thunderbird problem...

I'll check locally.

Comment 36

[Mike Kaply [:mkaply]]

You are absolutely correct. So there are two issues here.

1. Needed to flip sandbox pref for things to work (I've fixed that).

2. LDAP Autoconfig stopped working completely between 68.2.2 and 68.3. My bet is this is going to be unrelated to autoconfig...

Comment 37

[Mike Kaply [:mkaply]]

I'm going to mark this bug as fixed.

This bug was about the API being broken which I fixed. Now the problem is that bug 1576364 broke LDAP in Autoconfig.

This will be tracked in bug 1662433.

Where should this fix be uplifted to be in the next Thunderbird release?

Comment 38

[Magnus Melin [:mkmelin]]

It should go be uplifted through beta, then m-c esr78.

Comment 39

[Mike Kaply [:mkaply]]

I wonder if it's worth uplifting if LDAP is still broke because of 1662433

Comment 40

[Mike Kaply [:mkaply]]

Comment on attachment 9182984 [details]
Bug 1592922 - Account for possibility of sandbox when calling processLDAPValues. r?kmag!

Beta/Release Uplift Approval Request

• User impact if declined: Unable to call Autoconfig in Sandbox autoconfig environment.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Use instructions here:

https://bugzilla.mozilla.org/show_bug.cgi?id=1662433#c21

for autoconfig test. We aren't testing that it works entirely, just that there is no error making the call.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Autoconfig only, function is already broken.
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: thunderbird
• User impact if declined: Unable to call Autoconfig in Sandbox autoconfig environment.
• Fix Landed on Version: 84
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Autoconfig only, function is already broken.
• String or UUID changes made by this patch:

Comment 41

[Julien Cristau [:jcristau]]

Comment on attachment 9105527 [details]
Bug 1592922 - Fix typo in autoconfig's prefcalls.js. r=mkaply DONTBUILD

This patch landed a year ago.

Comment 42

[Pascal Chevrel:pascalc]

Mike, Magnus I don't get why this needs an uplift in beta/ESR now. It seems from the recent comments in this bug that this is a longstanding issue, can't this just ride the 84 train or this fix be cherry-picked by Thunderbird if you have an urgent need for it in the next release and also have a fix in time for bug 1662433?

Comment 43

[Magnus Melin [:mkmelin]]

The sooner the better, it's non functional at the moment, so we'd uplift as soon as we can (next beta, and then 78.5).
It's not the end of the world if it rides the 84 train and then into ESR 78.6, but OTOH the patch landed in this bug is a no-op for Firefox since there is no LDAP support in Firefox. So no risk as such.

Comment 44

[Pascal Chevrel:pascalc]

(In reply to Magnus Melin [:mkmelin] from comment #43)

The sooner the better, it's non functional at the moment, so we'd uplift as soon as we can (next beta, and then 78.5).
It's not the end of the world if it rides the 84 train and then into ESR 78.6, but OTOH the patch landed in this bug is a no-op for Firefox since there is no LDAP support in Firefox. So no risk as such.

The old autoconfig doc (https://developer.mozilla.org/en-US/docs/Archive/Misc_top_level/MCD,_Mission_Control_Desktop_AKA_AutoConfig) mentions that the Firefox home page can be set by the user labeledURI LDAP field and the code example given uses the processLDAPValues(), is that no longer possible with today's Firefox?

Comment 45

[Mike Kaply [:mkaply]]

(In reply to Pascal Chevrel:pascalc from comment #44)

(In reply to Magnus Melin [:mkmelin] from comment #43)

The sooner the better, it's non functional at the moment, so we'd uplift as soon as we can (next beta, and then 78.5).
It's not the end of the world if it rides the 84 train and then into ESR 78.6, but OTOH the patch landed in this bug is a no-op for Firefox since there is no LDAP support in Firefox. So no risk as such.

The old autoconfig doc (https://developer.mozilla.org/en-US/docs/Archive/Misc_top_level/MCD,_Mission_Control_Desktop_AKA_AutoConfig) mentions that the Firefox home page can be set by the user labeledURI LDAP field and the code example given uses the processLDAPValues(), is that no longer possible with today's Firefox?

Yeah, that's wrong. LDAP isn't even in the Firefox tree.

It's Thunderbird only.

Comment 46

[Mike Kaply [:mkaply]]

The reason this became urgent is it is blocking major companies from moving to Thunderbird 78. It was a problem before that no one researched why it was happening.

Comment 47

[Wayne Mery (:wsmwk)]

Correct. And because it blocks upgrading, it is by extension a security issue.

Comment 48

[Pascal Chevrel:pascalc]

Comment on attachment 9182984 [details]
Bug 1592922 - Account for possibility of sandbox when calling processLDAPValues. r?kmag!

Thanks for the additional context, taking for beta and ESR as this is not impacting Firefox so this is low risk.

Comment 49

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c0fb19c1e766

Comment 50

[Kris Maglione [:kmag]]

This never affected Firefox. The LDAP interfaces it affects only exist in comm-central.

Comment 51

[Ryan VanderMeulen [:RyanVM]]

It's easier for uplift tracking if we leave the flags as they previously were. Feel free to s/firefox/gecko if it makes it mentally easier to be happy with.

Comment 52

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/160fc20893c7

Comment 53

[Alin Ilea, Desktop Test Engineering]

Hi, is there a way for QA to verify this fix in Firefox or its only Thunderbird related?
Thanks!

Comment 54

[Mike Kaply [:mkaply]]

No, this can only be verified on Thunderbird.

Comment 55

[Brindusa Tot, Desktop QA]

Based on previous comments, removing the qe-verify+ flag.

Comment 56

[Pi]

Hello,
is this bug fixed in the 78.5.1 release ?
Thank you

Comment 57

[Magnus Melin [:mkmelin]]

It is.

Comment 58

[marinar]

Hello
I tried to plug the same autoconfig settings as in 68 to Thunderbird 78.5.1, but it seems it is not even reading config file.
The failover.js was not created. The sandbox is set to false.
Anybody had any success in making MCD working?

Please advise.
Thank you

Comment 59

[yr]

Hi,
The update to version 78.5.1 worked for us. Thank you very much!
We only used the sandbox setting to get a CA Cert distributed via autoconfig. We now use the GPO's for this.
So our sandbox is enabled again. getLDAPAttributes still working

Comment 60

[marinar]

Thunderbird or Firefox?
Can you please send sample autoconfig.js and .cfg

Thank you