etesync / etesync-dav

This is a CalDAV and CardDAV adapter for EteSync
https://www.etesync.com
GNU General Public License v3.0
290 stars 47 forks source link

Use self-signed leaf certificate rather than self-signed root CA #181

Closed AezNwBrZ6 closed 3 years ago

AezNwBrZ6 commented 3 years ago

etesync-dav currently generates a root CA, which could technically sign any public domain on the internet. Even though the cert is locally generated, it's a better idea to constrain the cert to a leaf cert for localhost only. So even if the local private key is compromised, say by another local application, it cannot be used to sign public domains. I cannot find any documentation where the private key is stored and whether it's stored encrypted.

Currently, every time I'm syncing I manually go to keychain to enable trust and then disable trust after sync, which is extremely tedious.

tasn commented 3 years ago

Oh, that's definitely not good! Are you sure this is the case? This is where the cert is generated from: https://github.com/etesync/etesync-dav/blob/master/etesync_dav/mac_helpers.py#L75

AezNwBrZ6 commented 3 years ago

Yes. basic_contraints = x509.BasicConstraints(ca=True, path_length=0) should be changed to basic_contraints = x509.BasicConstraints(ca=False, path_length=0)

Note that path_length=0 just means that the CA cannot issue intermediate cert, but can still issue leaf cert. Hence a chain like localhost CA -> Google.com is still valid for ca=True, path_length=0

tasn commented 3 years ago

What a silly oversight! When I sent you the link I only looked at line 75 onwards, not before. :facepalm: Thanks a lot, will fix it now and make a release.

AezNwBrZ6 commented 3 years ago

Thanks for the quick fix. Do you mind updating the commen?
# path_len=0 means this cert can only sign itself, not other certs.

path_len=0 path_length=0 just means that the CA cannot issue intermediate cert, but can still issue leaf cert. Hence a chain like localhost CA -> Google.com is still valid for ca=True, path_length=0

ca=False means this cert can only sign itself, not other certs

tasn commented 3 years ago

Done.