Why is Armory sending our *USERNAMES* to bitcoinarmory.com ‼️

[Dabs]

Well, if you have a properly configured router or proxy, Armory will never get to send any data out.

The best way is to accept the user's honesty and request for this information upon download. Meaning, just count the number of downloads from the armory website.

It's not perfect as some people will download the next version more than once.

You can't filter out dupes this way, as some people might use proxies or VPNs or coincidentally the same TOR exit node. And you'll miss all the other numbers from people mirroring the site.

I download the install files once from a different computer. I install it on my other computer or in a VM. I also use Deep Freeze, so often times Armory has no memory that I've already installed it yesterday, because as far as it's concerned, it's being installed for the first time.

[chrisrico]

in response to justus: we want an identifier that will be persistent between loads so that users that start the program over and over are not counted for each start

It seems like a GUID would be a great fit for this requirement. Try to load it from the settings file, if it's not there generate one and save it.

[ar9]

This is definitely concerning to me.
Thanks for blowing the whistle and thanks for accepting the criticism, devs.  Thumbs up all-around here.

[the joint]

Sorry guys, I've been out all day, but I've been thinking about this.  You all are absolutely right.  We made two mistakes:

(1) We assumed that because we choose not to store/process the IP data, that users would believe us that we don't
(2) We incorrectly assumed the that space of user home paths on your desktop was big enough that the 4 byte identifier would not have adverse privacy implications. I did not consider that people's home usernames might be, say, their username on bitcointalk.org

It's quite clear that those two pieces of data do have pretty serious privacy implications.  I want to fix this ASAP.  

To be clear, the reason we made the unique identifiers is because we don't want to store any IP data for the reasons described: if there was a subpoena of some sort, we'd prefer to not have anything to reveal.  And we don't store it.  But, we incorrectly assumed that the unique identifiers would be sufficiently non-privacy-leaking while still allowing us to remove duplicates (in response to justus: we want an identifier that will be persistent between loads so that users that start the program over and over are not counted for each start--as we all know, a lot of users have difficulty with Armory and will start it 300 times to try to get it to work).  It is clear these were bad assumptions since we technically could be storing both which would be quite bad.  

I hope we've generated enough good faith with the community that we get a little slack that this was not our intention.  I take the blame for not realizing that, and I want to make sure that it is fixed.  ASAP.  I will happily take feedback on how this should be adjusted so that we can meet our goals without compromising the privacy of the users.  

I agree we should decouple the option from the announcement fetching.  We consider announcements to be extremely important, and why we made that difficult to disable:  if there's a critical security (or privacy!) vulnerability in Armory, there is a very short window where someone might try to exploit it to steal peoples' coins, and there's no better way to help users than to make sure a big scary warning pops up the next time they start Armory.  The fact that we coupled the OS/version reporting with meant that it was as hard to disable that as it is the announcement fetching.  We can easily separate them and will happily make it easy to disable the OS/version reporting.


Re privacy policy:  On the advice of our lawyer, we included the "may collect IP addresses" because we have no way to prove that we don't.  And since our website uses google-analytics, we don't have control over what google does with the access patterns of users to our website.  It was a bit of CYA that companies have to abide by, especially in the US.  Note we describe at the bottom of that page we describe how to disable it with a link to our troubleshooting page.

On the upside: another positive example of the power of open-source software.  We have casually encouraged users to go through the code base, and we even contacted the Open Crypto Audit Project to try to get a code review (and never heard back from them).  We are obviously believers in open-source, and here's the first solid example of Armory getting better because of it.  We will get this fixed.

A very nice, professional response.  I'm glad to see this issue is being addressed in a transparent manner.

[DeeBo]

Well on the bright side Armory is open source so I forked it on GitHub to remove this behavior.  It looks to be done entirely in a single Python file so you don't even need to rebuild anything to make the change.  Just merge these changes into your announcefetch.py and you should be good to go.

https://github.com/weirddan455/BitcoinArmory/commit/09207dc089facfcea279e485077ed0848491995b

[ar9]

