1409275 - 64 bit windows firefox uses wrong default gssapi library name "gssapi32" instead of "gssapi64" DLL

Status: RESOLVED FIXED

(Core :: Networking, defect, P1)

Description

[odenbach]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170928210252

Steps to reproduce:

We use MIT Kerberos for authentication. Firefox used to work nearly out of the box, only had to tweak two settings:

network.auth.use-sspi (set to "false")
network.negotiate-auth.trusted-uris (set to "https://")

After the upgrade to version 56 and therefore the switch to the 64 bit build GSSAPI authentication did not work anymore. After searching for a while I found that I had to modify the setting

network.negotiate-auth.gsslib (set to "gssapi64.dll")

to make it work again.

I think the bug is in extensions/auth/nsAuthGSSAPI.cpp:

#ifdef XP_WIN
        char *libName = PR_GetLibraryName(nullptr, "gssapi32");
        if (libName) {
            lib = PR_LoadLibrary("gssapi32");
            PR_FreeLibraryName(libName);

No decision is made there between 32 and 64 bit builds.


Actual results:

GSSAPI authentication did not work because the default library name gssapi64.dll is not set.


Expected results:

Firefox should have looked automatically for gssapi64.dll, just as the 32 bit version automatically looked for gssapi32.dll.

Comment 1

[Pascal Chevrel:pascalc]

Chris, could you have a look at this 64 bits bug reported about our Kerberos integration? Thanks

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

We can release note this for 56, and should fix it for 57.
We could consider a dot release. Chris, what do you think?

Comment 3

[Chris Peterson [:cpeterson]]

@ odenbach: thanks for reporting this bug and including a clear diagnosis! :) In the meantime, you can work around the 64-bit problem by re-installing 32-bit Firefox 56.0.1 ("Windows 32-bit") from https://www.mozilla.org/firefox/all/ . You will not be migrated to 64-bit again after you are running version 56.0.1 or later.

@ Patrick: who is the module owner for GSSAPI and Kerberos authentication?

Comment 4

[Chris Peterson [:cpeterson]]

@ Liz: I agree that we should relnote this bug for 56. The fix would probably be safe to uplift to 57 because the code change will be small and it should only affect users who enable this feature.

Release Note Request (optional, but appreciated)

[Why is this notable]:
This is a regression after migrating 32-bit Firefox users to 64-bit on Windows. I don't know how many users use GSSAPI and would be affected.

[Affects Firefox for Android]:
No

[Suggested wording]:
GSSAPI and Kerberos authentication do not work in 64-bit Windows Firefox. You can work around the 64-bit problem by re-installing 32-bit Firefox 56.0.1 ("Windows 32-bit") from https://www.mozilla.org/firefox/all/ . You will not be migrated to 64-bit Firefox again after you are running version 56.0.1 or later.

[Links (documentation, blog post, etc)]:

Comment 5

[odenbach]

Hi,

I would like to point out that there is no need to work around the bug by installing 32-bit firefox. GSSAPI authentication works perfectly in 64-bit Firefox, the only thing to do is enter "gssapi64.dll" in the network.negotiate-auth.gsslib setting. This is easier and faster than a new install - and leaves you with all the advantages that 64-bit Firefox offers.

I would therefore recommend a different wording for the release notes:

[Suggested wording]:
GSSAPI and Kerberos authentication _do_ work in 64-Bit Windows Firefox. Currently however it requires the advanced setting "network.negotiate-auth.gsslib" to contain the string "gssapi64.dll" because the default value (which is chosen if left empty) is wrong. Future versions of Firefox will contain a correct default value.


Cheers,

Christopher

Comment 6

[Chris Peterson [:cpeterson]]

Thanks for the correction. I tagged my comment as obsolete and suggest we use yours. :)

Comment 7

[Patrick McManus [:mcmanus]]

if its as simple as comment 0 suggests, I bet Honza can fix this up asap and take care of uplifts.

Comment 8

[odenbach]

The workaround is as simple as I said. A real solution however would be to put some #ifdef statements around the call

            lib = PR_LoadLibrary("gssapi32");

so something like

#ifdef _WIN64
            lib = PR_LoadLibrary("gssapi64");
#else
            lib = PR_LoadLibrary("gssapi32");
#endif

(I did not check this myself). If someone wants me to test a version containing this I will happily assist.

Comment 9

[u408661]

Honza's out sick. I'll take care of writing this patch, per Jason's request.

Comment 10

[u408661]

Attachment: Bug 1409275 - Default to gssapi64 on 64-bit windows

Review commit: https://reviewboard.mozilla.org/r/190826/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190826/

Comment 11

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8919882 [details]
Bug 1409275 - Default to gssapi64 on 64-bit windows

https://reviewboard.mozilla.org/r/190826/#review196006

::: extensions/auth/nsAuthGSSAPI.cpp:132
(Diff revision 1)
>          if (libName) {
> +            #ifdef _WIN64
> +            lib = PR_LoadLibrary("gssapi64");
> +            #else
>              lib = PR_LoadLibrary("gssapi32");
> +            #endif

nit: would be cleaner if we defined just one kLibName at the top of the file and avoid duplicating code and multiple ifdefs.

Comment 12

[u408661]

Comment on attachment 8919882 [details]
Bug 1409275 - Default to gssapi64 on 64-bit windows

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190826/diff/1-2/

Comment 13

[u408661]

Comment on attachment 8919882 [details]
Bug 1409275 - Default to gssapi64 on 64-bit windows

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190826/diff/2-3/

Comment 14

[u408661]

@odenbach, could you try a build from https://tools.taskcluster.net/index/artifacts/gecko.v2.try.revision.ec8d7b2ca9e3afc007685a6b81d9c304ed8c9731.firefox/win64-opt (look for the "setup.exe" file, I think, I've never used the taskcluster index before) to verify that this uses the appropriate library? Thanks!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/121933d763b4
Default to gssapi64 on 64-bit windows r=valentin

Comment 16

[odenbach]

(In reply to Nicholas Hurley [:nwgh][:hurley] (also [e-mail]) from comment #14)
> @odenbach, could you try a build from
> https://tools.taskcluster.net/index/artifacts/gecko.v2.try.revision.
> ec8d7b2ca9e3afc007685a6b81d9c304ed8c9731.firefox/win64-opt (look for the
> "setup.exe" file, I think, I've never used the taskcluster index before) to
> verify that this uses the appropriate library? Thanks!

@hurley I just tried the setup.exe from above, but it does not install anything apart from a nearly empty "Nightly" directory. install.log contains:

��Nightly Installation Started: 2017-10-19 11:24:08
-------------------------------------------------------------------------------

-------------------------------------------------------------------------------
Installation Details
-------------------------------------------------------------------------------
  Install Dir: C:\Program Files\Nightly
  Locale     : en-US
  App Version: 58.0a1
  GRE Version: 58.0a1
  OS Name    : Windows 7
  Target CPU : x64

-------------------------------------------------------------------------------
Installing Main Files
-------------------------------------------------------------------------------

-------------------------------------------------------------------------------
DLL Registration
-------------------------------------------------------------------------------
  ** ERROR Registering: C:\Program Files\Nightly\AccessibleMarshal.dll **
  ** ERROR Registering: C:\Program Files\Nightly\AccessibleHandler.dll **

[...]

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/121933d763b4

Comment 18

[odenbach]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #17)
> https://hg.mozilla.org/mozilla-central/rev/121933d763b4

How can I verify your change? Code looks good to me, I just would like to check a build.

Comment 19

[(not currently active) Ted Mielczarek]

This should make it into the next nightly build that gets built. They happen twice a day now so that will be later today. If you reload the hg.mozilla.org URL, it should have links to the builds once it makes it into a build.

Comment 20

[(not currently active) Ted Mielczarek]

(In reply to Nicholas Hurley [:nwgh][:hurley] (also [e-mail]) from comment #14)
> @odenbach, could you try a build from
> https://tools.taskcluster.net/index/artifacts/gecko.v2.try.revision.
> ec8d7b2ca9e3afc007685a6b81d9c304ed8c9731.firefox/win64-opt (look for the
> "setup.exe" file, I think, I've never used the taskcluster index before) to
> verify that this uses the appropriate library? Thanks!

Almost! You wanted target.installer.exe:
https://index.taskcluster.net/v1/task/gecko.v2.try.revision.ec8d7b2ca9e3afc007685a6b81d9c304ed8c9731.firefox.win64-opt/artifacts/public/build/install/sea/target.installer.exe

or target.zip to get a non-installer package:
https://index.taskcluster.net/v1/task/gecko.v2.try.revision.ec8d7b2ca9e3afc007685a6b81d9c304ed8c9731.firefox.win64-opt/artifacts/public/build/target.zip

odenbach: you can test the try build from one of those links above if you don't want to wait for it to make a nightly build.

(setup.exe is some internal installer bit that gets used for repackaging builds or something)

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes with the proposed wording.

Comment 22

[odenbach]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #20)
> (In reply to Nicholas Hurley [:nwgh][:hurley] (also [e-mail])
> from comment #14)
> > @odenbach, could you try a build from
> > https://tools.taskcluster.net/index/artifacts/gecko.v2.try.revision.
> > ec8d7b2ca9e3afc007685a6b81d9c304ed8c9731.firefox/win64-opt (look for the
> > "setup.exe" file, I think, I've never used the taskcluster index before) to
> > verify that this uses the appropriate library? Thanks!
> 
> Almost! You wanted target.installer.exe:
> https://index.taskcluster.net/v1/task/gecko.v2.try.revision.
> ec8d7b2ca9e3afc007685a6b81d9c304ed8c9731.firefox.win64-opt/artifacts/public/
> build/install/sea/target.installer.exe

Just tried it - and it works! Thanks a lot!

Comment 23

[u408661]

ted - thanks for setting me straight!

odenbach - thanks for verifying this works!

I'll get the uplift requested so we can get this into 57.

Comment 24

[u408661]

Comment on attachment 8919882 [details]
Bug 1409275 - Default to gssapi64 on 64-bit windows

Approval Request Comment
[Feature/Bug causing the regression]: 64-bit windows builds
[User impact if declined]: gssapi auth doesn't work without a workaround
[Is this code covered by automated tests?]: not to my knowledge
[Has the fix been verified in Nightly?]: yes (see a couple comments up)
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: simple change to a lookup the 64-bit library on 64-bit platforms
[String changes made/needed]: none

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8919882 [details]
Bug 1409275 - Default to gssapi64 on 64-bit windows

Must fix, Beta57+

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8dbd91b51670

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Can you request uplift to m-r?  I'm getting a 56 dot release ready now, and think this would make a good ridealong issue to fix, especially as we keep increasing the percentage of users getting 64-bit over the next couple of weeks.

Comment 28

[u408661]

Comment on attachment 8919882 [details]
Bug 1409275 - Default to gssapi64 on 64-bit windows

Approval Request Comment: see comment 24

Comment 29

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8919882 [details]
Bug 1409275 - Default to gssapi64 on 64-bit windows

Taking this as a ridealong for 56.0.2.

Comment 30

[Alexandre Folle de Menezes]

Isn't this needed for ESR too?

Comment 31

[Sylvestre Ledru [:Sylvestre]]

> Isn't this needed for ESR too?
Nope, we are not migrating the ESR users.

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/a8690a840bd8