966353 - What should we do if CryptoUtils.computeHAWK fails?

Status: RESOLVED WORKSFORME

(Firefox :: Sync, defect, P2)

Description

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

computeHAWK is used to compute the HAWK auth header for both services/sync/modules/browserid_identity.js and services/common/rest.js.

Investigate possible failure modes and how those modules should respond to an error

Comment 1

[Mark Hammond [:markh] [:mhammond] - PTO until April 20]

Jed,
  What failure modes do you forsee here?  Or maybe just logging is necessary?

Comment 2

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

In Bug 957863 Comment 31, :rnewman asked whether CryptoUtils.computeHAWK could fail.

Since I didn't have an immediate answer, I found it a good question, and felt it deserved further thought.  So I opened this bug.  I cannot, however, see what special cases could induce this to fail.  CryptoUtils.computeHAWK already checks for the more likely errors and throws if necessary.  And we are not, as far as I can see, doing anything that would be likely to cause an out-of-memory eviction mid-process on b2g.  (The same goes for the rest of our crypto operations, in particular pbkdf2.)

In the end, I came to the same conclusion as you may have, namely that some verbose logging might be useful in case of error, but for now I don't see much else to do.

Comment 3

[Richard Newman [:rnewman]]

Part of the question is: if computeHAWK throws, do we do the right thing?

That should be easy to test by making computeHAWK always throw.

If there are no pernicious failure modes, and all throws are handled correctly, then thumbs up.

Comment 4

[Chris Karlof [:ckarlof]]

I had computeHAWK throw, and it put the browser in the "signin to reconnect to Sync" state, which is what I expected to happen. 

FxA already does some logging when its use of computeHAWK throws: 1394153287954	FirefoxAccounts	ERROR	HAWK.signCertificate error: {}

I don't think this should be a Fx29 blocker anymore though.

Comment 5

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

I agree

Comment 6

[Ryan Kelly [:rfkelly]]

It sounds like this is working more-or-less as intended and we can just close it out.