972770 - Test failure 'Element.waitForElement(): Element 'ID: errorTitleText' has been found' in /testSecurity/testSSLDisabledErrorPage.js

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P1)

Description

[Andrei Eftimie]

Module:    testDisableSSL
Test:      /testSecurity/testSSLDisabledErrorPage.js    
Failure:   Element.waitForElement(): Element 'ID: errorTitleText' has been found
Branches:  mozilla-release
Platforms: Windows 7
Report:    http://mozmill-release.blargon7.com/#/remote/report/e35f22a68fec3f6d144713f9abb4989e

We only have 1 recorded failure.
This doesn't reproduce (I've run it in the same environment).

The page we are checking is a the Firefox SSL error page that should be triggered when accessing a page through https when the encryption algorithms versions do not match.

I'm not sure what could have happened here.
Before we search for the `errorTitleText` element we issue a waitForPageLoad() call.
It's not clear to me how that call could pass and then failed to retrieve the wanted element.

Either instead of the error page we loaded the mail.mozilla.org page. Which we requested through https. If it did load through https (and our settings for the versions were applied correctly) that would mean a regression.

It could be that the request to https://mail.mozilla.org actually redirected to a non-secure version. (I'm not sure if this can actually happen). In this case we wouldn't have seen the error page at all.

Or maybe we failed to correctly set up the preference for "security.tls.version.min" in which case we would have properly loaded up the requested page via https.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I think we should start to add the URL waitForPageLoad() was waiting for. This we can do beside the requested feature on bug 957973.

Comment 2

[Andrei Eftimie]

