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

IsSignatureValid always false #11

Open
anghiulian opened this issue May 21, 2015 · 6 comments
Open

IsSignatureValid always false #11

anghiulian opened this issue May 21, 2015 · 6 comments

Comments

@anghiulian
Copy link

Hi,

I'm testing this framework with a Sagepay test account but apparently the signature is always invalid:

if (!sagePayResponse.IsSignatureValid(sagePayPayment.SecurityKey, SagePayMvc.Configuration.Current.VendorName))

When debugging, the Hash that is generated is different from the one received from the server.

Any ideas on I might be missing?

Thanks!

@JeremySkinner
Copy link
Owner

Hi, this was a change made by SagePay in the latest version of their protocol. They've changed how their signature is generated, and have provided no documentation on this, so there's no way to verify the signature anymore.

SagePay's own integration kits no longer perform signature checking, so I'd suggest removing the check completely.

@anghiulian
Copy link
Author

Okay, thanks for the very quick reply !

@MrChriZ
Copy link

MrChriZ commented Jul 15, 2015

Digging around a bit I've found this:
http://stackoverflow.com/questions/23480263/sagepay-server-integration-verify-signature

It seems that the MD5 now takes into account
DeclineCode + ExpiryDate + FraudResponse + BankAuthCode.

@MrChriZ
Copy link

MrChriZ commented Jul 15, 2015

Just to confirm that worked.
I modified SagePayBinder.cs to add in the additional strings:
const string DeclineCode = "DeclineCode";
const string ExpiryDate = "ExpiryDate";
const string FraudResponse = "FraudResponse";
const string BankAuthCode = "BankAuthCode";

and the constructor
response.DeclineCode = GetFormField(DeclineCode, bindingContext.ValueProvider);
response.ExpiryDate = GetFormField(ExpiryDate, bindingContext.ValueProvider);
response.FraudResponse = GetFormField(FraudResponse, bindingContext.ValueProvider);
response.BankAuthCode = GetFormField(BankAuthCode, bindingContext.ValueProvider);

and then SagePayResponse.cs - 4 new properties
public string DeclineCode { get; set; }
public string ExpiryDate { get; set; }
public string FraudResponse { get; set; }
public string BankAuthCode { get; set; }

Followed by updating GenerateSignature
builder.Append(DeclineCode);
builder.Append(ExpiryDate);
builder.Append(FraudResponse);
builder.Append(BankAuthCode);

@MrChriZ
Copy link

MrChriZ commented Jul 15, 2015

Having done this I can't help but thing it's a bit redundant! I can't see that it actually provides any additional security. In fact the only thing it seems to do is check that the data you've received is correct. However given that HTTP is a quality guaranteed protocol... I'm not sure of it's purpose...
Would be curious to know otherwise!

@JeremySkinner
Copy link
Owner

I agree - it's a bit redundant. Still, I'll make the fix.

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

No branches or pull requests

3 participants