722579 - JPakeClient.java shouldn't log encryption key / hmackey

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Android Sync, defect, P1)

Description

[David Chan [:dchan]]

The logging code should not log potentially sensitive data such as encryption keys. [1]

Are the encryptionKey and HMACKey the values used during the JPAKE exchange or are they the keys for user's WBO data?

[1] - https://github.com/mozilla-services/android-sync/blob/develop/src/main/java/org/mozilla/gecko/sync/jpake/JPakeClient.java#L508

Comment 1

[Richard Newman [:rnewman]]

(In reply to David Chan [:dchan] from comment #0)

> Are the encryptionKey and HMACKey the values used during the JPAKE exchange
> or are they the keys for user's WBO data?

The former. The Sync Key is exchanged as a string, and the keys used for user data are stored on the server, not distributed via J-PAKE.

Comment 2

[Richard Newman [:rnewman]]

Fixed in develop:

https://github.com/mozilla-services/android-sync/commit/7721c1c50f28113ddd0db235894175e317c41a41

Comment 3

[Richard Newman [:rnewman]]

https://hg.mozilla.org/mozilla-central/rev/1bfbf78359af

Comment 4

[Tracy Walker [:tracy]]

Sync Key is no longer displayed

however, filtering on hmac I'm seeing in log:

02-23 11:08:46.820: I/EnsureKeysStage(2752): Fetched keys: {"id":"keys","payload":"{\"ciphertext\":\"XLUS\/qqx6QXD\/70\/9VqCxLbew2B+StzQQqj0xh4JtP1w8QuoXC1Ut6nL9jVLhk452WmdLeFkWsf6Q43lNTDbMTY+5yw+ShT4cxiUqnszx4K+F6zpAblZO6MaEulP5k+rqIE3dtgabkM1VQVXXjp1+jxg6AkR7w6M\/\/jflkTNuMl703vY+BMmDPYsz7ujHbmkTFhdKuUWtUdJA5El4Ly+zg==\",\"IV\":\"q7NmZtaT0Y9FZIcGufFJlQ==\",\"hmac\":\"621f3d3f4cd2231db79b9ecaf2b551403e11741cb28a6ebfaa1c8a37e54a8c5f\"}","modified":1.32934794443E9}

is that relevant to this bug?

Comment 5

[Richard Newman [:rnewman]]

(In reply to Tracy Walker [:tracy] from comment #4)

> is that relevant to this bug?

Related bug :)

(That's your keybundle, which is encrypted by your sync key. We shouldn't log that by default, either.)

Comment 6

[Tracy Walker [:tracy]]

filed bug 730001

I don't see any other instances of keys, so I'll mark this verified and let the last bit be tracked in the related bug.