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

ParseSignedEventsWebhook is not supported on Linux/Ubuntu #377

Closed
LiorDueto opened this issue Mar 29, 2021 · 13 comments
Closed

ParseSignedEventsWebhook is not supported on Linux/Ubuntu #377

LiorDueto opened this issue Mar 29, 2021 · 13 comments
Assignees
Labels
Enhancement New feature or request
Milestone

Comments

@LiorDueto
Copy link

LiorDueto commented Mar 29, 2021

Hi,

When using StrongGrid on Ubuntu I encountered an issue with signature validation.
It seems that converting the header signature uses ECDsaCng which is Windows-specific, and raises a System.PlatformNotSupportedException on Linux machines.

the offending line:

var eCDsaCng = new ECDsaCng(cngKey);

According to the members at Azure/.Net core, this can be easily fixed using ECDsa instead of ECDsaCng.
(see dotnet/core#1918 (comment))

Is it possible to address this issue so that parsing events will support non-Windows platforms?

thanks and keep up the excellent work :)

@Jericho
Copy link
Owner

Jericho commented Mar 29, 2021

Thanks for letting me know. I'll need to investigate whether we can switch to the alternate cryptographic algorithm you suggest and still be able to validate SendGrid signature.

@Jericho Jericho self-assigned this Mar 29, 2021
@LiorDueto
Copy link
Author

Excellent.
I was able to solve this by using the starkbank-ecdsa library, and some wrapper code from SendGrid-CSharp :

  1. install starkbank-ecdsa
  2. add SendGrid's validator class (RequestValidator). it's a simple validator using the Ecdsa class
  3. use the following code to validate:
var signature = Request.Headers[RequestValidator.SIGNATURE_HEADER];
var timestamp = Request.Headers[RequestValidator.TIMESTAMP_HEADER];

// get the public key
var strongGridClient = new StrongGrid.Client(API_KEY);
var publicKey = await strongGridClient.WebhookSettings.GetSignedEventsPublicKeyAsync().ConfigureAwait(false);

// read the body
string requestBody;
using (var streamReader = new StreamReader(Request.Body))
{
   requestBody = await streamReader.ReadToEndAsync().ConfigureAwait(false);
};
 
// validate the signature
RequestValidator validator = new RequestValidator();
PublicKey pubKey = validator.ConvertPublicKeyToECDSA(publicKey);

bool verified = validator.VerifySignature(pubKey, requestBody, signature, timestamp);

I hope it helps,

@Jericho
Copy link
Owner

Jericho commented Mar 31, 2021

Do you use net5.0 by any chance? Do you think net5.0 would be a fair requirement in order to use StrongGrid's ParseSignedEventsWebhook on Linux/Ubuntu (developer running prior frameworks such as net461, netstandard2.1, etc. would continue to be able to parse signed webhooks on Windows only).

If so, I learned that built-in ECDsa cryptography classes have added support for importing public keys in recent version of .net and I could utilize that to provide Linux/Ubuntu support.

@LiorDueto
Copy link
Author

LiorDueto commented Mar 31, 2021

I don't use .Net 5.
Personally, I wouldn't consider it a fair requirement at this point (early 2021), I don't think many production-ready applications have migrated to the latest .Net version and IMO there are quite a few running on cloud-hosted Linux/Ubuntu.
Not being an expert on certificate/keys in .Net, it seems to me that the ability to import public keys in .Net 5 is more of a convenience than a neccessity.
Moreover, there are already some implementations that support .Net 3/4 on Linux/Ubuntu (for example, StarkBank's library that I mentioned and works for me), so I'd prefer these implementations over a strict requirement to use .Net 5 only because of one method in StrongGrid.

I'm quite happy with my current implementation of signature verification, it really is simple (see example above).
if you prefer not to depend on the StarkBank library, maybe you can copy the relevant code (PublicKey.cs, Utils/der.cs.. ) into StrongGrid.
StarkBank's ECDSA is under MIT license so it shouldn't be a problem (just remember the include the copyright notice).

@Jericho
Copy link
Owner

Jericho commented Mar 31, 2021

I mentioned it in my previous comment but want to repeat it in case I wasn't 100% clear: the net5.0 requirement would be for using ParseSignedEventsWebhook on Linux/Ubuntu only. The rest of the StrongGrid library would continue to be usable on Linux/Ubuntu with any other supported framework. In other words, this requirement would only affect developers who want to parse signed webhooks. I hope you didn't interpret my prior comment to mean all developers would have to upgrade to net5.0 to use StrongGrid on Ubuntu.

My reluctance to rely on the StarBank library is based on the fact that it was neglected for a long time (maybe still is, I don't know). For instance, they didn't support .NET core until last summer. I know their library is licensed under MIT and I could copy their code but, as an OSS library author myself, I wouldn't feel comfortable doing that to another OSS library author. SendGrid did that with some of my code and it infuriated me because they didn't even bother attributing the code they took. I had to ask them multiple times to attribute the code before they did so and they were pretty cavalier about the whole thing. They didn't think it was a big deal to take someone else's work. That's why, out of respect, I'm not inclined to copy/paste Starbank's code and therefore I would prefer to work with the .NET built-in cryptography classes.

I'll continue my investigation for a solution that does not require a specific .NET version.

@LiorDueto
Copy link
Author

Yes, you were perfectly clear about the .Net requirement being relevant for ParseSignedEventsWebhook on Linux/Ubuntu only. My only (very small) concern is that if SendGrid decides someday to enforce signed webhooks then it might become a little more demanding for some developers. But that is a big 'if' :)

And I totally understand, agree and respect your opinion about OSS and attribution practices. Unfortunately , it's too easy to "forget" proper attribution..

Thanks again for your efforts, they are much appreciated.

@Jericho
Copy link
Owner

Jericho commented Mar 31, 2021

Out of curiosity: which version(s) of .NET do you use?

@Jericho
Copy link
Owner

Jericho commented Apr 4, 2021

According to this article, .NET Core 2.0 used to be supported on Ubuntu but it's no longer supported and therefore you must run .NET Core 2.1 or a more recent version on Ubuntu. Therefore, the solution I am looking for must work on .NET Core 2.1 and I don't need to worry about older frameworks.

@Jericho Jericho added this to the 0.77.0 milestone Apr 4, 2021
@Jericho Jericho closed this as completed in 6c795f0 Apr 4, 2021
@LiorDueto
Copy link
Author

thank you for the quick fix!
will test it as soon as possible.

@Jericho
Copy link
Owner

Jericho commented Apr 4, 2021

I'll publish new version hopefully later today. I'm trying to setup a Ubuntu image on AppVeyor to automate unit testing but facing a few problems (you can see my progress here).

@Jericho
Copy link
Owner

Jericho commented Apr 4, 2021

I was finally able to successfully run the unit tests on Ubuntu (see here). So will be publishing 0.77.0 momentarily.

@Jericho
Copy link
Owner

Jericho commented Apr 4, 2021

🎉 This issue has been resolved in version 0.77.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

@LiorDueto
Copy link
Author

Thank you so much!

@Jericho Jericho removed this from the 0.77.0 milestone Dec 26, 2021
@Jericho Jericho added Enhancement New feature or request and removed New Feature labels Dec 26, 2021
@Jericho Jericho added this to the 0.77.0 milestone Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants