phax / as2-lib

A generic Java AS2 library, servlet and server
107 stars 43 forks source link

Cannot configure MDN signing #99 #104

Closed yaskovdev closed 4 years ago

yaskovdev commented 4 years ago

Fixed the issue with the MDN signing. Please review and merge if everything looks fine.

phax commented 4 years ago

@yaskovdev thanks for your effort. I disagree with your change, because signing is always done with the "sender" certificate. The goal is to prove, that the sender is really the sender. Therefore I need the sender's private key to encrypt the hash value. The receiver can check with the senders public key.

Only when encryption is used, the receivers public key is used to encrypt the message, so that only he can encrypt it.

Or did I miss something here?

yaskovdev commented 4 years ago

Thank you for the quick reply!

I believe that you are missing that we are dealing with MDN. This means that the code I have changed is running on the Receiver side. Your version of the code is doing the next thing: aCertFactory.getPrivateKey (aSenderCert), i.e., the Receiver is trying to obtain the Sender private key. This will always lead to an error since nobody knows the Sender private key, except for the Sender itself. Do you agree with that? If no, what part is wrong?

Speaking more abstractly, we are dealing with the situation when the Sender sends an AS2 message to Receiver. In response to that, the Receiver is going to sign MDN and send it back to Sender. In order to do it, the Receiver should take its own private key, sign the MDN and send the MDN to the Sender. Then Sender will take the Receiver public key and validate the signature. See, for example, the picture from this article, it illustrates the process quite clearly. In your current implementation (without my changes), the Receiver is trying to sign the MDN using the Sender private key for that, which is wrong. The Receiver never knows the Sender private key.

phax commented 4 years ago

Assume Alice is sending a message to Bob, and Bob respondes with a signed MDN:

And if you now look at line 188, we're looking up Bob's private key (because "Bob" is the "Sender" of the MDN) and that is exactly what signing should do. What may be confusing here (and yes, it's bad design), is that "getPrivateKey" in https://github.com/phax/as2-lib/blob/master/as2-lib/src/main/java/com/helger/as2lib/cert/CertificateFactory.java#L457 re-searches the keystore. So it is not inferring the private key from something.

And that's why I still believe ECertificatePartnershipType.SENDER is correct.

yaskovdev commented 4 years ago

Did not notice that the swap of aliases happens before the MDN sending, sorry. But still there is an issue, I believe. Here's what I am trying to do.

I am using the same Alice and Bob implementation as I used to reproduce the https://github.com/phax/as2-lib/issues/99 issue.

If I send the AS2 message from A (Alice) to B (Bob) and put the breakpoint on Bob's side in the AS2Helper class on the 269 line, then before execution of the line I can see that:

aMDN.partnership().getSenderX509Alias() -> B aMDN.partnership().getReceiverX509Alias() -> A

, i.e., before sending the MDN, the sender and receiver keystore aliases were successfully swapped, as you described.

But right after execution of the line I can see that:

aMDN.partnership().getSenderX509Alias() -> A aMDN.partnership().getReceiverX509Alias() -> B

, i.e., something happened on the line 269 which discarded the logic from lines 264 + 265.

By the way, if I take your code, but temporary comment out the 269 line, then everything starting working well, i.e., the https://github.com/phax/as2-lib/issues/99 issue does not happen anymore. :)

phax commented 4 years ago

Okay, we're getting closer :) What implementation of IPartnershipFactory are you using? And what is the underlying data for that particular partnership?

yaskovdev commented 4 years ago

I am using the simplest possible configuration for Bob, in particular, I am using SelfFillingPartnershipFactory. Correct me if I am wrong, but this implementation uses only data from the AS2 message itself to build the partnership in runtime.

phax commented 4 years ago

Okay, the SelfFillingPartnershipFactory is a simple, non-persisting one ideal for dynamic "on the fly connections". It uses the "updatePartnership" implementation of https://github.com/phax/as2-lib/blob/master/as2-lib/src/main/java/com/helger/as2lib/partner/AbstractPartnershipFactory.java

In that case https://github.com/phax/as2-lib/blob/master/as2-lib/src/main/java/com/helger/as2lib/partner/AbstractPartnershipFactory.java#L86 does the lookup of an existing partnership is already present. That uses the partnership name as the primary source, and my assumption is that another partnership with the name Partnership.DEFAULT_NAME is already present, but that comes from a different connection. And therefore all properties are incorrectly overridden.

I had a fix in AS2ServletPartnershipFactory for the double name. I now unified this for SelfFillingPartnershipFactory and SelfFillingXMLPartnershipFactory. Can you please test the SNAPSHOT 4.5.4 once https://travis-ci.org/github/phax/as2-lib/builds/661061997 is done?

yaskovdev commented 4 years ago

Tested with your changes, works well now! I think this PR can be deleted then and https://github.com/phax/as2-lib/issues/99 is also resolved. :)

phax commented 4 years ago

Thanks for retesting - that was a hard one ;-) I will create a new 4.5.4 release asap