1534692 - mock-idp.js does not seem to be working in the webrtc wpt

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, enhancement, P2)

Description

[Byron Campen [:bwc]]

This breaks most of the stuff in the webrtc-identity wpt.

Comment 1

[Byron Campen [:bwc]]

We seem to be hitting this error:

https://searchfox.org/mozilla-central/rev/f1c7ba91fad60bfea184006f3728dd6ac48c8e56/dom/media/IdpSandbox.jsm#72

The status code is 2153394151. I cannot find this or 805A2FE7 (hex encoding) in searchfox.

Comment 2

[Byron Campen [:bwc]]

Verified that this is indeed passed as an nsresult, but is not present in the nsresult enum. It has the same prefix (805A) as other errors related to CSP, SRI, and CMS:

https://searchfox.org/mozilla-central/source/__GENERATED__/xpcom/base/ErrorList.h#356-379

I'm going to guess that the way this mock idp is set up is violating security policy in some way.

@ckerschb, have any pointers here?

Comment 3

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Byron Campen [:bwc] from comment #2)

@ckerschb, have any pointers here?

Puh, not really to be honest. I suppose you have checked the web (and or browser) console? For all sorts of CSP, SRI, etc. security policy violations we usually do a good job in logging to the console.

Comment 4

[Byron Campen [:bwc]]

Nothing about sec policy violations in the console, sadly. I do see stderr logging like this though:

0:14.19 INFO STDERR: 127.0.0.1 - - [20/Mar/2019 12:33:37] code 400, message Bad request version ('\xa6\xe0\xe2\xbd\xba\xf5s\xd0W\xa0\x1d\x96b\xe9\xb2\xbd\xdfx<Q\xe2>S\xaae\xb7#"\xf52(\x00$\x13\x01\x13\x03\x13\x02\xc0+\xc0/\xcc\xa9\xcc\xa8\xc0,\xc00\xc0')
0:14.19 INFO STDERR: 127.0.0.1 - - [20/Mar/2019 12:33:37] "^V^C^A^B^@^A^@^Aü^C^CL:^BÛý;~]<87>I}yÌ<87>É^K"<99>Ä^U<91>^UW<80>;RlÿÓ=ö ¦à⽺õsÐW ^]<96>bé²½ßx<Qâ>Sªe·#"õ2(^@$^S^A^S^C^S^BÀ+À/̨̩À,À0À" 400 -

Looking at the request log, I see the GETs for the mock-idp.js going to https, whereas the test itself is loaded over http...

Comment 5

[Byron Campen [:bwc]]

Any ideas here?

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

https://github.com/web-platform-tests/wpt/issues/10140 seems to speak to this.

Comment 7

[Byron Campen [:bwc]]

Looked, and we've already fixed the problem mentioned in comment 6 quite some time ago.

Comment 8

[Byron Campen [:bwc]]

Changing the name of RTCPeerConnection-peerIdentity.html to RTCPeerConnection-peerIdentity.https.html seems to allow us to get further; now I see "self is not defined 2 mock-idp.js:59:16" in the console, which is another issue that was mentioned in comment 6.

Comment 9

[Byron Campen [:bwc]]

Removing all of the "self" stuff from mock-idp.js gets to some more legitimate looking errors. I'll keep digging.

Comment 10

[Byron Campen [:bwc]]

This line looks wrong:

https://searchfox.org/mozilla-central/rev/4eaf7c637a303e0f7adcf1ae45064b2d4bef9eb0/testing/web-platform/tests/.well-known/idp-proxy/mock-idp.js#144

mock-idp.js doesn't define a "param" property on the identity assertion it generates, but is assuming its presence when validating, which is throwing. It does define an "args" property that contains two of the properties ("contents" and "options") that it wants in the "param" property. Maybe this is just a typo?

Comment 11

[Byron Campen [:bwc]]

