Closed Bug 1465465 Opened 2 years ago Closed 2 years ago

Block more ports (427, 548, 6697)

Categories

(Core :: Networking, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: annevk, Assigned: baku)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Attachments

(2 files)

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.
Priority: -- → P3
Whiteboard: [necko-triaged]
Chrome landed a patch for blocking the other two. I suspect that means we can go ahead as well.
Assignee: nobody → amarchesini
Just an update of the port names in order to be in sync with the spec.
Attachment #8982144 - Flags: review?(annevk)
block the remaining ports
Attachment #8982145 - Flags: review?(annevk)
Attachment #8982144 - Flags: review?(ckerschb)
Attachment #8982145 - Flags: review?(ckerschb)
Attachment #8982145 - Attachment description: port2.patch → part 2 - disable AFP and IRC+TLS
Attachment #8982144 - Flags: review?(annevk) → review+
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+.
Attachment #8982145 - Flags: review?(annevk) → review+
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!
Attachment #8982144 - Flags: review?(ckerschb) → review?(valentin.gosu)
Attachment #8982145 - Flags: review?(ckerschb) → review?(valentin.gosu)
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" ?
Attachment #8982144 - Flags: review?(valentin.gosu) → review+
Attachment #8982145 - Flags: review?(valentin.gosu) → review+
Pushed by amarchesini@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/63652c45be83
https://hg.mozilla.org/mozilla-central/rev/a358755643e9
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
What's the story with port 6697? That's the port used by TB's chat component. Will that be blocked now?
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(florian)
Flags: needinfo?(clokep)
Richard, you're a Daily user, is chat still working in today's Daily?
Flags: needinfo?(richard.marti)
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.
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)
Flags: needinfo?(florian)
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.
It's only used for HTTP-like channels: https://searchfox.org/mozilla-central/search?q=NS_CheckPortSafety&redirect=false
Flags: needinfo?(valentin.gosu)
I'm using now today Daily and IRC with port 6697 works for me.
Flags: needinfo?(richard.marti)
Flags: needinfo?(clokep)
You need to log in before you can comment on or make changes to this bug.