1278767 - [Remote Control] Wrong time for Date.now() at the moment of generating a certificate for remote control service on FxOS TV

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TV, defect)

Description

[C.M.Chang[:chunmin]]

Device: Nexus player

Steps for reproduce
-----------------------------
1. Flash the TV build into nexus player
2. reboot the TV
3. call Date.now()


Expected result
-----------------------------
The time converted from Date.now() should be the current time(e.g., 2016-6-8 14:46:30).

Output result
-----------------------------
The time converted from Date.now() is 2000-xxx-xxxx
The year is apparently wrong.

Comment 1

[C.M.Chang[:chunmin]]

Hi Eric,
I open a bug for tracking the time issue causing the certificate error(bug 1276871). Please correct me if I say anything wrong of the reproducing steps. 

Hi Josh,
It may have potential risk if there is anything using 'time' as a parameter to generate or process something for further usage on TV. However, we already have a workaround for bug 1276871, so this issue currently causes no harm for remote control service.

Comment 2

[C.M.Chang[:chunmin]]

From my test, if we delay startup of remote-control service for 30 sec, than the time is write and bug 1284191 won't happen.

Comment 3

[C.M.Chang[:chunmin]]

Attachment: WIP: Delay loading RemoteControlService

Comment 5

[C.M.Chang[:chunmin]]

WIP: https://github.com/ChunMinChang/gecko-dev/commit/8581f3663c0ac9b7d71b6ef6169813240e13b641

If we delay to launch the remote-control service after the network is connected, then sometimes the "_mozilla_papi._tcp" broadcast messages for presentation-api will disappear. I am tracing why this happen.

Comment 6

[C.M.Chang[:chunmin]]

(In reply to Chun-Min Chang[:chunmin] from comment #5)
> WIP:
> https://github.com/ChunMinChang/gecko-dev/commit/
> 8581f3663c0ac9b7d71b6ef6169813240e13b641
> 
> If we delay to launch the remote-control service after the network is
> connected, then sometimes the "_mozilla_papi._tcp" broadcast messages for
> presentation-api will disappear. I am tracing why this happen.

If I add some logs in MulticastDNSDeviceProvider.cpp / nsDNSServiceDiscovery.cpp / MDNSResponderOperator.cpp to trace, then it won't happen again. If I add an extra delay before launching RecomteControlService.jsm, then this won't happen.
Thus, I guess it's some kind of race-condition. AFAIK, all main processing is on main thread, only some works will run on socket thread. However, I could reproduce it when I use GDB. More worse, using |p [variable]| in GDB can't work because FxOS on Nexusplayer can't disable optimizer by |export B2G_NOOPT=1|. I try using |volatile|, but it doesn't work.

The GDB server in FxOS on Nexusplayer can't work normally. It will get stuck all the time.
Check the process by |$ adb shell b2g-ps|

$ adb shell b2g-ps
APPLICATION    SEC USER     PID   PPID  VSIZE  RSS     WCHAN    PC        NAME
b2g              0 root      170   1     261524 115236 ffffffff f3484be7 t /system/b2g/b2g
(Nuwa)           0 root      366   170   134540 22732 ffffffff f736b522 S /system/b2g/b2g

If the b2g process shows |t| on PC, then it means b2g get stuck
We can use |kill -2| to interrupt the GDB server.
$ adb shell kill -2 170

Then your GDB console will show
---------------------------------------------------------------------------
Program received signal SIGINT, Interrupt.
__epoll_pwait () at bionic/libc/arch-x86/syscalls/__epoll_pwait.S:32
32	    cmpl    $-MAX_ERRNO, %eax
(gdb) 
Continuing.
---------------------------------------------------------------------------
Then you can press 'c' to continue it.

If it still gets stuck. Try again until it run.

Comment 7

[C.M.Chang[:chunmin]]

workaround
1) Add extra delay after receiving the signal that the network is active, 
2) Use nsIObserver to notify that the presentation api is started. Then remote_control.js should launch the RemoteControlService.jsm(_remotecontrol._tcp) after receiving this signal.

Comment 8

[C.M.Chang[:chunmin]]

Attachment: WIP: Add a service queue for mDNS

Comment 9

[C.M.Chang[:chunmin]]

Comment on attachment 8771244 [details]
WIP: Add a service queue for mDNS