(I should note that, for whatever reason, when these functions in mock-idp.js throw, no useful information about the error is making it back to the caller, and this is making it hard to debug)

Comment 12

[Byron Campen [:bwc]]

The more I look at this, the more I am convinced that these webrtc-identity tests are completely broken, and have never worked on any browser.

Comment 13

[Byron Campen [:bwc]]

Attachment: Bug 1534692: (WIP) Trying to get the identity tests working.

Comment 14

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 1: Stop using |global|, and fix a typo ('param' should have been 'args') in mock-idp.js r?jib

Comment 15

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 2: Make RTCPeerConnection-peerIdentity use https, and fix bugs in test. r?jib

Depends on D24786

Comment 16

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 3: Fix bug where PeerConnectionImpl::GetPeerIdentity would always return the empty string. r?mjf

Depends on D24787

Comment 17

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 4: Stop closing the pc when identity validation fails, because that's not what the spec says to do. r?jib

Depends on D24788

Comment 18

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 5: Teach PeerConnectionIdp to cope with SDP with no media sections. r?jib

Depends on D24789

Comment 19

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 6: Simplify this identity verification code. Fixes a bug where sRD would not wait for the verification to complete when the target peer identity was set. r?jib

Depends on D24790

Comment 20

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 7: Make RTCPeerConnection-getIdentityAssertion.sub use https, and fix bugs in the test. r?jib

Depends on D24791

Comment 21

[Byron Campen [:bwc]]

Attachment: Bug 1534692 - Part 8: In setIdentityProvider, fall back to the target peer identity if a peerIdentity is not passed. r?jib

Depends on D24792

Comment 22

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe1c443c2ce407cb5dc6f6094ca1aa304ba2879a

Comment 23

[Byron Campen [:bwc]]

Part 6 should be fixed now.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6adc2a2a4988
Part 1: Stop using |global|, and fix a typo ('param' should have been 'args') in mock-idp.js r=jib
https://hg.mozilla.org/integration/autoland/rev/b631b7d78674
Part 2: Make RTCPeerConnection-peerIdentity use https, and fix bugs in test. r=jib
https://hg.mozilla.org/integration/autoland/rev/bf904e78fbe4
Part 3: Fix bug where PeerConnectionImpl::GetPeerIdentity would always return the empty string. r=mjf
https://hg.mozilla.org/integration/autoland/rev/6b8b0c103a4f
Part 4: Stop closing the pc when identity validation fails, because that's not what the spec says to do. r=jib
https://hg.mozilla.org/integration/autoland/rev/5d233f551bad
Part 5: Teach PeerConnectionIdp to cope with SDP with no media sections. r=jib
https://hg.mozilla.org/integration/autoland/rev/f8c25b34cef3
Part 6: Simplify this identity verification code. Fixes a bug where sRD would not wait for the verification to complete when the target peer identity was set. r=jib
https://hg.mozilla.org/integration/autoland/rev/fc24b3375510
Part 7: Make RTCPeerConnection-getIdentityAssertion.sub use https, and fix bugs in the test. r=jib
https://hg.mozilla.org/integration/autoland/rev/c8cfeb029349
Part 8: In setIdentityProvider, fall back to the target peer identity if a peerIdentity is not passed. r=jib

Comment 25

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/6adc2a2a4988
https://hg.mozilla.org/mozilla-central/rev/b631b7d78674
https://hg.mozilla.org/mozilla-central/rev/bf904e78fbe4
https://hg.mozilla.org/mozilla-central/rev/6b8b0c103a4f
https://hg.mozilla.org/mozilla-central/rev/5d233f551bad
https://hg.mozilla.org/mozilla-central/rev/f8c25b34cef3
https://hg.mozilla.org/mozilla-central/rev/fc24b3375510
https://hg.mozilla.org/mozilla-central/rev/c8cfeb029349

Comment 26

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/16524 for changes under testing/web-platform/tests

Comment 27

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged