Test vectors bip70 #69

Some implementation links + test vectors

Although not standardized, there seems to be a tendency to use the .paymentrequest filename suffix for files containing BIP70 payment requests. It would make it easier to open your test vectors directly from a web browser.

Fixed

I'm not sure about the spec change at all. Are you sure you aren't just codifying some bug in the either the .NET or PHP protobuf library? The code in bitcoinj doesn't do anything special here and it verifies signatures just fine.

I have not changed the spec with this BIP, I am respecting it. Does BitcoinJ verifies without_version_details payments in the test vectors ?

If it does not, then there need some clarification to make to the BIP. What does it means "verifies signatures just fine", if the version_details is omitted on the wire ? As we talked at https://bitcointalk.org/index.php?topic=629176.20 this is a real interpretation problem we have, and I'm not sure what is intended by the creator of this BIP.

The question is : If the version_details is omitted in the wire, should it be included in the data that is signed ? From BIP70, it is since it specifies the signature include "All fields serialized in numerical order" (ie, even if version_details is omitted in the wire, the signature verification must include it)

Again, this is not a protobuf implementation bug, just a point unclear in the spec.

Does someone can put some light on this protocol ? I re explain the problem for the 10th time.

The question is simple : Does the signature of a PaymentRequest from the wire with details_version omitted is the same as the signature of a PaymentRequest from the write with details_version not omitted with default value ?

If the response is yes, then you can pull this request, if the response is no, please make the BIP clearer.

The simple answer (after writing a little test case to see):

No, the signature of a PaymentRequest from the wire with details_version omitted is NOT the same as a request with details_version explicitly set to 1 and serialized.

I'll put making the BIP clearer on my (long) TODO list...

Perfect, so my test case is wrong. I got confused with the following sentence :
"digital signature over a hash of the protocol buffer serialized variation of the PaymentRequest message, with all fields serialized in numerical order"

I need to change the test vectors

I clarified the BIP; see https://github.com/bitcoin/bips/commit/b2c0b87a9c5eac3fe7af3c7eb1655004fde19817

Excellent, I'll rebase and fix my test vectors later this week, thanks !

Thanks Gavin. The behaviour is as I thought it was.

I fixed NBitcoin, as well as the test vectors. I also added my own test certificate (+ private key) so implementers can verify if the signing process is correct.

Excellent; I'll pull as soon as I (or somebody else) has time to make sure the test vectors work with another implementation.

Question: when does the test certificate expire?

It expires in 2064

mikehearn, can you try parse it from BitcoinJ ?

Just tried most of them in bitcoinj. They fail with "java.security.cert.CertPathValidatorException: subject/issuer name chaining check failed"

Did you get the order of the chain right?

Also: If those are "vectors", what's the supposed outcome (asserts)?

Here is a printout of the cert chain to analyze:

X.509 Cert Path: length = 3. [ =========================================================Certificate 1 start. [ [ Version: V3 Subject: CN=Nicolas Dorier Signature Algorithm: SHA1withRSA, OID = 1.2.840.113549.1.1.5

Key: Sun RSA public key, 1024 bits modulus: 161683607155465416668943753903481315182858223542684446956210635897868237721237604824457500109786209274809315236072303379606790209573309211954183675785196631973482545120129722814911142426163961628316053512425138503661590965838221172088844425122708973402214894645746856968229057115842516587572634944130804939063 public exponent: 65537 Validity: [From: Mon May 28 19:03:41 CEST 2012, To: Thu May 15 19:03:41 CEST 2064] Issuer: CN=Nicolas Dorier SerialNumber: [ -01500cb9 0e]

] Algorithm: [SHA1withRSA] Signature: 0000: 74 54 87 02 E4 7A 9C 2C 1D 76 23 0D 17 5B 12 9D tT...z.,.v#..[.. 0010: A5 28 4A 86 3F 56 3D EF CF 5C 1D 36 F4 27 1D A7 .(J.?V=...6.'.. 0020: C4 74 45 ED 45 91 8B F8 1B 91 FA DE BF 4F D5 A5 .tE.E........O.. 0030: FB 92 13 90 2A EA B8 C9 D3 E5 72 E2 18 E3 5B A9 ...._.....r...[. 0040: 42 62 B5 94 50 F8 B9 8E E4 42 38 B6 C0 8F 45 1B Bb..P....B8...E. 0050: 09 A7 BD BC 5A 49 8D 4B 3A B1 9E D5 FE 08 3E 09 ....ZI.K:.....>. 0060: 03 B3 A0 F4 09 B1 6E 2A 30 D9 DA A8 8A 04 54 CE ......n_0.....T. 0070: BA 99 BB 57 4F 3A B8 6D 8B EB 73 EB 03 00 8C 5E ...WO:.m..s....^

] =========================================================Certificate 1 end.

=========================================================Certificate 2 start. [ [ Version: V3 Subject: CN=PositiveSSL CA 2, O=COMODO CA Limited, L=Salford, ST=Greater Manchester, C=GB Signature Algorithm: SHA1withRSA, OID = 1.2.840.113549.1.1.5

Key: Sun RSA public key, 2048 bits modulus: 29402787957702507317780306364213973830182532969652433670394326405162679708957794812952528186671360101402663773281638237822357109677456950883959223788097025228995086097406556923089493348439882508576598699107690681813892595724377039274811465437985619917662261355508210000071168811413336587834829995959703448097021854860923273706823604455971700624064050326599832000490169505468007975041850748683286967883290664896219004304816049616606726172078153275667450514837678595356174391089139848809252947000182466754307532753807371454385714485937701601601364936574689320282390728145208599491243491293099872729668507545052455645123 public exponent: 65537 Validity: [From: Thu Feb 16 01:00:00 CET 2012, To: Sat May 30 12:48:38 CEST 2020] Issuer: CN=AddTrust External CA Root, OU=AddTrust External TTP Network, O=AddTrust AB, C=SE SerialNumber: [ 076f1246 81459c28 d548d697 c40e001b]

Certificate Extensions: 7 [1]: ObjectId: 1.3.6.1.5.5.7.1.1 Criticality=false AuthorityInfoAccess [ [ accessMethod: caIssuers accessLocation: URIName: http://crt.usertrust.com/AddTrustExternalCARoot.p7c, accessMethod: caIssuers accessLocation: URIName: http://crt.usertrust.com/AddTrustUTNSGCCA.crt, accessMethod: ocsp accessLocation: URIName: http://ocsp.usertrust.com] ]

[2]: ObjectId: 2.5.29.35 Criticality=false AuthorityKeyIdentifier [ KeyIdentifier [ 0000: AD BD 98 7A 34 B4 26 F7 FA C4 26 54 EF 03 BD E0 ...z4.&...&T.... 0010: 24 CB 54 1A $.T. ]

]

[3]: ObjectId: 2.5.29.19 Criticality=true BasicConstraints:[ CA:true PathLen:0 ]

[4]: ObjectId: 2.5.29.31 Criticality=false CRLDistributionPoints [ [DistributionPoint: [URIName: http://crl.usertrust.com/AddTrustExternalCARoot.crl] ]]

[5]: ObjectId: 2.5.29.32 Criticality=false CertificatePolicies [ [CertificatePolicyId: [2.5.29.32.0] [] ] ]

[6]: ObjectId: 2.5.29.15 Criticality=true KeyUsage [ Key_CertSign Crl_Sign ]

[7]: ObjectId: 2.5.29.14 Criticality=false SubjectKeyIdentifier [ KeyIdentifier [ 0000: 99 E4 40 5F 6B 14 5E 3E 05 D9 DD D3 63 54 FC 62 ..@_k.^>....cT.b 0010: B8 F7 00 AC .... ] ]

] Algorithm: [SHA1withRSA] Signature: 0000: 9C 36 E3 4E AE F1 8A BB 6C 97 8C 8F 4B 67 D0 9F .6.N....l...Kg.. 0010: D8 84 AA 9F 21 5F 35 A1 5B C4 2B 63 0D E8 BC 77 ....!5.[.+c...w 0020: 5D A7 C4 37 FD 4B 2D 9E E8 1D 69 A1 C0 84 CC D1 ]..7.K-...i..... 0030: 6D 8B F3 81 CB 9F 4B 74 B0 49 2A 31 E8 37 40 EB m.....Kt.I_1.7@. 0040: 1F D9 97 A3 1A 11 D5 26 A7 6E 0F BA D5 BE 2C FD .......&.n....,. 0050: B4 91 64 DC BE 3B 19 50 0D 7A 95 F3 04 13 A9 BB ..d..;.P.z...... 0060: 47 0F 8B 5C D1 AC C2 7B 77 21 50 DD 5B AB EE F4 G......w!P.[... 0070: A6 D8 D4 4A 53 6B 4D AD B8 C8 E7 E6 52 58 4D 43 ...JSkM.....RXMC 0080: 4C C2 A2 23 4F 0E C0 20 39 AF DF 4F 42 5B 1E D3 L..#O.. 9..OB[.. 0090: 09 F4 18 09 59 2A D9 E8 4A 18 BF 32 FB FA 2D 64 ....Y..J..2..-d 00A0: 8B 87 CA 5B 2B E8 B8 0B 7E BE 17 12 C7 03 82 29 ...[+..........) 00B0: AF 58 AF 85 84 5D 3D 0A DF 23 51 C3 CD AF 10 BF .X...]=..#Q..... 00C0: 80 69 77 91 0A 4F E5 BA E1 AD 9B CE DF 33 4E 30 .iw..O.......3N0 00D0: 3B E9 8F 66 7F 82 FA 6B FA DB A3 C0 73 00 E3 D6 ;..f...k....s... 00E0: 12 AF 4D F2 0F 5A 14 51 1F 6D B8 86 81 62 07 CE ..M..Z.Q.m...b.. 00F0: 5C 72 C2 4F F3 57 2A 71 D9 D4 97 85 E6 18 53 B7 \r.O.W*q......S.

] =========================================================Certificate 2 end.

=========================================================Certificate 3 start. [ [ Version: V3 Subject: CN=AddTrust External CA Root, OU=AddTrust External TTP Network, O=AddTrust AB, C=SE Signature Algorithm: SHA1withRSA, OID = 1.2.840.113549.1.1.5

Key: Sun RSA public key, 2048 bits modulus: 23223460521213001555387570833114089730860753895612098652836669980305042236333373520864248457295413309113206184129614795458813002793321404739348347239478157361314575018921060208560207337879261682185633570259454980009323505955755478574737771466443477069914070664457159874562246026231674202013525055051064032749275350905982802892471956172353456348747163465303302884619908731908096753154575131719129358212869729511062189039464751904900614685539733488960803754548627928530841064764065036079799777523666592044408376641702595881456975606363374738747468052431570879657918206353629078434589859560212766197636528957975011023399 public exponent: 65537 Validity: [From: Thu Jun 24 20:57:21 CEST 1999, To: Mon Jun 24 21:06:30 CEST 2019] Issuer: CN=UTN - DATACorp SGC, OU=http://www.usertrust.com, O=The USERTRUST Network, L=Salt Lake City, ST=UT, C=US SerialNumber: [ 7ed1a9ab bee36f46 cd6b4e29 349056f3]

Certificate Extensions: 7 [1]: ObjectId: 1.3.6.1.5.5.7.1.1 Criticality=false AuthorityInfoAccess [ [ accessMethod: ocsp accessLocation: URIName: http://ocsp.usertrust.com] ]

[2]: ObjectId: 2.5.29.35 Criticality=false AuthorityKeyIdentifier [ KeyIdentifier [ 0000: 53 32 D1 B3 CF 7F FA E0 F1 A0 5D 85 4E 92 D2 9E S2........].N... 0010: 45 1D B4 4F E..O ]

]

[3]: ObjectId: 2.5.29.19 Criticality=true BasicConstraints:[ CA:true PathLen:2147483647 ]

[4]: ObjectId: 2.5.29.31 Criticality=false CRLDistributionPoints [ [DistributionPoint: [URIName: http://crl.usertrust.com/UTN-DATACorpSGC.crl] ]]

[5]: ObjectId: 2.5.29.32 Criticality=false CertificatePolicies [ [CertificatePolicyId: [2.5.29.32.0] [] ] ]

[6]: ObjectId: 2.5.29.15 Criticality=true KeyUsage [ Key_CertSign Crl_Sign ]

[7]: ObjectId: 2.5.29.14 Criticality=false SubjectKeyIdentifier [ KeyIdentifier [ 0000: AD BD 98 7A 34 B4 26 F7 FA C4 26 54 EF 03 BD E0 ...z4.&...&T.... 0010: 24 CB 54 1A $.T. ] ]

] Algorithm: [SHA1withRSA] Signature: 0000: 3C 25 87 28 6C 98 BD 9D 42 1C 5E 1E 64 0F 56 7F <%.(l...B.^.d.V. 0010: 96 3C F3 B9 F3 69 1A 69 F4 A4 08 D4 20 4D B1 F2 .<...i.i.... M.. 0020: 63 27 E1 9F 01 43 37 B0 B1 7A E7 71 C8 7A 21 EE c'...C7..z.q.z!. 0030: A8 35 C1 9D E6 BC 68 B3 46 80 9A 3D 04 72 3C 2F .5....h.F..=.r</ 0040: 48 FD E1 CC 42 77 0E B2 05 39 A4 00 F8 35 C2 AC H...Bw...9...5.. 0050: 78 C0 FD C7 13 BC 8B 20 4D 5A 35 AF 94 CA 32 B0 x...... MZ5...2. 0060: C6 79 D1 98 2E 3B C2 52 45 C1 9B B7 26 0B CB 04 .y...;.RE...&... 0070: 1A F6 6E 92 44 E1 7C 9C C1 12 78 A6 19 01 E2 EF ..n.D.....x..... 0080: 60 FB 7A 57 F3 32 28 06 B5 BA 1D 91 1B 28 5D 64 .zW.2(......(]d 0090: 6A 5F 53 9C 0E D5 EA CC 45 BD 7D 46 0B AF 53 49 j_S.....E..F..SI 00A0: C6 CC 80 18 5B 5B AD B1 62 13 60 4E 39 59 51 C4 ....[[..b.N9YQ. 00B0: A6 86 CB F1 0D 6D DE DD 31 0B 5F A3 07 A0 FB 3E .....m..1._....> 00C0: 46 AA 49 73 04 7D 8A 0B B0 2F 46 62 8E E1 BD 50 F.Is...../Fb...P 00D0: 65 28 B1 C8 76 4A F4 22 03 0C 55 D4 FC 0C 87 56 e(..vJ."..U....V 00E0: 7E 0B 65 EF 87 EB 7C F7 25 B7 CD 27 4C DB 3C 09 ..e.....%..'L.<. 00F0: 29 69 17 8E 8B B0 47 E8 DA 60 B7 A7 69 66 FB 0B )i....G..`..if..

] =========================================================Certificate 3 end.

]

schildbach, thanks for your help, I effectively added chain certificate to the request that should not be here. There is only the self signed certificate in the requests now. Can you try again ?

Retried payreq1_sha1.paymentrequest and payreq2_sha1.paymentrequest, doesn't change anything. They still contain 3 certs each.

Self signed certs are always rejected in BIP70 so I think it'd be better if we used a real cert, as we already do (I think) in the bitcoinj unit tests. I have a couple of SSL certs we can test with, if necessary.

Do you have the private key of such cert mike ?

Well they're private :) But for test vectors the private key should be unnecessary, all we're checking is that the signatures are verified correctly right?

schildbach, this is strange, I double checked, the requests don't have 3 certificates. I think the problem is that I stashed my changes, so your Pull might not have worked.

I'd like implementer to be able to test their signature algorithm.

Alternatively I can create a custom Certificate chain, can you deactivate trust verification in bitcoinJ ?

Yes but then what's the point? The tests are supposed to test verification, right?

If necessary we can get a free SSL cert for some throwaway test domain and put the private key into the test vectors, but I really don't think that's needed. The test data should only be checking verification works. Implementors can then make sure their signatures and cert chains are accepted by the main implementations if they have doubts that they got it right. Experience tells us that the info on what certs to include isn't right: lots of people get this wrong, especially as Bitcoin Core is quite permissive about what it allows.

No need for throwaway domain and free SSL, I can create an untrusted CA and emit a MerchantCertificate from it. I understand that testing verification is enough. I just don't see why you would not share the private key even for testing purpose, it's free.

For testing, trust checks and time validity checks should be off, so the vector don't suddenly become invalid. If you are willing to change tests to turn off these checks, then I can quickly generate a full chain. Does it sounds good ? Let's not forget that the important stuff we test in these vectors is just signature verification, not chain verification.

So far most of the bugs people have hit are in chain construction and verification actually, so it does seem important to have various combinations in the test setups.

OK, sure, we can require a custom root cert for unit tests.

Mike, I will add one test with a valid chain using Gavin's web tool. This will be usefull.

Note that in bitcoinj, there is already such a custom root cert, along with the private key.

See https://github.com/bitcoinj/bitcoinj/tree/master/core/src/test/resources/com/google/bitcoin/protocols/payments

Ok I updated everything, now all tests have a CA. As Mike said, it is good to add a test for valid chain checking, so I added one with a valid chain (thanks to gavin's payment request generator)

By testing chain checking against my own custom chain, and Gavin's chain, I've uncovered some bugs in NBitcoin Chain verification, so that was definitively a good idea ;)

Let me know if this pass correctly.

I get java.security.cert.CertPathValidatorException: Path does not chain with any of the trust anchors. Guess that's expected. Can you provide your CA cert as a JKS keystore or use the one I linked to in my last comment?

Just added a JKS with the CA inside

Now I get java.security.InvalidAlgorithmParameterException: the trustAnchors parameter must be non-empty

Can you try using the CA I linked?

Send them in PFX format and I will. The password of NicolasDorierCA is "password" if it does not work, try using nothing. The provided JKS contains the CA. (Check with keytool.exe -list -keystore NicolasDorierCA.jks) Alternatively, give me the command line so I can generate a JKS correctly. (I reused the importkeystore command from your link)

Or just remove trust check... I added a special test just for chain verification. All the other test should only test for signature verification, there is no point to check the chain for them.

Could you remove the trust check (except for the last vector that test the chain), or put the CA in a JKS store that work ? I admit I am a little lazy to change, extracting the pfx from your jks, change all my unit tests on NBitcoin, regenerate all the signatures, and then update this bip one more time, when the tests are not meant to check the trust.

I added a test vector at the end just for chain trust checking. (From Gavin's implementation)

Rebased. Can someone test these vectors without the trust check ? It would be useful to lots of people. :(

I am implementing BIP70 and really need these test vectors. Can you please confirm they are correct?

The reason we haven't verified it yet is that Java doesn't make it easy to extract the public key from the chain when that chain isn't actually valid. At least the "obvious" way I tried didn't work. And this key store doesn't seem to be valid - the cert is not marked as being a CA cert, so obviously, it's not happy (that's what the error message means).

Honestly I'm not convinced these test vectors are all that useful; we already have some test payment requests from Gavin's repo and demo site, along with "real world" tests like BitPay and Coinbase. The test vectors you produced are simply checking that protobufs works as we thought it worked and how it's documented to work - it must be your own .NET implementation that was non-conformant, so perhaps these tests are better re-cast as unit tests for whatever .NET protobuf library you're using rather than BIP70.

Mike, if I created this test it is because it was a real pain in the ass to conform just from the spec. I did that so other implementers does not have to take the minefield I took. (Precisely with how to calculate the signature)

I did all I can to help the community, my tests are not here to test the chain trust algorithm (even if I added a valid test destined to that), but for checking you sign the right thing.

I wasted hours, I would prefer other people would not. And the protobuf lib was not at fault, and was working fine. As the only implementation of the signature verification lies in NBitcoin and BitcoinJ, the only way to see if the signature verification is conform is to use BitcoinJ.

It tests some edge case that such "real world" implementation does not. And it was pure coincidence that BitcoinJ implemented right. (About the clarification Gavin's made in b2c0b87) If BitcoinJ, can't test it, and nobody wants to remove the chain trust check for signature verification, then I've done all I can.

A BIP without test vectors is dangerous.

Well, I'm indeed very sorry that this took up a lot of your time, but that doesn't change the base facts here: your protobuf library was wrong and that's where the tests should go. Actually I was wondering how it was possible that protobuf-net deviated from Google's spec API and implementation in such a basic way, but I think I see the issue. The website says:

Rather than being "a protocol buffers implementation that happens to be on .NET", it is a ".NET serializer that happens to use protocol buffers"

i.e. the fact that it appears to use the same wire format as BIP 70 is just an implementation detail. IMHO that's a very confusing design decision by the protobuf-net authors and I'm not surprised it tripped you up. I'd suggest avoiding this library in future and using something that is "a protocol buffers implementation that happens to be on .NET", otherwise you might hit other oddities in future.

BTW the reference implementation is in Bitcoin Core, not bitcoinj. I tested the bitcoinj implementation against the Bitcoin Core implementation when I wrote it.

Mike Hearn, it was NOT because of the protobuf-net implementation. The problem was the following : Do you take into account in the signature the omitted field or not. Depending on the lib you were using, by default you did... or not. But you have not consciously tried it.

That was clarified by Gavin, and this is what my tests are testing. I don't find the BIP70 implementation of Bitcoin Core. I can test against it, but I never saw it in https://github.com/bitcoin/bitcoin.git I am looking at the right repo ?

ALL protocol buffer libraries are meant to work this way and all the ones I've encountered do. Trust me on this, I was using protocol buffers almost every day for the last 7.5 years, long before they were open sourced. Look at the code generated by the reference implementations: the classes provide has_foo() accessors for optional fields. How could those ever return false if protobufs was meant to work the way you described?

The library you used even explicitly states that it's not a protocol buffers implementation at all!

Anyway, this is a pointless argument. We've now spelled out more of how protobufs work in BIP70, I feel like that is enough.

The C++ implementation is in the src/qt directory, where the paymentrequest.proto file is. Look at paymentrequestplus.cpp and the PaymentRequestPlus::getMerchant() method specifically.

Closing as there is no consensus to merge these.