Hi SC,
Could you give me some suggestions about this issue?

Comment 10

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8768755 [details]
WIP: Delay loading RemoteControlService

A better and more general solution is to monitor system time change event and renew certificate. On Firefox OS we can use https://developer.mozilla.org/en-US/docs/Mozilla/B2G_OS/API/Window/onmoztimechange.

Comment 11

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8771244 [details]
WIP: Add a service queue for mDNS

No sure if this patch really works since we haven't understand the root cause. Need @xeonchen's input on how to further debugging on this issue since he know better about the current MDNS service implementation than me.

Comment 12

[Gary Chen [:xeonchen]]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #11)
> Comment on attachment 8771244 [details]
> WIP: Add a service queue for mDNS
> 
> No sure if this patch really works since we haven't understand the root
> cause. Need @xeonchen's input on how to further debugging on this issue
> since he know better about the current MDNS service implementation than me.

If you want debug the b2g process, the only way I know is to attach it rather than running it directly.
Did you try |./run-gdb.sh attach|?

I'm not sure I understand the reason of the patch, when will the service queue contain more than one service?

BTW, not sure if it helps, but Bug 1187813 describes a scenario that NTP doesn't work correctly on B2G.

Comment 13

[C.M.Chang[:chunmin]]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #10)
> Comment on attachment 8768755 [details]
> WIP: Delay loading RemoteControlService
> 
> A better and more general solution is to monitor system time change event
> and renew certificate. On Firefox OS we can use
> https://developer.mozilla.org/en-US/docs/Mozilla/B2G_OS/API/Window/
> onmoztimechange.
Hmm...it indeed can avoid this happening when starting up, but we still suffer from same issue every time when network is turned on again. We will re-start both Presentation API and remote-control service when the network status is back to alive, so we might need another approach here.

Comment 14

[C.M.Chang[:chunmin]]