Well on the bright side Armory is open source so I forked it on GitHub to remove this behavior.  It looks to be done entirely in a single Python file so you don't even need to rebuild anything to make the change.  Just merge these changes into your announcefetch.py and you should be good to go.

https://github.com/weirddan455/BitcoinArmory/commit/09207dc089facfcea279e485077ed0848491995b

Awesome.  I will start using this if there isn't an official fix within a week.

[kerimk2]

Simple don't use armory, if you don't like it. This is absolutely unacceptable. This should be fixed right away.

[ar9]

Simple don't use armory, if you don't like it. This is absolutely unacceptable. This should be fixed right away.

Problem is, I love Armory.

[etotheipi]

Okay, so here's my plan.

I'd like to have a new version out by next week (0.92.2).  Given that the change is small and doesn't go by any critical code paths, the release testing process can be relaxed slightly.   We'll try to have it out by Monday.

Second, the way I'd like to do it is to put a 4-byte random identifier in the settings file, and use that to for duplicate detection.  That identifier will be overwritten/changed every month, so that any thing that would care about trying to match IDs to systems will expire after a month.  This allows us to aggregate up to monthly statistics.  Anything longer than that we can do without.  

Third, we will decouple the announcement stuff entirely from OS/version reporting.  We will add an option in File->Settings to completely disable this.  Then announcement fetching will send a bare string with no extra metadata.   And as suggested, no extra data needs to be sent on subsequent announcement fetches.

Fourth, we will add a command-line option called "--tor" (with an equivalent option in the settings).  Then we will adapt the code to use that flag  to implement all the standard Tor-based settings:  most likely "--skip-announce-check --skip-online-check --satoshi-port=X".  You guys will be able to examine what code paths are affected by this setting and make recommendations for us to improve it.   (note there is also a "--skip-version-check" flag, but that is no longer used, since we updated the announcement system in 0.91).

I want to reiterate that we do care about privacy -- I've personally been a proponent of security and privacy on these forums for years.  And it would be tough to see why we would really care to match up IDs with announcement fetch pings.  We're not in the data collection business, no advertising, nothing.  We wouldn't know what what to do with it even if we saved it.   We (I) simply made a judgment error when implementing this (compounded by the fact that it was developed as part of a feature we wanted to aggressively promote -- security announcements).  Armory is a massive program, and it is respected for being thorough and careful, but we can't get 100% right.  One of the nice things about open-source is that people can find issues, call them out, and get it fixed.  And that's exactly what happened here.  Thanks for your guys' patience and we'll get this fix out there.

  

[justusranvier]

Okay, so here's my plan.

I'd like to have a new version out by next week (0.92.2).  Given that the change is small and doesn't go by any critical code paths, the release testing process can be relaxed slightly.   We'll try to have it out by Monday.

Second, the way I'd like to do it is to put a 4-byte random identifier in the settings file, and use that to for duplicate detection.  That identifier will be overwritten/changed every month, so that any thing that would care about trying to match IDs to systems will expire after a month.  This allows us to aggregate up to monthly statistics.  Anything longer than that we can do without. 

Third, we will decouple the announcement stuff entirely from OS/version reporting.  We will add an option in File->Settings to completely disable this.  Then announcement fetching will send a bare string with no extra metadata.   And as suggested, no extra data needs to be sent on subsequent announcement fetches.

Fourth, we will add a command-line option called "--tor" (with an equivalent option in the settings).  Then we will adapt the code to use that flag  to implement all the standard Tor-based settings:  most likely "--skip-announce-check --skip-online-check --satoshi-port=X".  You guys will be able to examine what code paths are affected by this setting and make recommendations for us to improve it.   (note there is also a "--skip-version-check" flag, but that is no longer used, since we updated the announcement system in 0.91).

I want to reiterate that we do care about privacy -- I've personally been a proponent of security and privacy on these forums for years.  And it would be tough to see why we would really care to match up IDs with announcement fetch pings.  We're not in the data collection business, no advertising, nothing.  We wouldn't know what what to do with it even if we wanted to.   We (I) simply made a judgment error when implementing this.  Armory is a massive program, and it is respected for being thorough and careful, but we can't get 100% right.  One of the nice things about open-source is that people can find issues, call them out, and get it fixed.  And that's exactly what happened here.  Thanks for your guys' patience and we'll get this fix out there.

