1465465 - Block more ports (427, 548, 6697)

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P3)

Description

[Anne (:annevk)]

See https://github.com/whatwg/fetch/pull/738 for the change to the Fetch Standard.

Chrome already blocks 6697 and is planning on blocking the other two as well. We can ship 6697 directly, but might want to consider coordinating the others.

https://github.com/web-platform-tests/wpt/pull/11249 updates the tests.

Comment 1

[Anne (:annevk)]

Chrome landed a patch for blocking the other two. I suspect that means we can go ahead as well.

Comment 2

[Andrea Marchesini [:baku]]

Attachment: part 1 - name updates

Just an update of the port names in order to be in sync with the spec.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: part 2 - disable AFP and IRC+TLS

block the remaining ports

Comment 4

[Anne (:annevk)]

Comment on attachment 8982145 [details] [diff] [review]
part 2 - disable AFP and IRC+TLS

Just note I'm not an official reviewer so you probably need to wait for Christoph to say r+.

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8982144 [details] [diff] [review]
part 1 - name updates

I am fine with the changes, but I am not a Necko peer. I would prefer if some necko peer could sign off on that, hence 302 to valentin!

Comment 6

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

Comment on attachment 8982144 [details] [diff] [review]
part 1 - name updates

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

::: netwerk/base/nsIOService.cpp
@@ +164,1 @@
>    0,    // This MUST be zero so that we can populating the array

nit: Could you also fix this line? It bugs me :)
Not sure what it was supposed to be... "so we can stop populating the array" ?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/63652c45be83
Update blocked port names, r=annevk, r=valentin
https://hg.mozilla.org/integration/mozilla-inbound/rev/a358755643e9
Block ports 427, 548 and 6697, r=annevk, r=valentin

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/63652c45be83
https://hg.mozilla.org/mozilla-central/rev/a358755643e9

Comment 9

[Jorg K (CEST = GMT+2)]

What's the story with port 6697? That's the port used by TB's chat component. Will that be blocked now?

Comment 10

[Jorg K (CEST = GMT+2)]

Richard, you're a Daily user, is chat still working in today's Daily?

Comment 11

[Anne (:annevk)]

If that uses irc(s) URLs I think it should be okay (we safelist more schemes for those type of URLs). Rationale can be found here: https://bugs.chromium.org/p/chromium/issues/detail?id=676951.

Comment 12

[Florian Quèze [:florian]]

Is this only affecting the Fetch API? We use nsIRoutedSocketTransportService in the Thunderbird chat code (ie. https://searchfox.org/comm-central/rev/88224f63cb428c672929097abd8635138d9fc10f/chat/modules/socket.jsm#588-594)

Comment 13

[Anne (:annevk)]

No, it also affects, e.g., <img> (and any other web platform API). I'm not sure if it would restrict a dedicated socket API, however.

Comment 14

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

It's only used for HTTP-like channels: https://searchfox.org/mozilla-central/search?q=NS_CheckPortSafety&redirect=false

Comment 15

[Richard Marti (:Paenglab)]

I'm using now today Daily and IRC with port 6697 works for me.