(In reply to Gary Chen [:xeonchen] from comment #12)
> (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment
> #11)
> > Comment on attachment 8771244 [details]
> > WIP: Add a service queue for mDNS
> > 
> > No sure if this patch really works since we haven't understand the root
> > cause. Need @xeonchen's input on how to further debugging on this issue
> > since he know better about the current MDNS service implementation than me.
> 
> If you want debug the b2g process, the only way I know is to attach it
> rather than running it directly.
> Did you try |./run-gdb.sh attach|?
> 
> I'm not sure I understand the reason of the patch, when will the service
> queue contain more than one service?
We will use mDNS to register a remote-control service[0] when network is connected. I think presentation api also be registered in the same time.
What will happen when both services are registered? If the presentation api is registered first[1], is it possible to register the remote-control service before receiving the presentation api's reply[2]? 
I am not familiar with mDNS stuff, could you give me some suggestions?

[0] https://github.com/mozilla-b2g/gecko-b2g/blob/b2g48_v2_6/b2g/components/RemoteControlService.jsm#L434
[1] https://dxr.mozilla.org/mozilla-central/source/netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp#416
[2] https://dxr.mozilla.org/mozilla-central/source/netwerk/dns/mdns/libmdns/MDNSResponderOperator.cpp#527

Comment 15

[C.M.Chang[:chunmin]]

From Gary's testing, mDNS is able to register two services at the same time[0], so it should be fine without the queue.

[0] https://gist.github.com/xeonchen/1f8ab104b3d90c3cfb15c29b81f8edd9

Comment 16

[C.M.Chang[:chunmin]]

(In reply to Chun-Min Chang[:chunmin] from comment #15)
Well, I notice that there is one |nsIDNSServiceInfo| and |nsIDNSRegistrationListener| in MDNSResponderOperator[0], so it is possible to be overwritten when the second service is registered before receiving first service's reply.

[0] https://dxr.mozilla.org/mozilla-central/source/netwerk/dns/mdns/libmdns/MDNSResponderOperator.h#92

Comment 17

[C.M.Chang[:chunmin]]

Attachment: (Re)start remote-control service when time is changed

|window.onmoztimechange| can solve the issue for "generating certificate at the wrong time", so I submit the patch. The intermittent mDNS problem should be opened on another bug to trace it.

Comment 18

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

see my comment in pull request.

Comment 19

[C.M.Chang[:chunmin]]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

keep same structure

Comment 20

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

see my comment in pull request

Comment 21

[C.M.Chang[:chunmin]]

If we delete the previous certificate on every window.onmoztimechange, then the certificate will be different on every boot up. The certificate fingerprint is currently used as the server's identification. Thus, if it's always different, then the paired client can't recognize it (so we need to enter PIN code to pair devices every time). 

One way is to use other fixed value like MAC address as server's identification, but I am not sure if it's possible to get MAC address of server on client side. Another way is to find a way to validate the certificate's date. If the date is expired, then we update it on window.onmoztimechange.

Any idea?

Comment 22

[C.M.Chang[:chunmin]]

(In reply to Chun-Min Chang[:chunmin] from comment #21)
certificate.validity.notAfterLocalTime[0,1] might be a useful reference

[0] https://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/nsIX509Cert.idl#138
[1] http://searchfox.org/mozilla-central/source/security/manager/ssl/nsIX509CertValidity.idl#54

Comment 23

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Chun-Min Chang[:chunmin] from comment #21)
> If we delete the previous certificate on every window.onmoztimechange, then
> the certificate will be different on every boot up. The certificate
> fingerprint is currently used as the server's identification. Thus, if it's
> always different, then the paired client can't recognize it (so we need to
> enter PIN code to pair devices every time). 
> 
> One way is to use other fixed value like MAC address as server's
> identification, but I am not sure if it's possible to get MAC address of
> server on client side. Another way is to find a way to validate the
> certificate's date. If the date is expired, then we update it on
> window.onmoztimechange.
> 
> Any idea?

Per our discussion, we'll need a logic in RemoteControlService.jsm that checks if it needs to recreate the certificate if expired. This logic will be used every time server start or system time change.

Comment 24

[C.M.Chang[:chunmin]]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #23)
> (In reply to Chun-Min Chang[:chunmin] from comment #21)
> Per our discussion, we'll need a logic in RemoteControlService.jsm that
> checks if it needs to recreate the certificate if expired. This logic will
> be used every time server start or system time change.

I found the logic is inside LocalCertGetTask[0]

It explains why we can't start remote-control service upon TV is booted up, even when we use window.onmoztimechange to update the certificate. (we will get following sequence if we start service on boot op.) When |getOrCreateCert|[1] is called for starting service, it will validate the certificate's period. If current time < cert's started day or (current time - cert's expired day) < 1 day, then it's invalid[2]. 

0) flash
  Init time is 2016/xx/xx
  start service -> getOrCreateCert -> Get cert fail -> create cert 'A' started from 2016... 
1) first boot
  Init time is 2000/xx/xx
  start service -> getOrCreateCert -> Get cert 'A' -> validate fail -> create cert 'B' started from 2000...
  (onTimeChange -> time is 2016/xx/xx -> getOrCreateCert 'B' -> Get cert -> validate fail -> create cert 'B_1' started from 2016...)
2) second boot
  Init time is 2000/xx/xx
  start service -> getOrCreateCert -> Get cert 'B_1' -> validate fail -> create cert 'C' started from 2000...
  (onTimeChange -> time is 2016/xx/xx -> getOrCreateCert 'C' -> Get cert -> validate fail -> create cert 'C_1' started from 2016...)
...
...
...

[0] http://searchfox.org/mozilla-central/source/security/manager/ssl/LocalCertService.cpp#84
[1] https://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/nsILocalCertService.idl#30
[2] http://searchfox.org/mozilla-central/source/security/manager/ssl/LocalCertService.cpp#331

Comment 25

[C.M.Chang[:chunmin]]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

Keep anonymous function to create a local scope for all the remote control objects, but extract those duplicated code into functions.

For time-changed event, we will do the following things:
1) RemoteControlService.stop() if service is running.
2) Delete the previous certificate and generate a new one if it's invalid
   - nsILocalCertService.getOrCreateCert will do this based on the current time
3) RemoteControlService.start()

A |onTimeChange| function is added into remote_control.js to notify |RemoteControlService.jsm|. From perspective of remote_control.js, it doesn't know the certificate stuff, so it just passes this signal to RemoteControlService.jsm for further handling.

Comment 26

[C.M.Chang[:chunmin]]

