Randomize UDP listening port

RESOLVED FIXED in Firefox 26, Firefox OS v1.2

Status

()

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: willyaranda, Assigned: frsela)

Tracking

({csectype-dos, sec-low, verifyme})

unspecified
mozilla27
x86
Mac OS X
csectype-dos, sec-low, verifyme
Points:
---

Firefox Tracking Flags

(blocking-b2g:koi+, firefox25 wontfix, firefox26 fixed, firefox27 fixed, firefox-esr17 unaffected, firefox-esr24 unaffected, b2g18 wontfix, b2g-v1.2 fixed)

Details

(Whiteboard: [adv-main26-])

Attachments

(1 attachment, 2 obsolete attachments)

We have identified an attack regarding UDP and our current implementation (that will be fixed in the near future), but this will help on mitigating that.

I would suggest changing the fixed UDP listening port (2442, specified on the services.push.udp.port) to a random one (automatically given by the OS).

This is easy in the init() method of the nsIUDPServerSocket: http://mxr.mozilla.org/mozilla-b2g18/source/netwerk/base/public/nsIUDPServerSocket.idl#24

Nikhil, Doug: thoughts?

Comment 1

5 years ago
marking security sensitive.

Guillermo, details please.
Group: core-security
Sounds do-able. How would the server receive the randomly selected port number though?
I'm going to be out till Monday, will take a look at this then.
Hey guys,

We rely on UDP on the data the UA is sending to us. So if the UA says that it has a IP: 1.1.1.1, we accept that. Since all of the UDP requests come from a private network and our deployment is on the public Internet, the handset says that "I have the 1.1.1.1 IP" but the endpoint IP is one of the IPs that the carrier NAT has, like 2.2.2.2.

So the attack follows as this:

1) Device1 has UAID1, and says that its IP is 1.1.1.1 (this is a right user).
--> The server checks if it can wakeup the device based on the IP-MCC-MNC and reports back a UDP connector.
2) Device1 register an application, so he receives a nice URL for pushing.
3) Device1 is disconnected with the promise of being woken up.

3) Device2 has a UAID2, and says that its IP is 1.1.1.1 (this is a malicious user). This petition could come from the internet.
--> The server checks again the IP-MCC-MNC from the hello message and reports back a UDP connector.
4) Device2 register an application.
5) Device2 is disconnected with the promise of woken up.

