1160000 - There are no automated tests for NTLM authentication

Status: RESOLVED FIXED

(Core :: Networking, defect, P3)

Description

[Andrew Bartlett]

We need to have an automated test for NTLM authentication, to ensure that unrelated or deliberate changes do not break this important subsystem, which is not typically used by pre-release testers. 

Such a tool could use Samba's ntlm_auth binary to provide NTLM authentication services on the server, using the --password mode to use a fixed password rather than a real AD domain as the backend.

Comment 1

[Andrew Bartlett]

This is a follow-on from my work developing the NTLMv2 code in bug 423758, which could only be manually tested.

Comment 2

[Tl93xo9We56a]

You think that you help to make the best browser in the world?
It is really so.
Mozilla task is to make better for the people.

But is it possible to make better for people by force, by torture, by humiliation and bullying?
Or some people at the expense of others people?

Once in the USSR, some people believed that the state knows better how many tomatoes and cucumbers need to each particular person.
It turned out to be a mistake.
Would you like to help the officials of the USSR?
We would not want to help such officials.


Now Mozilla has decided that it knows best what and how FireFox extensions the people can use, and what and how the people cannot use.
As the developer of the extension, now I'll have to compile FireFox just to use their extension.

Officials Mozilla actually have not listen to us. It's just torture.
They were warned about all the problems. But in the end did what is not justified on any things.
For example, Mozilla blocked the AMO beta-channels for fully reviewed extensions. With that, then they're going to open them again.
It is not justified absolutely nothing.
Here already as 1.5 years I am saying that the situation with the time of the AMO review is invalid.
However, Mozilla is introducing even more complex and time-consuming procedure review, although Mozilla not the way to maintain the proper level of even this.
People maintain the developing add-ons for free. And get Mozilla all to stop their development.
To humiliate these people. Mozilla make it impossible for even a simple test for the extensions. (FireFox DE is not suitable for this)

This is the openness of FireFox? Internet freedom? Respect for users and developers?

Mozilla officials just don't listen to any arguments and don't even think about how to faithfully fulfil at least their own decisions.

Helping for Mozilla now, you help for the mad officials.
You are harming to people.
You're helping to torture and bullying under extension developers.
You are help to make evil.

I write this only because FireFox is a really good browser. I don't want to use another!
Help me as you can.
Stop you work. Do not help Mozilla to bear evil and turn from a good company at a mad officials like the deceased USSR.

It depends on each. It depends on you.
There are no excuses for condoning of torture.
Bad case cannot be justified by anything. Neither the problems nor a benefit to society.



Copy of the previous mail for the some mozilla members
---------------------------------------------------------------------------------------------------

[e-mail], [e-mail], [e-mail], [e-mail], [e-mail]
Mozilla policies
Hello


Below in this letter you can find information for the following issues:
1. The Mozilla communication policy with other people is offensive and ineffective. It brings to rabies and harms the health (I had a hypertensive crisis).
2. The AMO extensions review policy is well, but its implementation is offensive and unacceptable about prevent of the extensions development.
3. The extensions signing policy is bad, and its implementation make an unacceptable obstacle for the extensions developers. This policy is unprofessional implemented.

In advance I apologize, if it's not interesting. However, the policy of Mozilla does not allow me to make it to the competent specialists. At the same time, Mozilla causes me considerable moral and material harm. I am compelled to appeal to the greatest number of people, so the problem became known.


-------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------

1. Turning to the support specialist of any company, I expect he intends to solve the problem if possible and appropriate. I also expect that if this is not possible, then specialist tell to explain to me why and how can I minimize the damage.
If I developing the FireFox extension, I expect that Mozilla will not interfere with me to the extension development. Because it is profitable for Mozilla.
Also I expect the understanding that I work for free. Which means they have the very limited resources, and it is a personal resources. I spend my life for the extension development.

However, Mozilla is doing the opposite.

For example.
1) for a recent discussion of the new policy of the extension signing, the developer community have been specified in advance all the problems that arose later in its implementation. However, nobody listened to the developers. And issues that have been predicted, there was really.
2) Mozilla is in no way took into account the opinion of the developers, nor the degree of the harm inflicted. Not even bothering to explain what critical reason has forced Mozilla to do so. I don't think the explanation about the increase in security FireFox here may be the true explanation. Because the harm to developers far more than any benefit from the introduction of the new policy (see 3. item).
3) AMO editors constantly demanding for me the requirements that are not described (clearly or generally) in AMO extensions review policy.
Obviously, if AMO required any requirements for me, I should be able to know in advance about the availability of these requirements. Otherwise I can't do them.
For example, if AMO required for me to make the test plan of extension, this requirement should be clearly described. Otherwise I imposed unreasonable demands, as if it was my fault that I did not provide this plan. (And what could be the test plan, if AMO declares that reads all the code of the extension??? How do they read?)
4) Some of the requirements should not be submitted. For example, the requirement to replace the nsIObserverService to sdk/system/events. There is no harm for using nsIObserverService. The developer determines what interfaces it uses. However, AMO editors believe that they can interfere in the creative process of the extension developing.
5) Not everyone is equal before the Mozilla policy. Known extensions are review faster. Simple extension (that is useless) passes review faster then a difficult extensions.
Some additions described violate policy Mozilla. Because they "allowed". However, many others cannot. Where such privileges?

I don't need to explain the reasons for such problems. Just decide them! These reasons are disposable.



There was no mechanism for complaints. So, complaints about inadequate requirements AMO editors to go Jorge Villalobos (must die).
Complaints about the harm of Mozilla's policies go to Jorge Villalobos (must die) too.
Complaints Jorge Villalobos go to Jorge Villalobos (must die) too.
However, he is not going to listen for arguments. Does not intend to amend the policy or to solve problems. He has "no time" for me. He has "thousands of add-ons". Why you need Jorge, if he has no time for anyone?
He is not going to listen. He is not going to solve the problem.
This man serves to pretend only that listening to me.

This will not work. I'm not stupid. I understand, when people listening to me, and when people make me to going.
Why would Mozilla to produce of enemies? You so want to them?


Conclusions:
1) Any action by authorised persons must be posiible to challenged by other authorised persons.
2) Don't make authorised persons who pretend to listen. But do not have the authority, ability and willingness.
3) Problems must have solve, and do not tell how difficult it to solve and their causes.
It are possible to resolving. If there are no people, then you must admit that there are no people. And to make the procedure to simpler.

-------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------

2. The Existing extension review policy does not update the add-on in good time. Updating them takes at least several months. I talking about useful extensions. That is about complex extensions.
1) AMO is unable to support the declared certification (review) policy for extensions.
2) AMO should to cancel the all extensions checks, except for security checks. Check is good, but AMO is unable to hold them.
3) As suggested by another person, if the AMO does not have time to certify the extension within 10 days, it should be published without certification (only for extensions with certificated versions).
4) The developer channel should work with automatic signature in any case (only for extensions with certificated versions). The lack of a developer channel makes it impossible to solve many problems.
5) If 10 days is too low, set a longer period, subject to item 3) .

Extension development impossible without it. Notice, the NoScript extension and the some other extensions obviously gets earlier reviews of other. And the only reason he lives.

-------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------

3.
To the extension signing must check only what you can really check. No statically check by regex for the preferences name. It can be bypassed.
I don't know what caused such a critical change in policy. Why are they so quick that there is no way to keep up with them. But, in any case, protecting some people, no need to kill others people.

I think that if the extension in AMO more than 1 year, then it is quite possible to enable automatic signing the developer channel.
The problem is too acute, to be neglected by developers.
Even I myself will soon not be able to use their own addition, if all of this continues to be so. DE and Nightly not meant for everyday use. But there is no other version, which would take unsigned extensions.

It, including, will affect the quality of testing extensions. Since before the publication of the extension, the developer will now not be able to use it independently in usual daily work. Thus, many errors will not be found by the developer and the users.

-------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------

And again, people who participate in discussions by Mozilla, must be authoritative. Jorge Villalobos must die. He doesn't make decisions. And Jorge  not qualified to evaluate what the developers say. He can't even explain why adopted one or the other.
He can only ban.
And no one person can do this. Or FireFox developers should make the complaint themselves or Mozilla will also, as before, to torment the extensions developers.


STOP MOCK ON THE EXTENSION DEVELOPERS!

Ask for help from everyone. Bullying and torturing under people is unacceptable!

END.

Comment 3

[Gary Lockyer]

Attachment: 1160000.patch

Add some basic NTLM authentication unit tests

Note: The proxy authentication currently fails as the proxy connection is reused 
      on a failed authentication attempt.  This can cause issues on squid proxies.

Comment 4

[Gary Lockyer]

Honza is there a way to detect whether the password UI would have been displayed during a test run.

For the the case when a web server logon is rejected it would be useful to check that the proxy credentials are not prompted for,

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 8815964 [details] [diff] [review]
1160000.patch

f? me

Comment 6

[Gary Lockyer]

Attachment: Update unit tests

Added a unit test to detect bug 486508

Comment 7

[Gary Lockyer]

The current tests use an nsHttpChannel

What is also needed are tests that are higher up the stack to capture UI issues i.e. unnecessary password prompts, page load failures.

To do this need to identify the appropriate API to use for the higher level tests, and find a way to flag when a password or retry dialog was displayed in the tests.

Ideally a connection would be discarded once there is an error, this avoids issues with less compliant server implementations.  The unit tests in the patch don't enforce this at present, however the code can be uncommented when desired.

Comment 8

[Honza Bambas (:mayhemer)]

ni? me on comment 7

Comment 9

[Gary Lockyer]

Comment 7 is a summary of what needs to be done, for when I hopefully get back into this. However if you do have any ideas for the higher level tests that would be useful.

Comment 10

[Honza Bambas (:mayhemer)]

Gary, to answer to comment 7, you may look at [1] and surrounding.  To implement a handler for a request that will do the ntlm exchange, you need to create an .sjs file, see [1.1].  To store a state on the server between calls of the handler use setState and getState, see [1.2] for example.

For simpler (unit js) test you can also have your own implementation of nsIAuthPromptProvider (providing GetAuthPrompt method returning nsIAuthPrompt2 implementation that is then asked for a password via asyncPromptAuth method.) nsIAuthPromptProvider object is then attached to a channel through nsIChannel.notificationCallbacks.  For an example see [2].

to sum how that works:
- channel, when auth challenge is received and a prompt is needed, asks it's "notification callbacks" for nsIAuthPromptProvider interface (via nsIInterfaceRequestor interface)
- it gets an opaque object, a provider
- that has a method, getAuthPrompt, which returns an object implementing nsIAuthPrompt2 interface
- that object provides few methods to raise a prompt, mainly asyncPromptAuth
- if called, the channel expects to get (delayed) calls to onAuthAvailable or onAuthCanceled, see [3] interface, which is implemented by nsHttpChannelAuthProvider C class


one bad thing is that our testing httpd.js server doesn't support keep alive connections :/  we use it for both js unit tests (you provide in the patch) and mochitests (the test I refer at [1]).  but that is something that needs to be fixed elsewhere, not in this bug.


How is it going with verification that the tests catch actual problems?

Thanks so far!


[1] https://dxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/test/test_prompt_async.html
[1.1] https://dxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/test/authenticate.sjs
[1.2] https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/dom/xhr/tests/file_html_in_xhr.sjs#5,11
[2] https://dxr.mozilla.org/mozilla-central/search?q=asyncPromptAuth+path%3Anetwerk%2Ftest%2Funit&redirect=false
[3] https://dxr.mozilla.org/mozilla-central/rev/8103c612b79c2587ea4ca1b0a9f9f82db4b185b8/netwerk/base/nsIAuthPromptCallback.idl#29

Comment 11

[Gary Lockyer]

The last patch appears to detect 486508, and with the data on the nsiAuthPromptProvider it should be possible to to do more.

Comment 12

[Florian Bender]

What's the status here?

Comment 13

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 14

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

Comment 15

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

I'll try to see if we can land this patch as-is (or with minor fixups)

Comment 16

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

Attachment: Bug 1160000 - Add xpcshell-tests for NTLM authentication r=#necko

Comment 17

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

Attachment: Bug 1160000 - Fix up NTLM tests r=#necko

I just got the previously added tests to run properly.
While it would be nice to make them clean and modern I think it's not
worth the additional effort - just increasing the code coverage should
be enough.

Depends on D129007

Comment 18

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

Comment on attachment 8816936 [details] [diff] [review]
Update unit tests

Obsoleted by attachment 9246829 [details]

Comment 19

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

Attachment: Bug 1160000 - Fix netwerk/test/unit/xpcshell.ini lint warnings r=#necko

Depends on D129008

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b168979602ed
Add xpcshell-tests for NTLM authentication r=necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/4568eec2f406
Fix up NTLM tests r=necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/b44e97e2ca07
Fix netwerk/test/unit/xpcshell.ini lint warnings r=necko-reviewers,dragana

Comment 21

[Norisz Fay [:noriszfay]]

Backed out 3 changesets (Bug 1160000) for causing xpcshell failures on test_ntlm_web_auth.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/c3f6b7b6ee72ead8dd052d4e30c617c50dcd90ed

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&revision=b44e97e2ca075247b15f63c1598b890d6d0388fb&selectedTaskRun=QDvvqQfpQGCm5deDxJ3ifg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=355573630&repo=autoland

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2da1648e98dc
Add xpcshell-tests for NTLM authentication r=necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/f3b9b2ca9807
Fix up NTLM tests r=necko-reviewers,dragana
https://hg.mozilla.org/integration/autoland/rev/6a4706efa917
Fix netwerk/test/unit/xpcshell.ini lint warnings r=necko-reviewers,dragana

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2da1648e98dc
https://hg.mozilla.org/mozilla-central/rev/f3b9b2ca9807
https://hg.mozilla.org/mozilla-central/rev/6a4706efa917