(In reply to Chun-Min Chang[:chunmin] from comment #16)
This is incorrect. The RegisterOperator is newed and the serviceInfo is from underlying DNS Service Discovery(dns_sd.h). It's weird that the intermittent mDNS problem is never shown after I push the bluetooth service to nexus player....

Comment 27

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

see my comment in pull request.

Comment 28

[C.M.Chang[:chunmin]]

The remote-control service need to force setting |MOZ_B2G_REMOTECONTROLSERVICE| to true and comment some code in RemoteControlService.jsm to make it work. Mulet has neither nsINetworkManager[0] for monitoring the network status(it's for Gonk only) and nor window.onmoztimechange(Gonk only too?). 

In such case, we can't start the remote-control service if we don't run it directly on booting up. 

If we really need to run this service on non-gonk platform and avoid this issue, I think we need to manage the certificate by our own instead of nsILocalCertService. For example, 
 
RemoteControlService.jsm
--------------------------------------------

function start():
  ...
  ...

  cert = nsIX509CertDB.FindCertByNickname() // [1, 2]
  if (!cert)
    cert = nsILocalCertService.getOrCreateCert(...)
  }

  // The certificate will be validated upon receiving time-changed event
  ...

function onTimeChange(aTimestamp): // Triggered by window.onmoztimechange
  ...
  ...

  if (aTimestamp > cert.validity.notAfter) { // [3, 4]
    // The certificate is expired, get a new one
    nsILocalCertService.removeCert() // [5]
    cert = nsILocalCertService.getOrCreateCert(...)
  }
  
  ...
  ...


However, the window.onmoztimechange is also need to port to the non-gonk platform, otherwise, it won't be checked.

What do you think? Should we need to enable remote-control service on Mulet? Is it ok that we could just do the certificate part on Mulet and leave the window.onmoztimechange and nsINetworkManager part?

[0] https://dxr.mozilla.org/mozilla-esr45/source/dom/system/gonk/nsINetworkManager.idl
[1] http://searchfox.org/mozilla-central/source/security/manager/ssl/nsIX509CertDB.idl#86
[2] http://searchfox.org/mozilla-central/source/security/manager/ssl/nsNSSCertificateDB.cpp#98
[3] http://searchfox.org/mozilla-central/source/security/manager/ssl/nsIX509Cert.idl#138
[4] http://searchfox.org/mozilla-central/source/security/manager/ssl/nsIX509CertValidity.idl#47
[5] http://searchfox.org/mozilla-central/source/security/manager/ssl/nsILocalCertService.idl#39

Comment 29

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Thanks for the detailed explanation. Spending too much effort on enabling remote control on Mulet is unnecessary since we are not shipping TV 2.6 simulator. You can r? me when you address the one last comment I made on pull request.

Comment 30

[C.M.Chang[:chunmin]]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

The main difference in this version:
1. Implementation of comment 28
2. Start remote-control service on booting up
3. Divide _doStart into several components for readability
4. Check the discoverable before firing onTimeChange for RemoteControlService.jsm


The (1) will enable remote-control service on some platform without window.onmoztimechange and nsINetworkManager. It will run directly on starting up. However, it will make readers a bit confused about managing certificate ourselves instead of the inside logic in gecko. Leave some comments might be helpful.

Comment 31

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

see my comment on pull request.

Comment 32

[C.M.Chang[:chunmin]]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

The previous patch for implementation of comment 28 depends on Gonk's behaviour. Thus we wrap the code for Gonk only into preprocessor directives and leave other's behaviour as usual. 

For non-gonk platform, the service will be started on booting up and validate the certificate at the same time.

For gonk platform, the service will be started on booting up but use the previous certificate without validation. The certificate will be validated when receiving time-changed event.

Comment 33

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

see my comment in pull request.

Comment 34

[C.M.Chang[:chunmin]]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

Move those code related to |nsIIOService| out of MOZ_WIDGET_GONK directives.

Comment 35

[C.M.Chang[:chunmin]]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 36

[Josh Cheng [:josh]]

Comment on attachment 8772291 [details] [review]
(Re)start remote-control service when time is changed

Approve for TV 2.6

Comment 37

[C.M.Chang[:chunmin]]

Hi Gary,
Could you help to merge this?

Comment 38

[Gary Chen [:xeonchen]]

https://github.com/mozilla-b2g/gecko-b2g/commit/dd3ccdec45a855410cbdaced5efd2a90fa74ad52