+1

[ForgottenPassword]

Okay, so here's my plan.

I'd like to have a new version out by next week (0.92.2).  Given that the change is small and doesn't go by any critical code paths, the release testing process can be relaxed slightly.   We'll try to have it out by Monday.

Second, the way I'd like to do it is to put a 4-byte random identifier in the settings file, and use that to for duplicate detection.  That identifier will be overwritten/changed every month, so that any thing that would care about trying to match IDs to systems will expire after a month.  This allows us to aggregate up to monthly statistics.  Anything longer than that we can do without.  

Third, we will decouple the announcement stuff entirely from OS/version reporting.  We will add an option in File->Settings to completely disable this.  Then announcement fetching will send a bare string with no extra metadata.   And as suggested, no extra data needs to be sent on subsequent announcement fetches.

Fourth, we will add a command-line option called "--tor" (with an equivalent option in the settings).  Then we will adapt the code to use that flag  to implement all the standard Tor-based settings:  most likely "--skip-announce-check --skip-online-check --satoshi-port=X".  You guys will be able to examine what code paths are affected by this setting and make recommendations for us to improve it.   (note there is also a "--skip-version-check" flag, but that is no longer used, since we updated the announcement system in 0.91).

I want to reiterate that we do care about privacy -- I've personally been a proponent of security and privacy on these forums for years.  And it would be tough to see why we would really care to match up IDs with announcement fetch pings.  We're not in the data collection business, no advertising, nothing.  We wouldn't know what what to do with it even if we saved it.   We (I) simply made a judgment error when implementing this.  Armory is a massive program, and it is respected for being thorough and careful, but we can't get 100% right.  One of the nice things about open-source is that people can find issues, call them out, and get it fixed.  And that's exactly what happened here.  Thanks for your guys' patience and we'll get this fix out there.

Absolutely great! Thank you so much - this is very reassuring. This solution seems to solve everything that I was worried about.

For a while I thought maybe Armory (the company) had been served with some scary paperwork and told to collect info from users machines. Obviously I was wrong, clearly you guys care about privacy (and security of course) and you listen to your users (something a lot of software devs don't do). Again thanks for doing this, I'll be sending a donation soon and another once 0.92.2 is released and I take a look at the changes.

Armory FTW.

[r04r]

Okay, so here's my plan.

I'd like to have a new version out by next week (0.92.2).  Given that the change is small and doesn't go by any critical code paths, the release testing process can be relaxed slightly.   We'll try to have it out by Monday.

Second, the way I'd like to do it is to put a 4-byte random identifier in the settings file, and use that to for duplicate detection.  That identifier will be overwritten/changed every month, so that any thing that would care about trying to match IDs to systems will expire after a month.  This allows us to aggregate up to monthly statistics.  Anything longer than that we can do without.  

Third, we will decouple the announcement stuff entirely from OS/version reporting.  We will add an option in File->Settings to completely disable this.  Then announcement fetching will send a bare string with no extra metadata.   And as suggested, no extra data needs to be sent on subsequent announcement fetches.

Fourth, we will add a command-line option called "--tor" (with an equivalent option in the settings).  Then we will adapt the code to use that flag  to implement all the standard Tor-based settings:  most likely "--skip-announce-check --skip-online-check --satoshi-port=X".  You guys will be able to examine what code paths are affected by this setting and make recommendations for us to improve it.   (note there is also a "--skip-version-check" flag, but that is no longer used, since we updated the announcement system in 0.91).

I want to reiterate that we do care about privacy -- I've personally been a proponent of security and privacy on these forums for years.  And it would be tough to see why we would really care to match up IDs with announcement fetch pings.  We're not in the data collection business, no advertising, nothing.  We wouldn't know what what to do with it even if we saved it.   We (I) simply made a judgment error when implementing this (compounded by the fact that it was developed as part of a feature we wanted to aggressively promote -- security announcements).  Armory is a massive program, and it is respected for being thorough and careful, but we can't get 100% right.  One of the nice things about open-source is that people can find issues, call them out, and get it fixed.  And that's exactly what happened here.  Thanks for your guys' patience and we'll get this fix out there.

Thanks! That sounds great. Thank you for your continued development.

[etotheipi]

Good news!  I have implemented all the changes I mentioned earlier today.  It's all in the privacyfix082014 branch.   Here's the full diff between the current master and the new code:

https://github.com/etotheipi/BitcoinArmory/compare/master...privacyfix082014

There are no promises that this is totally complete and bug-free.  But I tested all the different settings, modifying the settings file to make it look like it's a new month, etc.  So far, it all looks good.  But admittedly I was a little rushed.

The bad news is that I am preoccupied until Friday (which is why I rushed), so I won't be able to do any formal testing or final releases before then.  But I wanted to get this out ASAP and get people reviewing the code, as well as let anyone test it that knows how to checkout the repo.

I will start the testing by offering 0.05 BTC for bugs found in that branch.  This only applies to bugs related to privacy settings, announcement checking, etc.  Although we would like to know about unrelated bugs in general, for now we are trying to stay focused on getting this bug fixed ASAP.    To 1a5f9842524:  I believe you said you ran some network tools to find the original issue.  Can you do this again to make sure I didn't miss something? 

[LeMiner]

To 1a5f9842524:  I believe you said you ran some network tools to find the original issue.  Can you do this again to make sure I didn't miss something? 

That's awesome, great work!

I believe on Ubuntu running it through terminal you can see the fetches/etc and all that comes along with it. If anyone would compile it and run it that way it should be easy to spot if it's still repeating previous behavior.

Sadly myself I'm not familiar enough with code to review it.

Thanks again and awesome work!

[bitpop]

Glad the fud is over. Let the guys go back to bip32 and armory integration.

[Dabs]

Actually, I'm just happy it finally works on XP. However, it seems it wants me to "install" bitcoin ... I'll figure it out, don't worry, that's just a problem on my own computer. (It's set up in such a way that all apps don't know about any other apps, mostly.)

[haploid23]

etotheipi, it's great that you taking action so quick. At first, you were digging yourself a bigger hole by what seemed like excuses. I was about to jump ship to another wallet, but glad to stay now.

[DeeBo]

To 1a5f9842524:  I believe you said you ran some network tools to find the original issue.  Can you do this again to make sure I didn't miss something? 

That's awesome, great work!

I believe on Ubuntu running it through terminal you can see the fetches/etc and all that comes along with it. If anyone would compile it and run it that way it should be easy to spot if it's still repeating previous behavior.

Sadly myself I'm not familiar enough with code to review it.

Thanks again and awesome work!

All the changes are to Python files so, again, no need to compile anything.  Python is interpreted so you simply save your changes and run.  It's also really easy to debug by adding print statements in the code so it'll output to the console exactly what it's doing.  For example, if you wanted to see the URL it's using, add a print statement in the getDecoratedURL function right above the return statement like:

Code:

def getDecoratedURL(self, url, verbose=False):
blahblahblah
print url + '?' + urllib.urlencode(argsMap)
return url + '?' + urllib.urlencode(argsMap)

That way it prints to the console exactly what the function returns to the caller.

[etotheipi]

Okay, so we've had zero reaction to the latest code, either running it or in code review.  I will try to post a signed build of it shortly, but I was hoping to have one of the original reporters of the issue to review the result (for which I posted a link to the diff on github in a previous message).  We will do some internal testing, but at the moment I can't officially release it in its current state.  I'll see what we can do this week to get it out there.

P.S. - There is a 0.05 bounty per bug!   You have every reason to try it out!

P.P.S. - We found a subtanstial improvement for OSX performance that was a small change so we decided that should go into 0.92.2.  Also should have minimal impact (simply disabling AppNap, I think). 

[josephbisch]

I'm looking forward to 0.92.2 since I am waiting to package that version.

I had taken a look at the code diff on Github and did not notice any bugs. I also ran the privacy fix branch of Armory, but didn't do much (just ran it and clicked around some of the menus).