Adding Celery to wallets

[dthakur]

PR adds Celery as a bitcoin wallet.

Celery is a hosted wallet and buying/selling service.

Related information:

• CoinDesk
• Transparency

Need some screening/review of the "check" sections.

[harding]

@dthakur thanks for submitting your wallet! I tested your wallet this afternoon (as much as I could) and didn't see anywhere it didn't meet our requirements. I do have a couple questions I hope you can answer:

1. Celery seems to accept weak passwords (I used "password") and has persistent sessions, at least to a certain degree. Because of this, do you require that users authenticate with their mobiles each time they want to buy or spend Bitcoins?
2. What happens if a user loses access to their mobile number for two-factor authentication?

In addition, your website says your source code is audited. Is there any chance that you can provide us with the name of the auditing firm? (Also, do you know if they have experience auditing cryptocurrency code for things like handling of conflicted transactions?)

Thanks again for your wallet submission!

[dthakur]

@harding Thanks for reviewing Celery.

1. Currently, a user session is valid for 24 hours. We did accept weak passwords. However, I have since modified Celery to no longer accept weak passwords. This is implemented on the server side (using zxcvbn). Currently, we do not require 2FA for sensitive operations.
2. When a user loses they ability to use 2FA, we are usually notified via an emailed support request and follow up with a phone call with the mobile phone we have verified upon initial on-boarding. We then confirm the user identity with the information provided by the user during on-boarding. After confirming the user's identity, we disable 2FA on the user's account. After which, the user can login in and can re-enable 2FA.

Auditing: we have been dealing with an auditing firm (with an infosec focus, rather than accounting focus). However, the project is still on-going. For now, I have removed the claim from our site until we have an audit report to share.

[harding]

@dthakur thank you for your quick answers, and for making the change to reject weak passphrases!

Regarding point 1, do you at least require re-entry of the passphrase for sensitive operations such as buying or spending bitcoins? I ask because one of our wallet requirements is "User session is not persistent, or requires authentication for spending."

Regarding point 2, I'm a bit confused. Your 2FA method is a text message to the user's mobile number, so what happens if (for example) a user changes their mobile number while they still have bitcoins on deposit at Celery? (You can't call their previous number in that case.) I ask this because one of our other requirements for hosted wallets is, "Provides account recovery feature."

Thanks again for your quick answers, and I apologize if all these questions sound a bit like an interrogation. :-)

[dthakur]

@harding

Regarding point 1: Let me review, and see if we can gain compliance. I'll update in a day or so.

Regarding point 2: Currently, we only support TOTP 2FA. We do not use SMS 2FA. The SMS verification code sent to the user when adding a phone number, is only used to verify the phone number. It is not used for logging in or any other privileged actions.

Doesn't feel like an interrogation at all. The thoroughness is refreshing.

[harding]

@dthakur re: point 1, thanks for considering this!

Re: point 2, oh, sorry, I misunderstood the help text on the phone number verification page. I now see the Security tab on the Account page, which will let me setup 2FA with Google Authenticator. Is it possible for you to tell the user about this during the registration process, or remind them about it periodically? One of our requirements is that hosted wallets must "Reminds the user to enable 2FA by email or in the main UI of the wallet".

[dthakur]

@harding Looks like we will need to make changes, which might take a few weeks:

• User session is not persistent, or requires authentication for spending.
• Reminds the user to enable 2FA by email or in the main UI of the wallet.

What would be the correct protocol? Should I close this PR and re-open once ready?

[saivann]

@dthakur Thanks for taking time to answer questions and take feedback into consideration! Unless @harding wishes to work differently, keeping this pull request open is fine. You can just comment here once ready.

@harding Just so you know, I didn't check how account recovery worked with 2FA with other wallets. This question took me by surprise, it's a good point! FWIW, I feel like this requirement is a double-edged sword, and may actually be worth updating somehow so it can't promote a security flaw (e.g. making it easy to bypass authentication). But that probably belongs to a separate discussion :) .

[harding]

@dthakur absolutely, please leave it open. (This also helps us, as often fans of a wallet will suggest we add it. I like being able to point them at an in-progress PR.) Thanks again for being so responsive!

@saivann oh, whoops. Maybe that means there's something else I'm not checking? What were you thinking when you wrote for the requirements, "Provides account recovery feature"?

I've been keeping my notes on Celery here, which I was planning to link to that when I made my final recommendation.

[saivann]

@harding I wrote this requirement only because web wallets all provided the feature, and I know people are used to expect it on web services. I just didn't later take time to think of it's interaction with 2FA.

Account recovery is an alternative way to authenticate so it carries the risk of being used to bypass stronger authentication if it's not enough demanding. On the other hand, if there is no way to recover an account if you lose your 2FA device, this also creates risk of losses.

Since it doesn't apply here I think it's fine. Maybe it would be interesting to see how other wallets deal with that issue at some point, and improve this requirement if it was to create issues.

[harding]

@saivann ahh, I see. It meant, "provides [a secure] account recovery feature". I'll see if I can rephrase it and capture some of the other nuances of this discussion, and open a PR updating the requirements list. Thanks!