View Issue Details

IDProjectCategoryView StatusLast Update
0001785FreeCADBugpublic2014-10-18 13:41
ReporteryorikAssigned Toyorik 
PriorityhighSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version 
Target Version0.15Fixed in Version 
Summary0001785: Security issue - FreeCAD downloads and execute code (debian bug)
DescriptionThe Draft module downloads and executes python code when importing or exporting to the DXF file format. This raises security issues.

The problem should be fixed by:

- Authenticating the downloads by comparing SHA1 codes (see attached example)
- Popping up a dialog explaining to the user that a download will take place
Additional InformationDebian bug: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764814
TagsNo tags attached.

Activities

yorik

2014-10-13 12:32

administrator  

patch-freecad (Attachment missing)

saso

2014-10-13 18:26

reporter   ~0005214

My view on this is that while I understand and agree on the general issue, personally I see it more as a bad ("unhygienic") behavior of FC. Good software should never automatically download something from the internet unknowingly to the user, so the popup to confirm the action would definitely be the first and IMO proper fix for this issue. More, it should be reviewed that FC is not similarly misbehaving in other places/situations and make sure to keep the good standard in the future (with new code/features).

Adding hashes (sha1 is no longer recommended) to such downloads would be a plus, but I am afraid that this could represent a problem to implement it safely and at the same time not complicating the management (updating) of such files in the future. Signing the files is generally a much better way for such situations, but for now it might be a bit to much of work if the download of this few files is at the moment the only such situation in FC. If this would be a more common thing in FC, for example having an online supported management (store) of macros, materials, objects library,... then implementing such a strong security system would make much more sense.

Reviewing this current situation one has to consider that the files are hosted on GitHub, which could generally be considered as a safe place/infrastructure; they are also downloaded over https, which is again a safe way to do it (but would be good to make sure it is not possible to go around it). So the only advice I would have, beside the popup for the user, is for yorik (and possible other developers with access to the code) to use the two-factor authentication for GitHub. Doing this two things and I don't really see the need for the hash at the moment.

The other thing that is giving me a bit more worries then the download of this few files is however the start page, it pulls data from internet at every star of FC and seem to aggregate the data from different sources. I confess I did not yet look at it closely to understand how it is implemented and what would be the potential attack vectors...

Personally however, regarding security, I would be much happier to see FC active on the Coverity scan.

shoogen

2014-10-17 14:11

developer   ~0005227

I would rather use the sha1 of the according git blob object.

def blobhash(s):
    import hashlib
    hash1=hashlib.sha1("blob %d\0" % len(s))
    hash1.update(s)
    return hash1.hexdigest()

yorik

2014-10-17 14:36

administrator   ~0005228

Anyway some people on the debian alsodon't want that download anymore... I think it's best to disable DXF import/export by default andlet the user install the required lib themselves.

saso

2014-10-17 17:42

reporter   ~0005229

Just to point this out again, from security point of view it is generally nothing wrong to check the hashes of this downloaded files and indeed it can be implemented quite easy. The problem with this solution is that it would not be possible even for developers to later change this files if something would need to be updated/fixed since the hashes would be hard coded with each FC release. Updates to this files would only be possible with next FC release.

shoogen

2014-10-17 17:45

developer   ~0005230

on debian/ubuntu we could sign them using gpg or openssl ;)

saso

2014-10-17 18:03

reporter   ~0005232

Reading the Debian bug about it the main concern is from an MITM attack (at download) from not checking the https certificate, which is a valid concern. Checking https certificates, as far as I know, also has a a number of its own issues...

GnuPG also came in to my mind, having it with FC (also on Windows) would not be potentially useful for such cases, but would also open interesting possibility for users to sign / encrypt their "top secret" project files :) For developers however it would probably bring up some problems with managing the keys.

Addressing the licensing issue would be however the best solution for this case.

shoogen

2014-10-17 19:50

developer   ~0005233

i meant checking a RSA signature using the openssl progam. Not doing using SSL or PKI

saso

2014-10-17 19:57

reporter   ~0005234

A possible way to quickly implement an signing mechanism with Ed25519
just a test page to play with this crypto http://ed25519.herokuapp.com/
page of it and python implementation http://ed25519.cr.yp.to/software.html
blogpost on how it works https://blog.mozilla.org/warner/2011/11/29/ed25519-keys/

This should be safe and easy to implement. The python implementation is not the fastest but in this case we don't have to worry about that (small files, one time download). The question I guess would only be how it stands as a long term implementation and if it is even critical to make a long term decision now.

saso

2014-10-17 20:09

reporter   ~0005235

shoogen, we just posted at the same time... right, I was mostly referring to the debian bug page in my first paragraph. i thought the second suggested solution in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=764814#17 was that (to check https cert), but I don't understand why they refer to "debian certificates", I guess it would have to be githubs :\

saso

2014-10-17 23:23

reporter   ~0005236

Last edited: 2014-10-17 23:32

View 2 revisions

to show how simple this implementation is

1. from http://ed25519.cr.yp.to/software.html the only thing needed to add to FC is ed25519.py

2. the pubkey is created simple from a seed (password) like
>>> ed25519.publickey("pass") <- change that to a good password
this pubkey is then also hard coded in to FC

3. to sign a file one then simple does something like
>>> f = open('dxfReader.py', 'r')
>>> m = f.read()
>>> ed25519.signature(m, "pass", pubkey)
this signature is then attached to the end or beginning of the file or saved in an external file. the developer signs all his files with the same pubkey and seed (password)

4. on download the file is verified by simple something like
>>> ed25519.checkvalid(sig, s, pubkey)
note, if the signature is added in the file as an extra line, one has to take care that one line is not included in the s

it is secure, simple, cross platform and no extra libraries have to be added to FC. the example above is also done so that there is no need to save and manage the private key since it is auto generated every time from the seed (password). it should be however also possible to do it so that the private key is generated and saved to a file that would then need to be securely stored by the developer.

yorik

2014-10-18 01:31

administrator   ~0005237

I see your point, but there is one guy at debian panel who insist in not downloading and executing any code, even with that kind of authentication. It is the same guy who insisted on the license issues that made freecad ejected from debian at some point in time, and I don't want to go through all that story once again. So I think I'll disable by default the automatic download of the dxf lib, and allow users to enable it again (maybe via the preferences or something). I think that should satisfy him...

shoogen

2014-10-18 08:07

developer   ~0005238

Last edited: 2014-10-18 08:08

View 2 revisions

You could send Anton a patch do set the default to disabled only for the official debian packages ;)
I did the same to solve the packaging problem by using the official python-ply (instead of including it)

saso

2014-10-18 10:20

reporter   ~0005239

I don't mind, I am ok with what ever you decide to do, was happy to play around a bit with crypto :)

yorik

2014-10-18 13:30

administrator   ~0005242

Ok I think I found an elegant solution that will satisfy everybody... Automatic download is now disable by default. On first try to import or export to DXF, the user will be shown a message requiring him to either enable automatic downloads in the preference settings, or download the file manually.

So the cautious user can leave things disabled, and never have FreeCAD to download anything, and the others can just click a checkbox in the preferences, and have things work just like before...

Related Changesets

FreeCAD: master bd1bbff8

2014-10-18 15:21:26

yorik

Details Diff
Draft: Disabled automatic DXF library download - fixes 0001785

Note for existing users: Can be reenabled in menu Edit > Preferences >
Import-Export > DXF > Automatic update
Affected Issues
0001785
mod - src/Mod/Draft/Draft_rc.py Diff File
mod - src/Mod/Draft/Resources/ui/userprefs-import1.ui Diff File
mod - src/Mod/Draft/importDXF.py Diff File

Issue History

Date Modified Username Field Change
2014-10-13 12:30 yorik New Issue
2014-10-13 12:30 yorik Status new => assigned
2014-10-13 12:30 yorik Assigned To => yorik
2014-10-13 12:31 yorik Summary Security issue - FreeCAD downloads and execute code (debian bug 764814) => Security issue - FreeCAD downloads and execute code (debian bug)
2014-10-13 12:32 yorik File Added: patch-freecad
2014-10-13 18:26 saso Note Added: 0005214
2014-10-17 14:11 shoogen Note Added: 0005227
2014-10-17 14:36 yorik Note Added: 0005228
2014-10-17 17:42 saso Note Added: 0005229
2014-10-17 17:45 shoogen Note Added: 0005230
2014-10-17 18:03 saso Note Added: 0005232
2014-10-17 19:50 shoogen Note Added: 0005233
2014-10-17 19:57 saso Note Added: 0005234
2014-10-17 20:09 saso Note Added: 0005235
2014-10-17 23:23 saso Note Added: 0005236
2014-10-17 23:32 saso Note Edited: 0005236 View Revisions
2014-10-18 01:31 yorik Note Added: 0005237
2014-10-18 08:07 shoogen Note Added: 0005238
2014-10-18 08:08 shoogen Note Edited: 0005238 View Revisions
2014-10-18 10:20 saso Note Added: 0005239
2014-10-18 13:30 yorik Note Added: 0005242
2014-10-18 13:41 yorik Changeset attached => FreeCAD Master master bd1bbff8
2014-10-18 13:41 yorik Status assigned => closed
2014-10-18 13:41 yorik Resolution open => fixed