-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make Airbitz default accounts system #842
Conversation
This breaks a few things and prevents ability for user to PIN login
Use can exit out and use fully clientside key management
Can the modal header text be changed to something like |
@paullinator If I register with airbitz, then try and use the login ID (copy/pasted from the Accounts page) to login using the "manual" (non-airbitz) login, I get an invalid username/password error -- any idea what's going on here? |
Also, when I do a non-PIN login using the Airbitz modal, the following error appears in the console:
(The login completes successfully, though.) |
Also, could @paullintor or @swansontec explain a bit about what updating is planned to be added to the login-account selector? if (loginAccount.airbitzAccount) {
loginAccount.onAirbitzManageAccount = () => {
require('../../../selectors').abc.openManageWindow(loginAccount.airbitzAccount, (result, account) => {
// Possibly update the loginAccount
});
};
} |
@tinybike In response to your question: "If I register with airbitz, then try and use the login ID (copy/pasted from the Accounts page) to login using the "manual" (non-airbitz) login, I get an invalid username/password error -- any idea what's going on here?" I believe in the Augur code, the password for the login ID is hardcoded to something fixed and not the actual Airbitz password since the Airbitz password might change but the encryption of the login ID doesn't change. I'll need to look into the code to see what password is used. |
@tinybike The hardcoded password for the LoginID that you pull out of the Account is simply "password". I looked at the code that generates the loginid and it's all in augur.js:loginWithMasterKey(). This was the routine you gave us at accomplishes the equivalent of augur.js:register() but with a privateKey given from the Airbitz SDK. There seems to be a few subtle differences between the two routines now and I'm wondering if they would be a cause for the issue. Specifically the kdfparams are determined a little differently in each. Can you take a look and see what would cause the differences? |
I'm confused. Why are there hardcoded constant password, IV, and salt variables in loginWithMasterKey? Shouldn't these be passed in from Airbitz? These look like simple placeholders in a routine that I wrote as an example, so you guys could work from it. var salt = new Buffer("6169fdd07cb61657ad0d1c60f1132eed52c91949d6d85654110b11ede80a6d2e", "hex");
var iv = new Buffer("ef40723ec10d95c4356c8d157ce4308e", "hex");
keys.deriveKey("password", salt, null, function (derivedKey) { ... }); Am I missing something / did I just not review your code thoroughly? This appears to be obviously insecure. Why are salt, iv, and "password" constants? |
@tinybike These are constants because Airbitz does its own encryption in its SDK using it's own random salt, IV, and the a strong scrypt hash of the user's username and password. These can't be transferred out of the SDK to Augur. The keys are still secure as they are encrypted by Airbitz. We simply kept the keythereum format internally within Augur to provide compatibility with all your code that expects that format. The keythereum format however requires encryption parameters so we just gave it constant values to get it to work. |
Ah, alright. So, the issue with this is that the "keystore" json that is generated can be trivially decrypted to reveal the plaintext private key (which is not supposed to be the case). However it shouldn't be necessary for the Airbitz login to mimic the "manual" augur login 100%. There are basically two things the derived key is used for by the front-end:
|
@tinybike #1 (prompting the user to re-enter a password) is perfect if you don't mind the user doing that. Note that it's possible that the user enters a different password for #1 than they actually had in their Airbitz account. #2. I wouldn't directly use the private key for encryption but instead apply a HMAC_SHA256 of the private key and use that for the AES128 encryption. @swansontec is more an expert on this than me and can comment further. |
Addendum to point (1) re: exporting the keystore JSON to file. Is this a capability you are planning to add to Airbitz itself? The reason we have this as an option in augur is so users can export their "augur account" to a file which can be seamlessly read by geth (or other local ethereum node). Since Airbitz is now an Ethereum wallet, this would IMO be a nice feature for it to have anyway, to make it fully interoperable with other Ethereum clients. If Airbitz wanted to offer this export functionality, we could just disable that export altogether from augur for Airbitz accounts, and instead it would be done thru the Airbitz interface. Alternately, Airbitz could just pass augur the keystore JSON. (The "keythereum format" you refer to isn't actually something peculiar to keythereum -- it's an implementation of https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition, which is something that geth and other eth clients also implement.) |
#1 We do plan to offer key export eventually and even natively support ethereum in the Airbitz mobile wallet but not in the near future and not before Augur launch. For export from the Airbitz mobile wallet, we would probably prefer the 12 word mnemonic format vs the keystore JSON however since exporting a JSON file from a mobile device is pretty problematic. From within the JS SDK we could offer both options pretty easily. #2 BTW. We currently store in the user's account a random string as the raw entropy for the master seed for creating the private key. If we instead stored a BIP44 word list, is there a standard way to go from word list to JSON keystore that's compatible will most ethereum dapps? I think future dapps may only want to support the word list and we'd like those wallets to be compatible with Augur. We should always be able to go from word list to JSON keystore, but if the JSON keystore is derived from a random string/buffer, we can't ever go back to a word list. |
For (1), would it be an issue for you guys to import keythereum into Airbitz and add a function to your JS SDK that just does this: Airbitz.getKeystoreJSON = function (password, privateKey, salt, iv, callback) {
return keythereum.dump(password, privateKey, salt, iv, {}, callback);
}; (where password, privateKey, salt, and iv can be provided as either strings or Buffers, whichever is easier.) This function would be called on register/login, and the JSON output would be sent to augur. |
In BIP44, the word list is used in combination with a user-supplied password to derive the private key, correct? As I understand it, the JSON keystore is sort of a level above that -- you've already generated the private key, then you encrypt it (with a password), and the JSON keystore is just the encrypted private key (+ parameters for the encryption). Or, am I misunderstanding your question? |
If you're asking if Ethereum has a standard way of going from the mnemonic to the private key, unfortunately I don't think so at the moment -- googled a bit and it seems like it's still in the discussion phase. Here's a "pre-EIP" and some discussion: https://www.reddit.com/r/ethereum/comments/55i25c/preeip_password_protected_paper_wallets_feedback/ And here's an actual EIP that doesn't appear to have been acted on yet: ethereum/EIPs#75 I know Jaxx, MetaMask, Ledger Nano S, et al do use mnemonics, but I'm unfortunately not knowledgeable enough about them to know if they're compatible with each other. I hope they are :/ |
The immediate issue is some of the stuff related to our accounts won't work with Airbitz logins -- specifically, the giant goofy login ID (which is the keystore JSON in base58) and the account export features. What would you think of, instead of having a big blue "Manage Airbitz Account" button and modal, just replacing the top section on the account page with the airbitz account manager (for airbitz logins only, of course)? You'd have to display the account address, but other than that, it should be pretty much a drop-in thing. |
Regarding the BIP44 mnemonic, @paullinator, I don't think we should go that route. Ethereum as an ecosystem has settled on an encrypted file as their key storage mechanism, so we should accept that choice and move on. There are usability tradeoffs either way, and besides, you yourself are always talking about how sucky mnemonics are. Airbitz is just a key store. In fact, you can see the code in Augur that creates the ethereum key and puts it into Airbitz. Getting the key out is the same way. Airbitz's job is to manage and secure this key, which may or may not involve a password. In the case of Edge login, it's an explicit design goal not to know the password on the client. Given an active Airbitz login, you can implement This key, being 256 bits long, can be used both for EC stuff and with AES-256. The 256 refers to the expected key length, so this is the right crypto algorithm. Actually, you want AES-256-CBC + HMAC-SHA256 to ensure message integrity, but proper AEAD constructions are a whole other can of worms (which I am happy to open if you care to). If you were really paranoid, you could maybe use |
@swansontec The things needed to construct the keystore JSON are 1) the private key, 2) the user's password, and 3) the salt (+ initialization vector, if applicable). The password is presumably not stored anywhere by Airbitz, so there are two options for constructing the JSON:
|
@tinybike Of those two above options:
|
@paullinator: I'm closing this b/c it conflicts with our recent auth refactoring / reskinning. Does the current setup at https://app.augur.net look alright to you? We thought the auto-popup wasn't great UX, so instead, we put the "login with airbitz" button at the top, and put the "manual" login below it. (Once you're logged in with airbitz, you receive the popup automatically, which we thought was reasonable since the user has already decided to use Airbitz at that point.) Let me know what you think! |
Always popup Airbitz login modal when user clicks SignUp/Login