eBay / eBay-Notification-SDK-Dot-Net-Core

8 stars 8 forks source link

Signature verification does not work #2

Open axhussain opened 3 years ago

axhussain commented 3 years ago

I am unable to get VerifySignature to return true.

This could be because the payload sent from eBay is first of all, automatically deserialized by the controller, public ActionResult process([FromBody] Message message, [FromHeader(Name = "X-EBAY-SIGNATURE")] String signatureHeader),

but then serialized again in SignatureValidatorImpl, i.e. var plainTextBytes = Encoding.UTF8.GetBytes(getJSONString(message));

During this deserialize/serialize process, if a property name is changed from lowercase to PascalCase, or even a single space gets added or removed, this will prevent the signatures from matching.

SDP190 commented 3 years ago

Hi @axhussain, just my 2 cents here

You can at least try to test if this indeed has to do with deserialize/serialize by changing the code of SignatureValidatorImpl to take the raw json body string instead of passing it the Message object.

To obtain the body stream in the controller action you would have to first enable buffering. Please read this SO post for more info on that

axhussain commented 3 years ago

Thanks for the suggestion, @SDP190, however the deadline to handle eBay Account Deletion Notifications is 31st August, so I didn't have time to debug this.

As a workaround, I am simply persisting the notifications and then it'll have to be a manual process to verify that the payload actually came from eBay. :(

rbrugnollo commented 3 years ago

Thanks for the suggestion, @SDP190, however the deadline to handle eBay Account Deletion Notifications is 31st August, so I didn't have time to debug this.

As a workaround, I am simply persisting the notifications and then it'll have to be a manual process to verify that the payload actually came from eBay. :(

Same issue here, tried using the sample code but it didn't work with the payload ebay is sending to our API. Got to workaround just to be compliant with eBay.

SDP190 commented 3 years ago

Hi @rbrugnollo, can you confirm whether my suggestion above to use the actual payload does work?

rbrugnollo commented 3 years ago

@SDP190 Yes I tried using the payload, the message string would be something like the following and it didn't work...

"{\r\n \"metadata\": {\r\n \"topic\": \"MARKETPLACE_ACCOUNT_DELETION\",\r\n \"schemaVersion\": \"1.0\",\r\n \"deprecated\": false\r\n },\r\n \"notification\": {\r\n \"notificationId\": \"49feeaeb-4982-42d9-a377-9645b8479411_33f7e043-fed8-442b-9d44-791923bd9a6d\",\r\n \"eventDate\": \"2021-03-19T20:43:59.462Z\",\r\n \"publishDate\": \"2021-03-19T20:43:59.679Z\",\r\n \"publishAttemptCount\": 1,\r\n \"data\": {\r\n \"username\": \"test_user\",\r\n \"userId\": \"ma8vp1jySJC\",\r\n \"eiasToken\": \"nY+sHZ2PrBmdj6wVnY+sEZ2PrA2dj6wJnY+gAZGEpwmdj6x9nY+seQ==\"\r\n }\r\n }\r\n}"

rbrugnollo commented 3 years ago

@SDP190 I also tried cleaning the extra spaces and line breaks from it and still NO success. :/

richardcox13 commented 3 years ago

Any attempt to cryptographically sign something that does not guarantee preservation of the document being signed at a bit level is doomed to only work if you are lucky.

JSON (see RFC 8259) includes both (my emphasis):

In the introduction:

An object is an unordered collection of zero or more name/value pairs

And in the grammar:

Insignificant whitespace is allowed before or after any of the six structural characters

Ie. there is no bitwise preservation of the document across any de-serialisation/serialisation process. Depending on the library in use there may be global options that impact how any JSON is serialised, what keeps the JSON valid, but results in a different string.

SDP190 commented 3 years ago

@rbrugnollo @richardcox13 what I meant to suggest was to inspect and retrieve the actual request body ( - for which you would have to re-read the request stream - ) and use that for the signature validation, instead of relying on the Deserialized then Serialized "Message" object.

rbrugnollo commented 3 years ago

@rbrugnollo @richardcox13 what I meant to suggest was to inspect and retrieve the actual request body ( - for which you would have to re-read the request stream - ) and use that for the signature validation, instead of relying on the Deserialized then Serialized "Message" object.

@SDP190 the tests I last ran was re-reading the request stream and using the body from there. No lucky with the validation.

richardcox13 commented 3 years ago

@rbrugnollo did you read it as a string or a byte[]?

It is possible the signature being sent isn't correct (maybe depends on some internal detail of whatever platform eBay themselves use), or it could be some string level encoding issue as well?

SDP190 commented 3 years ago

If anyone here has enough time to play around, a possible way to track this down would be to test with the javascript sdk (if that one does work?) we should then try to compare and match all the code + returned values from both sdk's step by step to see where they differ.

jadanah commented 3 years ago

The refactored code I'm using in prod is here https://github.com/JoJo2406/Ebay-EventNotification-Sdk

I can make a pull request if needed https://github.com/eBay/eBay-Notification-SDK-Dot-Net-Core/issues/1

SDP190 commented 3 years ago

@JoJo2406, can you confirm that SignatureValidation from your fork works fine in your production environment with real notification requests from eBay?

jadanah commented 3 years ago

Yes I can confirm it working perfectly as the 1000's of messages I receive everyday are all validated successfully. But to be honest I don't think I had an issue with the original codebase validating messages. My changes were mainly for improved DI integration, async support, removing unrequired dependencies, .NET naming conventions/code styles and netcoreapp3.1 through to net6.0 support. I haven't really documented the changes but if you look at https://github.com/JoJo2406/Ebay-EventNotification-Sdk/tree/main/src/Ebay.EventNotification.Sdk it might help.

All you need to do is the below:

appsettings.json

{
  "EbayEventNotification": {
    "ClientCredentialsFile": "RELATIVE_PATH_TO_EBAY_CONFIG_YAML",
    "Environment": "PRODUCTION",
    "Endpoint": "https://PUBLIC_URL_OF_SERVICE/ebay-event-notification/webhook",
    "VerificationToken": "YOUR_VERIFICATION_TOKEN"
  }
}

Startup.cs

public void ConfigureServices(IServiceCollection services)
 { 
    services.AddEbayNotificationSdkServices(configuration);
    // register you own processor below is for an example
    services.RegisterEbayNotificationProcessor<AccountDeletionData, ExampleAccountDeletionMessageProcessor>(); 
 }

and it will auto register a controller for your service at https://YOUR_SERVICE_URL/ebay-event-notification/webhook

jadanah commented 3 years ago

@SDP190 if your using net5.0 or above have a look at DotnetSignatureValidator or alternatively you can use the BouncyCastleSignatureValidator version

vadzvnik commented 2 years ago

just use request stream instead message json

amirvenus commented 1 year ago

I have tried following the steps but when eBay calls my endpoint, 302 is returned

I wish there was a nice little tutorial on how to do this

amirvenus commented 1 year ago

EDIT:

I finally managed to get it working. I needed to enable anonymous authentication on the controller in .net 7.

My question is how can I receive notifications for other topics? Is creating message processor enough or should I manually subscribe to those topics?