(In reply to Henrik Skupin (:whimboo) from comment #1)
> I think we should start to add the URL waitForPageLoad() was waiting for.
That is a good idea!

Comment 3

[Mihaela Velimiroviciu (:mihaelav)]

Failed again today on Win 8

Report: http://mozmill-daily.blargon7.com/#/remote/report/43b0604030afed1afa485902879d30ca

Comment 4

[Andreea Matei [:AndreeaMatei]]

This fails a lot now, on all platforms, even on esr. Let's add a skip patch, Teodor could you please do that?

Comment 5

[Teodor Druta]

Attachment: skiptestssldisablederror.patch

Comment 6

[Teodor Druta]

Attachment: skiptestssldisablederror.patch

Comment 7

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8523842 [details] [diff] [review]
skiptestssldisablederror.patch

Review of attachment 8523842 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, disabled:
http://hg.mozilla.org/qa/mozmill-tests/rev/000a4e7f83bb (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/5ef88b926b71 (aurora)

Comment 8

[Andrei Eftimie]

This is not the same failure :(
This failed once in February.

We already had a fix up in bug 1099123 for the failure we're seeing since Thursday.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Right. This was a networking issue yesterday. So the skip for default and aurora can most likely backed-out again, right?

Comment 10

[Andreea Matei [:AndreeaMatei]]

I can still see this failure even with the fix in bug 1099123. So we'll skip on beta too and Cosmin is looking into this now.
Disabled:
http://hg.mozilla.org/qa/mozmill-tests/rev/9174c2569816 (beta)

Comment 11

[Cosmin Malutan, [:cosmin-malutan]]

What the test does:
1 Disable SSL 3.0, TLS 1.0 and TLS 1.1 for secure connections by forcing the use of TLS 1.2
2 Opens https://mozqa.com to test that setting an unsupported security protocol version returns an error page
3 Checks for an error title(which obviously appears if the certificate is not supported)

The issue is that now https://mozqa.com has an TSL 1.2 certificate, and in setup we allowed only this certificate. Now I don't now why/where the certificate changed, but changing the preferences security.tls.version.min and security.tls.version.max to anything else than 3(which enables TSL 1.2) will make the test work again and test what it suppose to test.
http://hg.mozilla.org/qa/mozmill-tests/file/000a4e7f83bb/firefox/tests/remote/testSecurity/testSSLDisabledErrorPage.js#l30

Right now the change is on server-side(we have different certificate) that's why it fails on all branches at the same time.

Comment 12

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v1.0

So this patch disables the TSL 1.2 and leaves only TSL 3.0 enabled. Since the tested page has a TSL 1.2 certificate we want it disabled so we can test the Error Page.

http://mozmill-crowd.blargon7.com/#/remote/report/a6906174787fc0d722108aa1fac931cd

Comment 13

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v2.0

Per bug 1099123 comment 15:
I have to agree with Andrei it will be better to enforce the 1.2 which is the latest certificate and open and lower certificate version than to enforce an older version (3.0) and to load the latest certificate(1.2) to check that is disabled. 

http://mozmill-crowd.blargon7.com/#/remote/report/a6906174787fc0d722108aa1fad7d842

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Just to re-iterate here to make it clear what is happening. Last weekend we had an update of the Zeus Load Balancer (ZLB) which made all domains to use TLSv1.2. This was fixed by bug 1100236. Now our specific protocol versions work again. But I don't know which protocol was in use before for https://mozqa.com. But that doesn't matter. If we want to test specific protocol versions, we should clearly use the appropriate subdomains as setup in bug 891288.

So the fix here would be to check which protocol version the current test is checking, and as best also updating the test name to make it visible. testDisableSSL is not that helpful here. Later with bug 1086334 we will get specific protocol version tests implemented for the other created sub domains.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8524560 [details] [diff] [review]
patch v2.0

Review of attachment 8524560 [details] [diff] [review]:
-----------------------------------------------------------------

::: firefox/tests/remote/testSecurity/testSSLDisabledErrorPage.js
@@ +9,5 @@
>  var prefs = require("../../../lib/prefs");
>  var tabs = require("../../../lib/tabs");
>  var utils = require("../../../../lib/utils");
>  
> +const TEST_DATA = "https://tlsv1-0.mozqa.com";

Please see the comment inside of setupModule:

// Disable SSL 3.0, TLS 1.0 and TLS 1.1 for secure connections
// by forcing the use of TLS 1.2

So we currently test TLS 1.2, but not 1.0. So the chosen subdomain is not correct, and I don't see how this test could pass right now.

Comment 16

[Cosmin Malutan, [:cosmin-malutan]]

(In reply to Henrik Skupin (:whimboo) from comment #15)
> > +const TEST_DATA = "https://tlsv1-0.mozqa.com";
> 
> Please see the comment inside of setupModule:
> 
> // Disable SSL 3.0, TLS 1.0 and TLS 1.1 for secure connections
> // by forcing the use of TLS 1.2
> 
> So we currently test TLS 1.2, but not 1.0. So the chosen subdomain is not
> correct, and I don't see how this test could pass right now.
We currently test that TLS 1.0 won't work when is disabled, is disabled when we have 1.2 enforced(right now).

What would be needed here as an follow up? Just to rename the test file?

Comment 17

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 8524560 [details] [diff] [review]
patch v2.0

Review of attachment 8524560 [details] [diff] [review]:
-----------------------------------------------------------------

I already pushed before the comments from Henrik, so please followup with the changes for the test name and comment if needed.
All passes now:
http://hg.mozilla.org/qa/mozmill-tests/rev/fc036d4dee12 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/3764b4e42e5b (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/99b73aeb41af (beta)

Release and esr will need separate fix with all included.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Cosmin Malutan from comment #16)
> We currently test that TLS 1.0 won't work when is disabled, is disabled when
> we have 1.2 enforced(right now).
> 
> What would be needed here as an follow up? Just to rename the test file?

Ah I see. Lots of confusion when the test is not described via a comment. So it's fine for now but I would suggest we get requested additions via bug 1086334 implemented soon to make the test easier to read and to cover all protocol versions.

Comment 19

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v2.0 [release]

Here is the patch for release and esr31.
Thanks

Comment 20

[Andreea Matei [:AndreeaMatei]]

http://hg.mozilla.org/qa/mozmill-tests/rev/ae5d2757f347 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/7aa7084a31cf (esr31)

Thanks Cosmin!