So right now, Device2 is on the internet (so it can't be woken up) but with a correct URL that will wake up a device inside the carrier network.

When the attacker sends any notification, we end up flooding with UDP packets the network, wakening up all devices that, each of them, connects to the push server.

The problem comes here is that the attacker need to register all the IPs from the private network (maybe 16M registrations, but that's is not too much time). The thing is that we have a fixed port (2442), so it only need to register those 16M IPs.

Mitigation: if we could register a random port for UDP the attacker would need to register 16.000.000 * (aprox) 64.000 ports different devices to make the device to wakeup. 

----
Our server solution will be based on the origin IP we got from the connection. It can say that it is on a private network but if the origin IP is not from our source of trust (official endpoints from carriers) we will not give them any UDP mechanism, but instead a Websocket.
As a security problem I have to rate this sec-low since it's mostly a DoS-type attack, but please don't interpret that as meaning it's unimportant!
Keywords: csec-dos, sec-low
Beatriz, nominating koi? since I think we should need this in 1.2.
blocking-b2g: --- → koi?
Flags: needinfo?(brg)
(In reply to Guillermo López (:willyaranda) from comment #5)
> Beatriz, nominating koi? since I think we should need this in 1.2.
Thanks for the analysis. Yes please let fix this in the near future! please set the koi+
Flags: needinfo?(brg)
(In reply to Beatriz Rodríguez [:brg] from comment #6)
> (In reply to Guillermo López (:willyaranda) from comment #5)
> > Beatriz, nominating koi? since I think we should need this in 1.2.
> Thanks for the analysis. Yes please let fix this in the near future! please
> set the koi+

That's not an argument to block. Something that is wanted to be fixed is something we should get on the radar of developers. But in terms of blocking, this does not meet the criteria to block - it's not a regression from the past release that already shipped with this bug, which makes this a non-blocker for release.
(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Beatriz Rodríguez [:brg] from comment #6)
> > (In reply to Guillermo López (:willyaranda) from comment #5)
> > > Beatriz, nominating koi? since I think we should need this in 1.2.
> > Thanks for the analysis. Yes please let fix this in the near future! please
> > set the koi+
> 
> That's not an argument to block. Something that is wanted to be fixed is
> something we should get on the radar of developers. But in terms of
> blocking, this does not meet the criteria to block - it's not a regression
> from the past release that already shipped with this bug, which makes this a
> non-blocker for release.

Sorry, I think I was not clear enough. We need this to be fixed in v1.2, so please Doug keep it in your radar as Jason suggested. Once the branch of v1.2 will be created it will be very unfortunate if we forgot something important. Thats why I wanted to set the koi+ flag. I know this is not a blocker. Only bug fixing! Thanks Doug!

Updated

5 years ago
blocking-b2g: koi? → koi+

Comment 9

5 years ago
per antonio
Assignee: nobody → frsela
Created attachment 806611 [details] [diff] [review]
WIP - Opening a random UDP port

A WIP patch.

It opens a UDP port with a random port

--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--

Proto Recv-Q Send-Q Local Address          Foreign Address        State
 tcp       0      0 0.0.0.0:2828           0.0.0.0:*              LISTEN
 tcp       0      0 127.0.0.1:5037         0.0.0.0:*              LISTEN
 tcp       0      0 0.0.0.0:666            0.0.0.0:*              LISTEN
 udp       0      0 0.0.0.0:36027          0.0.0.0:*              CLOSE


--8<----8<----8<----8<----8<----8<----8<----8<----8<----8<--

I/Gecko   ( 2020): -*- PushService.jsm: init()
I/Gecko   ( 2020): -*- PushService.jsm: PushDB()
I/Gecko   ( 2020): -*- PushService.jsm: getAllChannelIDs()
I/Gecko   ( 2020): -*- PushService.jsm: beginWSSetup()
I/Gecko   ( 2020): -*- PushService.jsm: Network is offline.
I/Gecko   ( 2020): -*- PushService.jsm: shutdownWS()
I/Gecko   ( 2020): -*- PushService.jsm: getAllChannelIDs()
I/Gecko   ( 2020): -*- PushService.jsm: beginWSSetup()
I/Gecko   ( 2020): -*- PushService.jsm: serverURL: wss://uapush-nv.srv.openwebdevice.com/
I/Gecko   ( 2020): -*- PushService.jsm: wsOnStart()
I/Gecko   ( 2020): -*- PushService.jsm: listenForUDPWakeup()
I/Gecko   ( 2020): -*- PushService.jsm: getNetworkState()
I/Gecko   ( 2020): -*- PushService.jsm: Running on mobile data
I/Gecko   ( 2020): -*- PushService.jsm: listenForUDPWakeup listening on 36027
I/Gecko   ( 2020): -*- PushService.jsm: getNetworkState()
I/Gecko   ( 2020): -*- PushService.jsm: Running on mobile data
I/Gecko   ( 2020): -*- PushService.jsm: getAllChannelIDs()
I/Gecko   ( 2020): -*- PushService.jsm: Sending message: {"messageType":"hello","uaid":"f83b3092-dbf0-4c0a-af2e-d9ba51bcbfe1@544e6dc3efb426f264387985847a4295ad4e6745","wakeup_hostport":{"ip":"10.172.220.237","port":36027},"mobilenetwork":{"mcc":"214","mnc":"07"},"channelIDs":["332ed0e6-4839-46fb-8c3f-8a02778eed52"]}
I/Gecko   ( 2020): -*- PushService.jsm: wsOnMessageAvailable() {"messageType":"hello","uaid":"f83b3092-dbf0-4c0a-af2e-d9ba51bcbfe1@544e6dc3efb426f264387985847a4295ad4e6745","status":201}
I/Gecko   ( 2020): -*- PushService.jsm: handleHelloReply()
I/Gecko   ( 2020): -*- PushService.jsm: New _UAID: f83b3092-dbf0-4c0a-af2e-d9ba51bcbfe1@544e6dc3efb426f264387985847a4295ad4e6745
I/Gecko   ( 2020): -*- PushService.jsm: _processNextRequestInQueue()
I/Gecko   ( 2020): -*- PushService.jsm: Request queue empty
I/Gecko   ( 2020): -*- PushService.jsm: Set alarm 1800000 in the future 17
I/Gecko   ( 2020): -*- PushService.jsm: wsOnServerClose() 4774 UDP Wakeup
I/Gecko   ( 2020): -*- PushService.jsm: Server closed with promise to wake up
I/Gecko   ( 2020): -*- PushService.jsm: wsOnStop()
I/Gecko   ( 2020): -*- PushService.jsm: shutdownWS()
I/Gecko   ( 2020): -*- PushService.jsm: Stopped existing alarm 17
Attachment #806611 - Flags: feedback?(willyaranda)
Attachment #806611 - Flags: feedback?(nsm.nikhil)
Attachment #806611 - Flags: feedback?(doug.turner)
Comment on attachment 806611 [details] [diff] [review]
WIP - Opening a random UDP port

And +1 for the WS send message debug.
Attachment #806611 - Flags: feedback?(willyaranda) → feedback+
Comment on attachment 806611 [details] [diff] [review]
WIP - Opening a random UDP port

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

Two things apart from the comments.

1)Why the new layer of sendWSMessage(). Is there a use of _ws.sendMsg() where we don't have state checks in place to ensure the socket is defined? We should fix those instead. The PushService does need some state machine cleanup.

2) You don't seem to be sending the actual UDP port to the push server with the {ip, mcc, mnc} pair (so {ip, port, mcc, mnc}). Without this the push server/UDP adaptor won't be able to wake up the device either.

::: b2g/app/b2g.js
@@ +420,4 @@
>  // enable udp wakeup support
>  pref("services.push.udp.wakeupEnabled", true);
>  // port on which UDP server socket is bound
> +pref("services.push.udp.port", -1);   // -1 => System assigned by default

Just get rid of the pref entirely and always randomize.

::: dom/push/src/PushService.jsm
@@ +425,5 @@
>     */
>    _willBeWokenUpByUDP: false,
>  
> +  /**
> +   * Sends a message to the PNS through an open websocket

s/PNS/Push Server

@@ +1309,5 @@
>  
> +    // Openning UDP. If _udpPort is -1 the system will assign a free one
> +    // _listenForUDPWakeup will update _udpPort with the final one
> +    this._listenForUDPWakeup();
> +

The problem with starting up a UDP socket when the PushService starts is that it is now always open even when not required, and open on all devices. This seems like opening up another attack vector. 

You either want to immediately shutdown this socket or manually choose a random port (using Math.random() or similar) and then checking if it is already being used by another socket or not. Afaik with SO_REUSEADDR or similar it may work even if another service is using it. The catch in that case is that if that other service receives data our UDP socket will also receive it. We'll need to use a magic number or some other data that is sent on the socket that uniquely identifies that it was sent by the push protocol.
Attachment #806611 - Flags: feedback?(nsm.nikhil)
Attachment #806611 - Flags: feedback?(doug.turner)
Attachment #806611 - Flags: feedback-
(In reply to Nikhil Marathe [:nsm] from comment #12)
> >  
> > +    // Openning UDP. If _udpPort is -1 the system will assign a free one
> > +    // _listenForUDPWakeup will update _udpPort with the final one
> > +    this._listenForUDPWakeup();
> > +
> 
> The problem with starting up a UDP socket when the PushService starts is
> that it is now always open even when not required, and open on all devices.
> This seems like opening up another attack vector. 
> 
> You either want to immediately shutdown this socket or manually choose a
> random port (using Math.random() or similar) and then checking if it is
> already being used by another socket or not. Afaik with SO_REUSEADDR or
> similar it may work even if another service is using it. The catch in that
> case is that if that other service receives data our UDP socket will also
> receive it. We'll need to use a magic number or some other data that is sent
> on the socket that uniquely identifies that it was sent by the push protocol.

Antonio, ni? on this.
Status: NEW → ASSIGNED
Flags: needinfo?(amac)
(In reply to Guillermo López (:willyaranda) from comment #13)
> (In reply to Nikhil Marathe [:nsm] from comment #12)
> > >  
> > > +    // Openning UDP. If _udpPort is -1 the system will assign a free one
> > > +    // _listenForUDPWakeup will update _udpPort with the final one
> > > +    this._listenForUDPWakeup();
> > > +
> > 
> > The problem with starting up a UDP socket when the PushService starts is
> > that it is now always open even when not required, and open on all devices.
> > This seems like opening up another attack vector. 
> > 
> Antonio, ni? on this.

I assume the NI is for this part. The UDP port part of the protocol was designed specifically to minimize any kind of attack (other than awaking the phone). The data that arrives to the port shouldn't be used for anything (since we don't send any data through that). As such, I don't see a problem in having the port open while we're on 3G, and closing it while on Wi-Fi. TBH, I don't see much problem on leaving it open on wifi either. As I said, we don't do anything with the data, and we don't even acknowledge it through that port.
Flags: needinfo?(amac)
well when some data arrives on the UDP port, we try to connect to the push server. In addition, I don't know if keeping a UDP socket open uses any battery/CPU resources like keeping a WebSocket alive does.
(In reply to Nikhil Marathe [:nsm] from comment #15)
> well when some data arrives on the UDP port, we try to connect to the push
> server. 
Yeah, that was the awaking the phone attack, and that's why it's being randomized here. Anyway we would only try to connect to the push server if we had any channel registered, won't we? And in that case we should be already connected. And if we don't have any channel we don't open the port. So the added risk if we open the UDP port beforehand goes just from the phone boot up time till the connection to the server time. After that we should be on the same situation.

The way I think this should work is:

PushService starts. We check if there's any channel. If there isn't any, we don't have anything to do. If we have any channel then:
 * We get a UDP port
 * We connect to the server
 * If the server tells us to disconnect then just close the websocket (we already have the port)
   else close the UDP port

And anytime we get a network change event (where we reconnect to the server) we repeat the procedure. That way the only difference with what we have now is that the socket is opened *always* before connecting but stays opened only if it's actually needed.


> In addition, I don't know if keeping a UDP socket open uses any
> battery/CPU resources like keeping a WebSocket alive does.

I sure hope not :) Otherwise all of this would be a moot point!
(In reply to Nikhil Marathe [:nsm] from comment #12)
> 1)Why the new layer of sendWSMessage(). Is there a use of _ws.sendMsg()
> where we don't have state checks in place to ensure the socket is defined?
> We should fix those instead. The PushService does need some state machine
> cleanup.
> 

Short answer: Have a chance to trace sent packages.

Long answer: If we allow to enable/disable debug, we think it's very interesting to also trace sent packages to the server, so we needed to create a new method.

Since we've a new method now, we also moved the object to string conversion (JSON.stringify) to the method so we have a clearer code along all the file ;)

> 2) You don't seem to be sending the actual UDP port to the push server with
> the {ip, mcc, mnc} pair (so {ip, port, mcc, mnc}). Without this the push
> server/UDP adaptor won't be able to wake up the device either.
> 

I don't understand. We're sending the opened port to the PNS:

I/Gecko   ( 2020): -*- PushService.jsm: listenForUDPWakeup listening on 36027

I/Gecko   ( 2020): -*- PushService.jsm: Sending message: {"messageType":"hello","uaid":"f83b3092-dbf0-4c0a-af2e-d9ba51bcbfe1@544e6dc3efb426f264387985847a4295ad4e6745","wakeup_hostport":{"ip":"10.172.220.237","port":36027},"mobilenetwork":{"mcc":"214","mnc":"07"},"channelIDs":["332ed0e6-4839-46fb-8c3f-8a02778eed52"]}

Send the port to the server is really important since it's needed to wakeup it ;)

> ::: b2g/app/b2g.js
> @@ +420,4 @@
> >  // enable udp wakeup support
> >  pref("services.push.udp.wakeupEnabled", true);
> >  // port on which UDP server socket is bound
> > +pref("services.push.udp.port", -1);   // -1 => System assigned by default
> 
> Just get rid of the pref entirely and always randomize.
> 

Agree, I left it couse I thought you'll prefer to allow a way to fix it in some cases, but anyway allways random it is ok to me too :)

> ::: dom/push/src/PushService.jsm
> @@ +425,5 @@
> >     */
> >    _willBeWokenUpByUDP: false,
> >  
> > +  /**
> > +   * Sends a message to the PNS through an open websocket
> 
> s/PNS/Push Server
> 

Agree :) - I'm used to always use this abbreviation but agree this is not ok for the code comments ;)

> @@ +1309,5 @@
> >  
> > +    // Openning UDP. If _udpPort is -1 the system will assign a free one
> > +    // _listenForUDPWakeup will update _udpPort with the final one
> > +    this._listenForUDPWakeup();
> > +
> 
> The problem with starting up a UDP socket when the PushService starts is
> that it is now always open even when not required, and open on all devices.
> This seems like opening up another attack vector. 
> 

As Antonio said, I don't see any issue with it:

1.- We're on WiFi so websocket is not closed. No issue since the UDP port is not opened couse _getNetworkState will answer with "No IP" and _listenForUDPWakeup will return without opening the UDP port.

2.- If we're in 3G with an enabled wakeup host, the UDP port SHALL be always opened so we're opening it ;)

3.- If the 3G network is not enabled, you've the UDP opened but the Websocket is not closed. If you receive a UDP datagram, we call _beginWSSetup but this method will exit if the current state is not valid (this._currentState != STATE_SHUT_DOWN) so no collateral effects.

Also if the 3G network is not wakeup enable, the handset SHOULD be configured to disable UDP wakeups ;)

4.- With TEF PNS we've a change to close the UDP port if the server don't have a wakeup host in your nerwork since we've to status responses to the HELLO package (200 no UDP, 201 UDP) but as discussed in Madrid some months ago, you suggested to only have one status response and only if the socket is closed the client knowns if the UDP should be maintained open or not (based on the disconection status).

If you agree to recover the double status codes, if the client receives a 200 to the HELLO command, we can close UDP port without any collateral effect ;)

> You either want to immediately shutdown this socket or manually choose a
> random port (using Math.random() or similar) and then checking if it is
> already being used by another socket or not. Afaik with SO_REUSEADDR or
> similar it may work even if another service is using it. The catch in that
> case is that if that other service receives data our UDP socket will also
> receive it. We'll need to use a magic number or some other data that is sent
> on the socket that uniquely identifies that it was sent by the push protocol.

Select a RANDOM port is not a valid solution since can be used when we'll try to open it in the future and the port we sent to the server is not a valid one and the server can't wakeup us.
(In reply to Fernando R. Sela (no CC, needinfo please) [:frsela] from comment #17)
> (In reply to Nikhil Marathe [:nsm] from comment #12)
> > 1)Why the new layer of sendWSMessage(). Is there a use of _ws.sendMsg()
> > where we don't have state checks in place to ensure the socket is defined?
> > We should fix those instead. The PushService does need some state machine
> > cleanup.
> > 
> 
> Short answer: Have a chance to trace sent packages.
> 
> Long answer: If we allow to enable/disable debug, we think it's very
> interesting to also trace sent packages to the server, so we needed to
> create a new method.
> 
> Since we've a new method now, we also moved the object to string conversion
> (JSON.stringify) to the method so we have a clearer code along all the file
> ;)

Agreed, please call it '_wsSendMessage()'

> 
> > 2) You don't seem to be sending the actual UDP port to the push server with
> > the {ip, mcc, mnc} pair (so {ip, port, mcc, mnc}). Without this the push
> > server/UDP adaptor won't be able to wake up the device either.
> > 
> 
> I don't understand. We're sending the opened port to the PNS:

Sorry, I haven't read the code in a while, missed the hostport part.

Also, I agree, keeping the UDP socket always open is acceptable.

Please update the patches and ask for review. Thank you.
(In reply to Nikhil Marathe [:nsm] from comment #18)

Thank you Nikhil. I'll update it asap and upload it again.

Thanks again for your feedback ;)
Created attachment 807694 [details] [diff] [review]
WIP V2 - Opening a random UDP port

All commented changes done. I hope I didn't forget anyone ;)
Attachment #806611 - Attachment is obsolete: true
Attachment #806611 - Flags: feedback?(doug.turner)
Attachment #807694 - Flags: feedback?(willyaranda)
Attachment #807694 - Flags: feedback?(nsm.nikhil)
Attachment #807694 - Flags: feedback?(doug.turner)
Attachment #807694 - Flags: feedback?(amac)
Attachment #807694 - Flags: review?(nsm.nikhil)
Attachment #807694 - Flags: feedback?(willyaranda)
Attachment #807694 - Flags: feedback?(nsm.nikhil)
Attachment #807694 - Flags: feedback?(doug.turner)
Attachment #807694 - Flags: feedback?(amac)
Comment on attachment 807694 [details] [diff] [review]
WIP V2 - Opening a random UDP port

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

r=me with fixes.

::: dom/push/src/PushService.jsm
@@ +425,5 @@
>     */
>    _willBeWokenUpByUDP: false,
>  
> +  /**
> +   * Sends a message to the PNS through an open websocket

Nit: Push Server ;)
And add a period at the end.

@@ +429,5 @@
> +   * Sends a message to the PNS through an open websocket
> +   */
> +  _wsSendMessage: function(msg) {
> +    if (!this._ws) {
> +      debug("No WebSocket initialized. We can not send a message");

Nit: Cannot send a message. And remove 'We'

@@ +432,5 @@
> +    if (!this._ws) {
> +      debug("No WebSocket initialized. We can not send a message");
> +      return;
> +    }
> +    if (typeof(msg) != 'string') {

Drop this check and force a specific type. This is an internal method and is called by trusted code. I'd add a comment stating what type msg is supposed to be, and throw if it is not that (I'd prefer msg be passed in as an object.)

@@ +458,4 @@
>  
>      this._requestTimeout = prefs.get("requestTimeout");
>  
> +    this._udpPort = null;   // Opened UDP port

Nit: Don't set this in init() when it isn't required. Top level property lookup on 'this' will just resolve to undefined anyway.

@@ +1307,4 @@
>      // Since we've had a successful connection reset the retry fail count.
>      this._retryFailCount = 0;
>  
> +    // Openning UDP. If _udpPort is -1 the system will assign a free one

Typo Opening.

Nit: Update comment since the -1 behaviour is no longer implemented.

@@ +1309,5 @@
>  
> +    // Openning UDP. If _udpPort is -1 the system will assign a free one
> +    // _listenForUDPWakeup will update _udpPort with the final one
> +    this._listenForUDPWakeup();
> +

Would this be cleaner if _listenForUDPWakeup() returned the port number? Since it is only being called in the same scope where the port is required, we could drop the global _udpPort variable. Please file a follow up bug to fix this if it makes sense.

@@ +1458,2 @@
>      this._udpServer.asyncListen(this);
> +    this._udpPort = this._udpServer.port;

Can you swap this line and the asyncListen() line. It just reassures readers that port is indeed set soon after init. This way it is a bit worrying because 'async' makes it seem like it will be initialized only after the event loop is run.

@@ +1477,4 @@
>     */
>    onStopListening: function(aServ, aStatus) {
>      debug("UDP Server socket was shutdown. Status: " + aStatus);
> +    this._udpPort = null;

undefined
Attachment #807694 - Flags: review?(nsm.nikhil) → review+
(In reply to Nikhil Marathe [:nsm] from comment #21)
> Comment on attachment 807694 [details] [diff] [review]
> WIP V2 - Opening a random UDP port
> 
> Review of attachment 807694 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with fixes.
> 

Thanks ;)

> 
> Would this be cleaner if _listenForUDPWakeup() returned the port number?
> Since it is only being called in the same scope where the port is required,
> we could drop the global _udpPort variable. Please file a follow up bug to
> fix this if it makes sense.
>

Agree with the first part, now _listenForUDPWakeup will return the port number, but we'll need to store it since is used in other places.

We can also use this._udpServer.port but more checks will be needed to assure _udpServer is defined. right?

> @@ +1458,2 @@
> >      this._udpServer.asyncListen(this);
> > +    this._udpPort = this._udpServer.port;
> 
> Can you swap this line and the asyncListen() line. It just reassures readers
> that port is indeed set soon after init. This way it is a bit worrying
> because 'async' makes it seem like it will be initialized only after the
> event loop is run.
> 

Line dropped, now it's returned and assigned in the call. I'll upload the new patch with this nits just now.
Created attachment 809774 [details] [diff] [review]
Opening a random UDP port

Nits addressed.
Attachment #807694 - Attachment is obsolete: true
Attachment #809774 - Flags: checkin?(nsm.nikhil)
Comment on attachment 809774 [details] [diff] [review]
Opening a random UDP port

In the future, you can just use checkin-needed for single patch bugs :)
https://hg.mozilla.org/integration/b2g-inbound/rev/501023f14f41
Attachment #809774 - Flags: checkin?(nsm.nikhil) → checkin+
https://hg.mozilla.org/mozilla-central/rev/501023f14f41
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox27: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
https://hg.mozilla.org/releases/mozilla-aurora/rev/56e145d04df1
status-b2g-v1.2: --- → fixed
status-firefox25: --- → wontfix
status-firefox26: --- → fixed
status-firefox-esr17: --- → unaffected
status-firefox-esr24: --- → unaffected

Updated

5 years ago
Keywords: verifyme

Comment 27

5 years ago
b2g18 WONTFIX due to this being a sec-low and we're likely not to backport it.
status-b2g18: --- → wontfix
Guillermo, may I ask you to verify this is fixed on 26 and 27?  Thanks
To add to Tracy's comment 28 - if you have a test for us, or ways to verify, this can also help QA verify the bug.
Point of clarification - this right now can only be tested on Firefox OS. This isn't related to Desktop Firefox.

The verification would probably require verifying that the UDP Port used for Simple Push in the web socket is randomized - it shouldn't be the same every time the socket is opened. You would probably need to sniff UDP packets outgoing from the phone to check that the port used is different every time the socket opens.
We can test this bug, and I think our QA has test cases for
Whiteboard: [adv-main26-]
Group: core-security
You need to log in before you can comment on or make changes to this bug.