Skip to content
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

Support EdDSA signatures #187

Open
vslavik opened this issue Dec 10, 2018 · 14 comments
Open

Support EdDSA signatures #187

vslavik opened this issue Dec 10, 2018 · 14 comments

Comments

@vslavik
Copy link
Owner

vslavik commented Dec 10, 2018

(Mac) Sparkle now implements more secure signatures using EdDSA (ed25519).

See Sparkle 1.21.0 release and the bug that led to it — sparkle-project/Sparkle#1037.

This doesn't appear to be documented anywhere except the example, but the signature is stored in the appcast as base64-encoded value in sparkle::edSignature property.

On our end, we discussed this in #157 particularly in regard to DSA being difficult on Windows. Sparkle uses a small portable library for it from https://github.com/orlp/ed25519 — we could use that too or we could use win32 API if possible. In any case, it's probably worth considering doing OpenSSL-less builds too, then.

/CCing @Youw as this may be of interest to you, and as I'd definitely appreciate your view on the ease (or not) of implementing ed25519 natively with win32 crypto API.

@Youw
Copy link
Contributor

Youw commented Dec 10, 2018

Good job, Sparkle. I remember discussion regarding EdDSA was there for quite a while. Now they finally done it.

It's been some time, since I look at crypto implementations/APIs.
I'll do a deeper check later, when have more free time.

Some notes on top of my mind:

  • OpenSSL definitely supports EdDSA; but need to double-check which version, and either that support included with the version, embedded into winsparkle;
  • I have another implementation of DSA signature verification, based on Win32 Crypto API, in my win_crypto_dsa_verify branch; as far as I remember, in order to migrate from DSA to EdDSA, it is only a matter of changing provider type and provider name (compare here and here), but, most likely, it will not be supported by Windows 8 or older.

@vslavik
Copy link
Owner Author

vslavik commented Dec 11, 2018

Sorry for being unclear regarding OpenSSL: I'm wondering about ways to remove that ~500kB of code. If we make Ed25519 the default and deprecate DSA (and I think we should, just as Sparkle did), it becomes OK-ish to make DSA verification a bit more complicated, especially considering how recently we added support for it; so e.g. requiring Windows ≥ 8 or OpenSSL DLL (as you did originally) may be fine. But let's postpone this part for now.

Regarding Ed25519, I'm actually having trouble finding any reference to Ed25519, EdDSA or RFC 8032 in Microsoft's CryptoAPI or CNG docs.

@Youw
Copy link
Contributor

Youw commented Dec 11, 2018

I'm actually having trouble finding any reference to Ed25519, EdDSA or RFC 8032 in Microsoft's CryptoAPI

So do I.

in order to migrate from DSA to EdDSA, it is only a matter of changing provider type and provider name

I guess, I meant, that it would be that easy only if it is supported

@estan

This comment has been minimized.

Repository owner deleted a comment from estan Dec 20, 2018
@Youw
Copy link
Contributor

Youw commented Dec 22, 2018

I did some deeper investigation on MS Crypto API, and here is my discoveries:

  1. CryptoAPI is considered to be an "old" approach. Instead, MS recommends CNG (Cryptography API: Next Generation) - supported, starting with Windows Vista, but still, on my opinion, not been up-to-date with modern world requirements;
    Generally not a problem from technical perspective to use a new API;
  2. According to list of supported algorithms:
    • no support for EdDSA/25519;
    • DSA keys larger than 1024 bits are supported only starting with Windows 8; but here is the catch - maximum supported DSA keys are 3072 bits long.

I tried a small test app on my local (Win10) machine - nothing has changed on Windows 10. The largest supported DSA key is 3072 bits size.
This makes CNG useless for WinSparkle, since we use 4096 keys by default.

My conclusion: if we want to keep backward compatibility with version 0.6.0 - we have to keep OpenSSL around(
And use 3d-party library for EdDSA/25519 signatures.

@vslavik
Copy link
Owner Author

vslavik commented Dec 22, 2018

My conclusion: if we want to keep backward compatibility with version 0.6.0 - we have to keep OpenSSL around(

Yes, but let me rephrase for accuracy: we need to keep some implementation of our own around. I'll see (later) if we can use something more lightweight.

@puetzk

This comment has been minimized.

@pipboy96

This comment has been minimized.

@vslavik
Copy link
Owner Author

vslavik commented Apr 24, 2019

Please refrain from adding further noise to this issue by recommending libraries (the original comment already says it!) or adding comment without substance (several of which I had to delete already).

@Youw
Copy link
Contributor

Youw commented Oct 4, 2019

So I made several attempts to have an implementation for EdDSA for WinSparkle over the last few months, and I simply don't have enough free time to finish everything and prepare PR (and none of my current projects depend on this change, so I cannot squeeze this time as part of my main tasks).

If anyone explicitly waiting on my promise, please excuse me.

@vslavik
Copy link
Owner Author

vslavik commented Oct 4, 2019

If anyone explicitly waiting on my promise

Not at all, I'm just insanely busy lately myself. If you have some unfinished code that you could share (either by pushing to your fork or sharing privately or as a Work-in-Progress PR, whatever you'd be comfortable with) that would be great and I could pick it up from there.

@Youw
Copy link
Contributor

Youw commented Oct 4, 2019

I had something (just a simple test app), but need to check if I didn't lost it on a dead hard drive.

@kornelski
Copy link

I've added EdDSA support to Sparkle, and I can recommend using https://github.com/orlp/ed25519

It's small and easy to build. It's entirely self-contained, so there are no pains with system and dependency versioning. The API is super simple — I wouldn't be confident getting signing with OpenSSL right, but this library has literally sign and verify functions.

@vslavik
Copy link
Owner Author

vslavik commented May 28, 2020

@kornelski Yes, it’s even linked in the first comment – given your use in Sparkle, it’s a no brainer to use it too, despite my earlier (but not well-founded) reservations to it.

Aspirational goal is to remove the huge OpenSSL dependency altogether (or at least make it optional), so that one is definitely off the table even if its API wasn’t a minefield.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants