Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Mon, Jan 26, 2015 at 10:35 AM, Gregory Maxwell gmaxw...@gmail.com wrote: I'd like to request a BIP number for this. Sure. BIP0066. Four implementations exist now: * for master: https://github.com/bitcoin/bitcoin/pull/5713 (merged) * for 0.10.0: https://github.com/bitcoin/bitcoin/pull/5714 (merged, and included in 0.10.0rc4) * for 0.9.4: https://github.com/bitcoin/bitcoin/pull/5762 * for 0.8.6: https://github.com/bitcoin/bitcoin/pull/5765 The 0.8 and 0.9 version have reduced test code, as many tests rely on new test framework code in 0.10 and later, but the implementation code is identical. Work to improve that is certainly welcome. -- Pieter -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
Could we see a PR that adds it to BIP 66? Perhaps we'd all agree quickly that its so simple we can just add it... In either case it doesn't seem strictly necessary to me that it was non-standard before it becomes a soft-fork... On Tue, Feb 3, 2015 at 7:00 AM, Wladimir laa...@gmail.com wrote: One way to do that is to just - right now - add a patch to 0.10 to make those non-standard. This requires another validation flag, with a bunch of switching logic. The much simpler alternative is just adding this to BIP66's DERSIG right now, which is a one-line change that's obviously softforking. Is anyone opposed to doing so at this stage? Not opposed, but is kind of late for 0.10, I had hoped to tag rc4 today. Wladimir -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
One way to do that is to just - right now - add a patch to 0.10 to make those non-standard. This requires another validation flag, with a bunch of switching logic. The much simpler alternative is just adding this to BIP66's DERSIG right now, which is a one-line change that's obviously softforking. Is anyone opposed to doing so at this stage? Not opposed, but is kind of late for 0.10, I had hoped to tag rc4 today. Wladimir -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Tue, Feb 3, 2015 at 4:00 AM, Wladimir laa...@gmail.com wrote: One way to do that is to just - right now - add a patch to 0.10 to make those non-standard. This requires another validation flag, with a bunch of switching logic. The much simpler alternative is just adding this to BIP66's DERSIG right now, which is a one-line change that's obviously softforking. Is anyone opposed to doing so at this stage? Not opposed, but is kind of late for 0.10, I had hoped to tag rc4 today. I understand it's late, which is also why I ask for opinions. It's also not a priority, but if we release 0.10 without, it will first need a cycle of making this non-standard, and then in a further release doing a second softfork to enforce it. It's a 2-line change; see #5743. -- Pieter -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
I think we should just do it, and include it with the other DERSIG changes for 0.10. On Tue, Feb 3, 2015 at 1:15 PM, Pieter Wuille pieter.wui...@gmail.com wrote: I understand it's late, which is also why I ask for opinions. It's also not a priority, but if we release 0.10 without, it will first need a cycle of making this non-standard, and then in a further release doing a second softfork to enforce it. It's a 2-line change; see #5743. -- Pieter -- -- Gavin Andresen -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
+1 I just ran an it-works test on #5743. Not exhaustive, but I do agree it should be included w/ other DERSIG changes. On Tue, Feb 3, 2015 at 1:19 PM, Gavin Andresen gavinandre...@gmail.com wrote: I think we should just do it, and include it with the other DERSIG changes for 0.10. On Tue, Feb 3, 2015 at 1:15 PM, Pieter Wuille pieter.wui...@gmail.com wrote: I understand it's late, which is also why I ask for opinions. It's also not a priority, but if we release 0.10 without, it will first need a cycle of making this non-standard, and then in a further release doing a second softfork to enforce it. It's a 2-line change; see #5743. -- Pieter -- -- Gavin Andresen -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development -- Jeff Garzik Bitcoin core developer and open source evangelist BitPay, Inc. https://bitpay.com/ -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Tue, Feb 3, 2015 at 10:15 AM, Pieter Wuille pieter.wui...@gmail.com wrote: The much simpler alternative is just adding this to BIP66's DERSIG right now, which is a one-line change that's obviously softforking. Is anyone opposed to doing so at this stage? I'm retracting this proposed change. Suhar Daftuas pointed out that there remain edge-cases which are not covered (a 33-byte R or S whose first byte is not a zero). The intent here is really making sure that signature validation and parsing can be entirely separated, and that signature checking itself does not need a third return value (invalid encoding, in addition to valid signature and invalid signature). If we don't want to make assumptions about how that implementation works, the only guaranteed way of doing that is requiring that R and S are in fact within the range allowed by secp256k1, which would require an integer decoder inside the signature encoding checker. I consider that to be unreasonable. In addition, a much cleaner solution that covers this as well has already been proposed: only allow 0 (the empty byte vector) as invalid signature. That would 100% align signature validity with decoding, and is much simpler to implement. -- Pieter -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Sun, Jan 25, 2015 at 6:48 AM, Gregory Maxwell gmaxw...@gmail.com wrote: So I think we should just go ahead with R/S length upper bounds as both IsStandard and in STRICTDER. I would like to fix this at some point in any case. If we want to do that, we must at least have signatures with too-long R or S values as non-standard. One way to do that is to just - right now - add a patch to 0.10 to make those non-standard. This requires another validation flag, with a bunch of switching logic. The much simpler alternative is just adding this to BIP66's DERSIG right now, which is a one-line change that's obviously softforking. Is anyone opposed to doing so at this stage? -- Pieter -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Tue, Feb 3, 2015 at 12:44 AM, Pieter Wuille pieter.wui...@gmail.com wrote: The much simpler alternative is just adding this to BIP66's DERSIG right now, which is a one-line change that's obviously softforking. Is anyone opposed to doing so at this stage? Thats my preference. -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Mon, 26 Jan 2015, Gregory Maxwell wrote: On Mon, Jan 26, 2015 at 5:14 AM, Pieter Wuille pieter.wui...@gmail.com wrote: On Tue, Jan 20, 2015 at 8:35 PM, Pieter Wuille pieter.wui...@gmail.com wrote: I therefore propose a softfork to make non-DER signatures illegal (they've been non-standard since v0.8.0). A draft BIP text can be found on: https://gist.github.com/sipa/5d12c343746dad376c80 I'd like to request a BIP number for this. Sure. BIP0066. There was also some feedback on Bitcointalk, which I think you've addressed Progress information for the list: there is now a pull request implementing the strict DER verification behavior, as well as the deployment specified in BIP66 for Bitcoin Core. It needs your review and testing: https://github.com/bitcoin/bitcoin/pull/5713 Wladimir -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Mon, Jan 26, 2015 at 5:14 AM, Pieter Wuille pieter.wui...@gmail.com wrote: On Tue, Jan 20, 2015 at 8:35 PM, Pieter Wuille pieter.wui...@gmail.com wrote: I therefore propose a softfork to make non-DER signatures illegal (they've been non-standard since v0.8.0). A draft BIP text can be found on: https://gist.github.com/sipa/5d12c343746dad376c80 I'd like to request a BIP number for this. Sure. BIP0066. There was also some feedback on Bitcointalk, which I think you've addressed: https://bitcointalk.org/index.php?topic=932054.0 I also had off-list positive feedback from Amir Taak, so we have positive feedback from several implementers. One of the points that was raised which we'd discussed pre-proposal that was brought up there that I thought I should summarize here was the possibility that someone had previously authored an nlocked spend with an invalidly encoded signature. In those cases the signature can just be mutated to get it mined, and would need to be already to pass IsStandard rules. A case that isn't covered if if they have a chain of transactions after that nlocked transaction, but those cases would already be at extreme risk of malleability (esp since their unchanged form is non-standard), and that coupled with the fact that avoiding this would undermine the intent of the BIP (independence from a specific encoding scheme) seems to have been convincing as much. -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Thu, Jan 22, 2015 at 6:41 PM, Zooko Wilcox-OHearn zo...@leastauthority.com wrote: * Should the bipstrictder give a rationale or link to why accept the 0-length sig as correctly-encoded-but-invalid? I guess the rationale is an efficiency issue as described in the log entry for https://github.com/sipa/bitcoin/commit/041f1e3597812c250ebedbd8f4ef1565591d2c34 I've lately been updating the BIP text without updating the code in the repository; I've synced them now. The sigsize=0 case was actually already handled elsewhere already, so I removed the code and added a comment about it now in the BIP text. * Does this mean there are still multiple ways to encode a correctly encoded but invalid signature, one of which is the 0-length string? Would it make sense for this change to also treat any *other* correctly-encoded-but-invalid sig (besides the 0-length string) as incorrectly-encoded? Did I just step in some BIP62? You didn't miss anything; that's correct. In fact, Peter Todd already pointed out the possibility of making non-empty invalid signatures illegal. The reason for not doing it yet is that I'd like this BIP to be minimal and uncontroversial - it's a real problem we want to fix as fast as is reasonable. It wouldn't be hard to make this a standardness rule though, and perhaps later softfork it in as consensus rule if there was sufficient agreement about it. * It would be good to verify that all the branches of the new IsDERSignature() from https://github.com/sipa/bitcoin/commit/0c427135151a6bed657438ffb2e670be84eb3642 are tested by the test vectors in https://github.com/sipa/bitcoin/commit/f94e806f8bfa007a3de4b45fa3c9860f2747e427 . Eyeballing it, there are about 20 branches touched by the patch, and about 24 new test vectors. A significiant part of DERSIG behaviour (which didn't change, only the cases in which it is enforced) was already tested, in fact. Some branches remained untested however; I've added extra test cases in the repository. They give 100% coverage for IsValidSignatureEncoding (the new name for IsDERSignature) now (tested with gcov). * It would be good to finish the TODOs in https://github.com/sipa/bitcoin/commit/b7986119a5d41337fea1e83804ed6223438158ec so that it was actually testing the upgrade behavior. I agree, but that requires very significant changes to the codebase, as we currently have no way to mine blocks with non-acceptable transactions. Ideally, the RPC tests gain some means of building/mining blocks from without the Python test framework. Things like that would make the code changes also hard to backport, which we definitely will need to do to roll this out quickly. * missing comment: https://github.com/sipa/bitcoin/commit/e186f6a80161f9fa45fbced82ab1d22f081b942c#commitcomment-9406643 Fixed. Okay, that's all I've got. Hope it helps! Thanks again for your good work! Thanks! -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Tue, Jan 20, 2015 at 8:35 PM, Pieter Wuille pieter.wui...@gmail.com wrote: I therefore propose a softfork to make non-DER signatures illegal (they've been non-standard since v0.8.0). A draft BIP text can be found on: https://gist.github.com/sipa/5d12c343746dad376c80 I'd like to request a BIP number for this. -- Pieter -- Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Wed, Jan 21, 2015 at 8:32 PM, Rusty Russell ru...@rustcorp.com.au wrote: One weirdness is the restriction on maximum total length, rather than a 32 byte (33 with 0-prepad) limit on signatures themselves. Glad that you point this out; I believe that's a weakness with more impact now that this function is used for consensus. Let me clarify. This function was originally written for Bitcoin Core v0.8.0, where it was only used to enforce non-standardness, not consensus. In that setting, there was no need to require a maximum length for the R and S arguments, as overly-long R or S values (which, because of a further rule, do not have excessive padding) will always result in integers = 2^256, which means the encoded signature would never be valid according to the ECDSA specification. A restriction on the total length is required however, as BER allows multi-byte length descriptors, which this function cannot (and shouldn't, as it's not DER) parse. However, in the currently proposed soft fork, non-DER results in immediate script failure, which is distinguishable from invalid signatures (by negating the result of a CHECKSIG, for example using a NOT after it). I must admit that having invalid signatures with overly-long R or S but acceptable R+S size be distinguishable from invalid signatures where R+S is too large is ugly, and unnecessary. Adding individual R and S length restrictions (ideally: saying that no more than 32 bytes, excluding the padding 0 byte in front, is invalid) would be trivial, but it means deviating slightly from the standardness rule implementation that has been deployed for a while. There should not really be much risk in doing so, as there are still no node implementation releases (apart from the v0.10.0 rc's) that would mine a CHECKSIG whose result is negated. So, I think there are two options: * Just add this R/S length restriction rule as a standardness requirement, but not make it part of the soft fork. A later softfork can then add this easily. The same can be done for several other changes if they are deemed useful, like only allowing 0 (the empty array) as invalid signature (any other causes failure script immediately), requiring correct encoding even for non-evaluated signatures, ... * Add it to the softfork now, and be done with it. Opinions? -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
.Hi there. Thank you for your work on this. I've looked over https://gist.github.com/sipa/5d12c343746dad376c80 and https://github.com/sipa/bitcoin/commit/bipstrictder . I didn't actually audit the included reference implementation of IsValidSignatureEncoding(), and I didn't check whether the test vectors in https://github.com/sipa/bitcoin/commit/f94e806f8bfa007a3de4b45fa3c9860f2747e427 exercise all of the branches that are changed by this patch. I have the following comments: * It seems like a good idea to do this. * I don't see any problem with using the upgrade mechanism from BIP 34 for this. It's cool! I'm happy that such a mechanism seems to work in practice. * Should the bipstrictder give a rationale or link to why accept the 0-length sig as correctly-encoded-but-invalid? I guess the rationale is an efficiency issue as described in the log entry for https://github.com/sipa/bitcoin/commit/041f1e3597812c250ebedbd8f4ef1565591d2c34 . * Does this mean there are still multiple ways to encode a correctly encoded but invalid signature, one of which is the 0-length string? Would it make sense for this change to also treat any *other* correctly-encoded-but-invalid sig (besides the 0-length string) as incorrectly-encoded? Did I just step in some BIP62? * It would be good to verify that all the branches of the new IsDERSignature() from https://github.com/sipa/bitcoin/commit/0c427135151a6bed657438ffb2e670be84eb3642 are tested by the test vectors in https://github.com/sipa/bitcoin/commit/f94e806f8bfa007a3de4b45fa3c9860f2747e427 . Eyeballing it, there are about 20 branches touched by the patch, and about 24 new test vectors. * It would be good to finish the TODOs in https://github.com/sipa/bitcoin/commit/b7986119a5d41337fea1e83804ed6223438158ec so that it was actually testing the upgrade behavior. * missing comment: https://github.com/sipa/bitcoin/commit/e186f6a80161f9fa45fbced82ab1d22f081b942c#commitcomment-9406643 Okay, that's all I've got. Hope it helps! Thanks again for your good work! Regards, Zooko -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 2015/1/20 19:35, Pieter Wuille wrote: Hello everyone, Comments/criticisms are very welcome, but I'd prefer keeping the discussion here on the mailinglist (which is more accessible than on the gist). Nice paper, Pieter. I do have a bit of feedback. 1)The first sentence of Deployment has a typo. We reuse the double-threshold switchover mechanism from BIP 34, with the same *thresholds*, [] 2)I think the handling of the sighash byte in the comments of IsDERSignature() could use a little tweaking. If you look at CheckSignatureEncoding() in the actual code (src/script/interpreter.cpp in master), it's clear that the sighash byte is included as part of the signature struct, even though it's not part of the actual DER encoding being checked by IsDERSignature(). This is fine. I just think that the code comments in the paper ought to make this point clearer, either in the sighash description, or as a comment when checking the sig size (i.e., size-3 is valid because sighash is included), or both. 3)The paper says a sig with size=0 is correctly coded but is neither valid nor DER. Perhaps this code should be elsewhere in the Bitcoin code? It seems to me that letting a sig pass in IsDERSignature() when it's not actually DER-encoded is incorrect. Thanks. - --- Douglas Roark Senior Developer Armory Technologies, Inc. d...@bitcoinarmory.com PGP key ID: 92ADC0D7 -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) Comment: GPGTools - https://gpgtools.org iQIcBAEBCgAGBQJUv/4vAAoJEGybVGGSrcDXMxkP/1N2lLAloCKdRUpMBLPEZ5jh bJ4reCeqrMy6JetsKSGfGKdAe7kGkeRl6s8dlHYnpUmnODXU9BCku3zHi3+qm8IC GZlwSdSSgmRneP7btPula0CG31o7X2UJiDW/2IOZl6ul8b7LB2L56O+Ew+PNm+at tCfRcpKtq9LYCnRYR0azd4c5YY9/o7zlkpGi8CututzuEa4Rcm92U1extoo2tC/j nzUfbfcQVL0a7JaRU4VYNceYrcG/xSpKPjsEU/F+5IwnUxL/kebz0EDt1kzm+fOE EMUMXyYgoyW5VDFNjxu00PnJUfVNCOXN/N/h9eCdskCL3AtH6xg1kzam5OGvpEZS QDMNSmQl4Zpx5WiATylNkhhzb/8GowamkSFg4SUjBsjpwOTMTIF0Qhnt+DdzwpI2 etxCGds154nL4p/bkulseczwxOZWin9oZxJnCxp40oFl8fva0BwHVx45uMyI61Ko qRJ9Ol0CDoId3h1EMTt4uyoNxrOzgrj8/+V4BBytOAMMmsfD0VgY68xzdywJxYnC jgU99huhwtJpn9QT6JAbgPAaboomu6hDCohV+J+DCCkIiYFk1jxp+FQ4xZDzcKeo gMYpmFefPAxnHvDXf1v1A+Xw8plN6/NREaIpprh7Ep+q/8vYAiwwHfKjubdMkB3D WnTR5YbqyGxc/Pvh9Ncq =C/wj -END PGP SIGNATURE- -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Tue, Jan 20, 2015 at 11:45 PM, Rusty Russell ru...@rustcorp.com.au wrote: // Null bytes at the start of R are not allowed, unless it would otherwise be // interpreted as a negative number. if (lenS 1 (sig[lenR + 6] == 0x00) !(sig[lenR + 7] 0x80)) return false; You mean null bytes at the start of S. Thanks, fixed. -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
I've read this and it looks A-OK to me. Andrew On Tue, Jan 20, 2015 at 07:35:49PM -0500, Pieter Wuille wrote: Hello everyone, We've been aware of the risk of depending on OpenSSL for consensus rules for a while, and were trying to get rid of this as part of BIP 62 (malleability protection), which was however postponed due to unforeseen complexities. The recent evens (see the thread titled OpenSSL 1.0.0p / 1.0.1k incompatible, causes blockchain rejection. on this mailing list) have made it clear that the problem is very real, however, and I would prefer to have a fundamental solution for it sooner rather than later. I therefore propose a softfork to make non-DER signatures illegal (they've been non-standard since v0.8.0). A draft BIP text can be found on: https://gist.github.com/sipa/5d12c343746dad376c80 The document includes motivation and specification. In addition, an implementation (including unit tests derived from the BIP text) can be found on: https://github.com/sipa/bitcoin/commit/bipstrictder Comments/criticisms are very welcome, but I'd prefer keeping the discussion here on the mailinglist (which is more accessible than on the gist). -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development -- Andrew Poelstra Mathematics Department, University of Texas at Austin Email: apoelstra at wpsoftware.net Web: http://www.wpsoftware.net/andrew If they had taught a class on how to be the kind of citizen Dick Cheney worries about, I would have finished high school. --Edward Snowden pgpgbq38zIFUD.pgp Description: PGP signature -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 2015/1/21 15:30, Pieter Wuille wrote: Thanks for the comments. I hope I have clarified the text a bit accordingly. You're welcome. All the revisions look good to me. - --- Douglas Roark Senior Developer Armory Technologies, Inc. d...@bitcoinarmory.com PGP key ID: 92ADC0D7 -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) Comment: GPGTools - https://gpgtools.org iQIcBAEBCgAGBQJUwA6WAAoJEGybVGGSrcDXvmEP/A09j4lq2P0RMqrvtwnDQRmH oimbGwC2a/BbpACBegn0cdFYMURFFcec4gHKyvuN7xR4SRsgQ+Djq/KranAMkYbs ZQVFGXRWdZhfsh7bY4zbBUj+H8c8PAsKL0D7S8r4iXviuUimXJXqESUYote9Ylz3 rwjiK3oRiCSMpTMiI3eDjrbQt5HHLw3hKL7W6zTerx64eCaO2JsIn/Pk4Krf9xwd 1ejpyqrK/9s90NPB0Qqieqbgg7WoQYP+ZMzFi5oNxtNrZjlOCNSQKLN0IXqnnMnS +AoB4B5TUGCdLq3Wlo69mhLaLYNaPNHEoGNUwikXqsd5WeqsayuYDl36rI4MLWgB ZBVO6D2BErqdqMTrmUEurubXMb6CCAuFu6iYjO3vucQ0l+7xD7OW/XiK7ZPNFuwj 2fJCjRHjqgDwKlIUF3Gh7BwRrT2iZRoFYWXDVRBMiJpHvs1+U79pQENp4BmQLWE+ xn3gX9r755mVDJL10MFM6jKijgTCGA2hEFjK2Vu1JJMeVSIGaOdEIen2DxS2mqnZ b/t9VDxfbFQRw5pj2zHsvFDGBe7DEhvBSqbNtiPrY5/LITeP8Nt4CZ9PHrYPJV5A ocUx98l1sqy7P0QiYzAEp5tpdjTS17MVNPt84JLJnk7wL+fDRfKKV3A7tI/ziFJe hjW91YNTIrs+ZFLV/HJc =Rjcd -END PGP SIGNATURE- -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Wed, Jan 21, 2015 at 3:37 PM, Gavin Andresen gavinandre...@gmail.com wrote: DERSIG BIP looks great to me, just a few nit-picky changes suggested: You mention the DER standard : should link to http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf (or whatever is best reference for DER). this would simplify avoiding OpenSSL in consensus implementations -- this would make it easier for non-OpenSSL implementations causing opcode failure : I know what you mean by opcode failure, but it might be good to be more explicit. since v0.8.0, and nearly no transactions -- and very few transactions... reducing this avenue for malleability is useful on itself as well : awkward English. How about just This proposal has the added benefit of reducing transaction malleability (see BIP62). Nit addressed, hopefully. -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
-BEGIN PGP SIGNED MESSAGE- Hash: SHA512 On 2015/1/21 15:37, Gavin Andresen wrote: You mention the DER standard : should link to http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf (or whatever is best reference for DER). The link you gave is to the 2002 revision. http://www.itu.int/rec/T-REC-X.690-200811-I/en has the latest revision (Nov. 2008) and, AFAIK, is the most visible link to people searching for X.690. That said, X.690 is the definitive DER document (if not exactly the easiest read). A link to it wouldn't hurt. this would simplify avoiding OpenSSL in consensus implementations -- this would make it easier for non-OpenSSL implementations causing opcode failure : I know what you mean by opcode failure, but it might be good to be more explicit. since v0.8.0, and nearly no transactions -- and very few transactions... reducing this avenue for malleability is useful on itself as well : awkward English. How about just This proposal has the added benefit of reducing transaction malleability (see BIP62). These all look good to me. - --- Douglas Roark Senior Developer Armory Technologies, Inc. d...@bitcoinarmory.com PGP key ID: 92ADC0D7 -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.22 (Darwin) Comment: GPGTools - https://gpgtools.org iQIcBAEBCgAGBQJUwBF8AAoJEGybVGGSrcDXBxcP/j9dKIeXkOvDFgSzON2hmjxT nzpPcxovGt+ds1KqHMtuMm8+Mmc/Z8kOhKWzgQKYlxq8eQayQ4X/DUr97IY248NX udVM6vEp/azPkXLOQnO6POpv8Il6twyuYGvFAHLiYe9k9qMfdSKZetx5xFKVBsuj DhRY2TnWC7/OXNUrT7H5TPHDaGHyXeJ47XSOVjGQ/qxdczIzvmt11amZ/Vn2+uXh Rvz+0CzbpXYaqYB04ZnIv5lxknmjWGbxPdht/SoOly8INehQacWnwUNZJpilKb6x qEpbDGNxW2zHEFgfNHmtr9PCBN8KyiVnTt+VZpNNl7PJCxZiK6uiwyNxsmOBhBtm Hrsvxb9GqEO/6PKesEo+Hi+6hhzzQRC6Xrf85SaFMzw9UjKuuRhstxx7XhudKFkN lBJcxd40G7kWk0Gv+YQmhFUyXUBqloEFGrFlzWniFKaJGzZs5D0JPd83DsPI4RuT 0M63YabL8qplYN8vnyUXabFpzglvQdAFqZS2GsO6zwAeWrqxsojpcEpikj4T+izR W1TzaRDdm5pEaMMxvb6wFIgO32uAjN1a8GrRj+uk5cxuiOuk/C4Ii18FYhqEtDNd Gv80rPxWEOxbCoSqH6igPnySw3ePFLBzgC4eSLBTnqfKYltd8fTeS9wGy47+L1YO qb5K/xlqt+REOdbTGLHi =MNXG -END PGP SIGNATURE- -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
DERSIG BIP looks great to me, just a few nit-picky changes suggested: You mention the DER standard : should link to http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf (or whatever is best reference for DER). this would simplify avoiding OpenSSL in consensus implementations -- this would make it easier for non-OpenSSL implementations causing opcode failure : I know what you mean by opcode failure, but it might be good to be more explicit. since v0.8.0, and nearly no transactions -- and very few transactions... reducing this avenue for malleability is useful on itself as well : awkward English. How about just This proposal has the added benefit of reducing transaction malleability (see BIP62). -- -- Gavin Andresen -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Wed, Jan 21, 2015 at 2:29 PM, Douglas Roark d...@bitcoinarmory.com wrote: Nice paper, Pieter. I do have a bit of feedback. Thanks for the comments. I hope I have clarified the text a bit accordingly. 1)The first sentence of Deployment has a typo. We reuse the double-threshold switchover mechanism from BIP 34, with the same *thresholds*, [] Fixed. 2)I think the handling of the sighash byte in the comments of IsDERSignature() could use a little tweaking. If you look at CheckSignatureEncoding() in the actual code (src/script/interpreter.cpp in master), it's clear that the sighash byte is included as part of the signature struct, even though it's not part of the actual DER encoding being checked by IsDERSignature(). This is fine. I just think that the code comments in the paper ought to make this point clearer, either in the sighash description, or as a comment when checking the sig size (i.e., size-3 is valid because sighash is included), or both. I've renamed the function to IsValidSignatureEncoding, as it is not strictly about DER (it adds a Bitcoin-specific byte, and supports and empty string too). 3)The paper says a sig with size=0 is correctly coded but is neither valid nor DER. Perhaps this code should be elsewhere in the Bitcoin code? It seems to me that letting a sig pass in IsDERSignature() when it's not actually DER-encoded is incorrect. I've expanded the comments about it a bit. -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
I'm really glad to see this proposal. We already treat non-DER signatures as non-standard in btcd and agree that extending them be illegal as a part of a soft fork is a smart and sane thing to do. It's also good to see the explicit use of signature parsing since it matches what we already do as well because we noticed noticed OpenSSL's notion of big numbers (unsigned) didn't agree with Go's (signed). By having the explicit signature scheme and checking clearly called out in a BIP, it greatly lowers the chances of there being any disagreement about what is valid or invalid due to an underlying dependency. +1 On 1/20/2015 6:35 PM, Pieter Wuille wrote: Hello everyone, We've been aware of the risk of depending on OpenSSL for consensus rules for a while, and were trying to get rid of this as part of BIP 62 (malleability protection), which was however postponed due to unforeseen complexities. The recent evens (see the thread titled OpenSSL 1.0.0p / 1.0.1k incompatible, causes blockchain rejection. on this mailing list) have made it clear that the problem is very real, however, and I would prefer to have a fundamental solution for it sooner rather than later. I therefore propose a softfork to make non-DER signatures illegal (they've been non-standard since v0.8.0). A draft BIP text can be found on: https://gist.github.com/sipa/5d12c343746dad376c80 The document includes motivation and specification. In addition, an implementation (including unit tests derived from the BIP text) can be found on: https://github.com/sipa/bitcoin/commit/bipstrictder Comments/criticisms are very welcome, but I'd prefer keeping the discussion here on the mailinglist (which is more accessible than on the gist). signature.asc Description: OpenPGP digital signature -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
Pieter Wuille pieter.wui...@gmail.com writes:
Hello everyone,
We've been aware of the risk of depending on OpenSSL for consensus
rules for a while, and were trying to get rid of this as part of BIP
62 (malleability protection), which was however postponed due to
unforeseen complexities. The recent evens (see the thread titled
OpenSSL 1.0.0p / 1.0.1k incompatible, causes blockchain rejection.
on this mailing list) have made it clear that the problem is very
real, however, and I would prefer to have a fundamental solution for
it sooner rather than later.
OK, I worked up a clearer (but more verbose) version with fewer
magic numbers. More importantly, feel free to steal the test cases.
One weirdness is the restriction on maximum total length, rather than a
32 byte (33 with 0-prepad) limit on signatures themselves.
Apologies for my babytalk C++. Am sure there's a neater way.
/* Licensed under Creative Commons zero (public domain). */
#include vector
#include cstdlib
#include cassert
#ifdef CLARIFY
bool ConsumeByte(const std::vectorunsigned char sig, size_t off,
unsigned int val)
{
if (off = sig.size()) return false;
val = sig[off++];
return true;
}
bool ConsumeTypeByte(const std::vectorunsigned char sig, size_t off,
unsigned int t)
{
unsigned int type;
if (!ConsumeByte(sig, off, type)) return false;
return (type == t);
}
bool ConsumeNonZeroLength(const std::vectorunsigned char sig, size_t off,
unsigned int len)
{
if (!ConsumeByte(sig, off, len)) return false;
// Zero-length integers are not allowed.
return (len != 0);
}
bool ConsumeNumber(const std::vectorunsigned char sig, size_t off,
unsigned int len)
{
// Length of number should be within signature.
if (off + len sig.size()) return false;
// Negative numbers are not allowed.
if (sig[off] 0x80) return false;
// Zero bytes at the start are not allowed, unless it would
// otherwise be interpreted as a negative number.
if (len 1 (sig[off] == 0x00) !(sig[off+1] 0x80)) return false;
// Consume number itself.
off += len;
return true;
}
// Consume a DER encoded integer, update off if successful.
bool ConsumeDERInteger(const std::vectorunsigned char sig, size_t off) {
unsigned int len;
// Type byte must be integer
if (!ConsumeTypeByte(sig, off, 0x02)) return false;
if (!ConsumeNonZeroLength(sig, off, len)) return false;
// Now the BE encoded value itself.
if (!ConsumeNumber(sig, off, len)) return false;
return true;
}
bool IsValidSignatureEncoding(const std::vectorunsigned char sig) {
// Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S]
[sighash]
// * total-length: 1-byte length descriptor of everything that follows,
// excluding the sighash byte.
// * R-length: 1-byte length descriptor of the R value that follows.
// * R: arbitrary-length big-endian encoded R value. It cannot start with
any
// null bytes, unless the first byte that follows is 0x80 or higher, in
which
// case a single null byte is required.
// * S-length: 1-byte length descriptor of the S value that follows.
// * S: arbitrary-length big-endian encoded S value. The same rules apply.
// * sighash: 1-byte value indicating what data is hashed.
// Accept empty signature as correctly encoded (but invalid) signature,
// even though it is not strictly DER.
if (sig.size() == 0) return true;
// Maximum size constraint.
if (sig.size() 73) return false;
size_t off = 0;
// A signature is of type compound.
if (!ConsumeTypeByte(sig, off, 0x30)) return false;
unsigned int len;
if (!ConsumeNonZeroLength(sig, off, len)) return false;
// Make sure the length covers the rest (except sighash).
if (len + 1 != sig.size() - off) return false;
// Check R value.
if (!ConsumeDERInteger(sig, off)) return false;
// Check S value.
if (!ConsumeDERInteger(sig, off)) return false;
// There should exactly one byte left (the sighash).
return off + 1 == sig.size() ? true : false;
}
#else
bool IsValidSignatureEncoding(const std::vectorunsigned char sig) {
// Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S]
[sighash]
// * total-length: 1-byte length descriptor of everything that follows,
// excluding the sighash byte.
// * R-length: 1-byte length descriptor of the R value that follows.
// * R: arbitrary-length big-endian encoded R value. It must use the
shortest
// possible encoding for a positive integers (which means no null bytes
at
// the start, except a single one when the next byte has its highest
bit set).
// * S-length: 1-byte length descriptor of the S value that follows.
// * S: arbitrary-length big-endian encoded S value. The same rules apply.
// * sighash: 1-byte value indicating
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
To be more in the C++ spirit, I would suggest changing the (const
std::vectorunsigned char sig, size_t off) parameters to
(std::vectorunsigned char::const_iterator itr, std::vectorunsigned
char::const_iterator end).
Example:
bool ConsumeNumber(std::vectorunsigned char::const_iterator itr,
std::vectorunsigned char::const_iterator end, unsigned int len)
{
// Length of number should be within signature.
if (itr + len = end) return false;
// Negative numbers are not allowed.
if (*itr 0x80) return false;
// Zero bytes at the start are not allowed, unless it would
// otherwise be interpreted as a negative number.
if (len 1 (*itr == 0x00) !(*(itr + 1) 0x80)) return false;
// Consume number itself.
itr += len;
return true;
}
On Thursday, 22 January 2015, at 11:02 am, Rusty Russell wrote:
Pieter Wuille pieter.wui...@gmail.com writes:
Hello everyone,
We've been aware of the risk of depending on OpenSSL for consensus
rules for a while, and were trying to get rid of this as part of BIP
62 (malleability protection), which was however postponed due to
unforeseen complexities. The recent evens (see the thread titled
OpenSSL 1.0.0p / 1.0.1k incompatible, causes blockchain rejection.
on this mailing list) have made it clear that the problem is very
real, however, and I would prefer to have a fundamental solution for
it sooner rather than later.
OK, I worked up a clearer (but more verbose) version with fewer
magic numbers. More importantly, feel free to steal the test cases.
One weirdness is the restriction on maximum total length, rather than a
32 byte (33 with 0-prepad) limit on signatures themselves.
Apologies for my babytalk C++. Am sure there's a neater way.
/* Licensed under Creative Commons zero (public domain). */
#include vector
#include cstdlib
#include cassert
#ifdef CLARIFY
bool ConsumeByte(const std::vectorunsigned char sig, size_t off,
unsigned int val)
{
if (off = sig.size()) return false;
val = sig[off++];
return true;
}
bool ConsumeTypeByte(const std::vectorunsigned char sig, size_t off,
unsigned int t)
{
unsigned int type;
if (!ConsumeByte(sig, off, type)) return false;
return (type == t);
}
bool ConsumeNonZeroLength(const std::vectorunsigned char sig, size_t off,
unsigned int len)
{
if (!ConsumeByte(sig, off, len)) return false;
// Zero-length integers are not allowed.
return (len != 0);
}
bool ConsumeNumber(const std::vectorunsigned char sig, size_t off,
unsigned int len)
{
// Length of number should be within signature.
if (off + len sig.size()) return false;
// Negative numbers are not allowed.
if (sig[off] 0x80) return false;
// Zero bytes at the start are not allowed, unless it would
// otherwise be interpreted as a negative number.
if (len 1 (sig[off] == 0x00) !(sig[off+1] 0x80)) return false;
// Consume number itself.
off += len;
return true;
}
// Consume a DER encoded integer, update off if successful.
bool ConsumeDERInteger(const std::vectorunsigned char sig, size_t off) {
unsigned int len;
// Type byte must be integer
if (!ConsumeTypeByte(sig, off, 0x02)) return false;
if (!ConsumeNonZeroLength(sig, off, len)) return false;
// Now the BE encoded value itself.
if (!ConsumeNumber(sig, off, len)) return false;
return true;
}
bool IsValidSignatureEncoding(const std::vectorunsigned char sig) {
// Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S]
[sighash]
// * total-length: 1-byte length descriptor of everything that follows,
// excluding the sighash byte.
// * R-length: 1-byte length descriptor of the R value that follows.
// * R: arbitrary-length big-endian encoded R value. It cannot start with
any
// null bytes, unless the first byte that follows is 0x80 or higher,
in which
// case a single null byte is required.
// * S-length: 1-byte length descriptor of the S value that follows.
// * S: arbitrary-length big-endian encoded S value. The same rules apply.
// * sighash: 1-byte value indicating what data is hashed.
// Accept empty signature as correctly encoded (but invalid) signature,
// even though it is not strictly DER.
if (sig.size() == 0) return true;
// Maximum size constraint.
if (sig.size() 73) return false;
size_t off = 0;
// A signature is of type compound.
if (!ConsumeTypeByte(sig, off, 0x30)) return false;
unsigned int len;
if (!ConsumeNonZeroLength(sig, off, len)) return false;
// Make sure the length covers the rest (except sighash).
if (len + 1 != sig.size() - off) return false;
// Check R value.
if
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
Seems like a good change to me.
On Wed, Jan 21, 2015 at 7:32 PM, Rusty Russell ru...@rustcorp.com.au
wrote:
Pieter Wuille pieter.wui...@gmail.com writes:
Hello everyone,
We've been aware of the risk of depending on OpenSSL for consensus
rules for a while, and were trying to get rid of this as part of BIP
62 (malleability protection), which was however postponed due to
unforeseen complexities. The recent evens (see the thread titled
OpenSSL 1.0.0p / 1.0.1k incompatible, causes blockchain rejection.
on this mailing list) have made it clear that the problem is very
real, however, and I would prefer to have a fundamental solution for
it sooner rather than later.
OK, I worked up a clearer (but more verbose) version with fewer
magic numbers. More importantly, feel free to steal the test cases.
One weirdness is the restriction on maximum total length, rather than a
32 byte (33 with 0-prepad) limit on signatures themselves.
Apologies for my babytalk C++. Am sure there's a neater way.
/* Licensed under Creative Commons zero (public domain). */
#include vector
#include cstdlib
#include cassert
#ifdef CLARIFY
bool ConsumeByte(const std::vectorunsigned char sig, size_t off,
unsigned int val)
{
if (off = sig.size()) return false;
val = sig[off++];
return true;
}
bool ConsumeTypeByte(const std::vectorunsigned char sig, size_t off,
unsigned int t)
{
unsigned int type;
if (!ConsumeByte(sig, off, type)) return false;
return (type == t);
}
bool ConsumeNonZeroLength(const std::vectorunsigned char sig, size_t
off,
unsigned int len)
{
if (!ConsumeByte(sig, off, len)) return false;
// Zero-length integers are not allowed.
return (len != 0);
}
bool ConsumeNumber(const std::vectorunsigned char sig, size_t off,
unsigned int len)
{
// Length of number should be within signature.
if (off + len sig.size()) return false;
// Negative numbers are not allowed.
if (sig[off] 0x80) return false;
// Zero bytes at the start are not allowed, unless it would
// otherwise be interpreted as a negative number.
if (len 1 (sig[off] == 0x00) !(sig[off+1] 0x80)) return
false;
// Consume number itself.
off += len;
return true;
}
// Consume a DER encoded integer, update off if successful.
bool ConsumeDERInteger(const std::vectorunsigned char sig, size_t off)
{
unsigned int len;
// Type byte must be integer
if (!ConsumeTypeByte(sig, off, 0x02)) return false;
if (!ConsumeNonZeroLength(sig, off, len)) return false;
// Now the BE encoded value itself.
if (!ConsumeNumber(sig, off, len)) return false;
return true;
}
bool IsValidSignatureEncoding(const std::vectorunsigned char sig) {
// Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S]
[sighash]
// * total-length: 1-byte length descriptor of everything that follows,
// excluding the sighash byte.
// * R-length: 1-byte length descriptor of the R value that follows.
// * R: arbitrary-length big-endian encoded R value. It cannot start
with any
// null bytes, unless the first byte that follows is 0x80 or
higher, in which
// case a single null byte is required.
// * S-length: 1-byte length descriptor of the S value that follows.
// * S: arbitrary-length big-endian encoded S value. The same rules
apply.
// * sighash: 1-byte value indicating what data is hashed.
// Accept empty signature as correctly encoded (but invalid) signature,
// even though it is not strictly DER.
if (sig.size() == 0) return true;
// Maximum size constraint.
if (sig.size() 73) return false;
size_t off = 0;
// A signature is of type compound.
if (!ConsumeTypeByte(sig, off, 0x30)) return false;
unsigned int len;
if (!ConsumeNonZeroLength(sig, off, len)) return false;
// Make sure the length covers the rest (except sighash).
if (len + 1 != sig.size() - off) return false;
// Check R value.
if (!ConsumeDERInteger(sig, off)) return false;
// Check S value.
if (!ConsumeDERInteger(sig, off)) return false;
// There should exactly one byte left (the sighash).
return off + 1 == sig.size() ? true : false;
}
#else
bool IsValidSignatureEncoding(const std::vectorunsigned char sig) {
// Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S]
[sighash]
// * total-length: 1-byte length descriptor of everything that follows,
// excluding the sighash byte.
// * R-length: 1-byte length descriptor of the R value that follows.
// * R: arbitrary-length big-endian encoded R value. It must use the
shortest
// possible encoding for a positive integers (which means no null
bytes at
// the start, except a single one when the next byte
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
On Wed, Jan 21, 2015 at 11:18 PM, Matt Whitlock b...@mattwhitlock.name wrote: To be more in the C++ spirit, I would suggest changing the (const std::vectorunsigned char sig, size_t off) parameters to (std::vectorunsigned char::const_iterator itr, std::vectorunsigned char::const_iterator end). I agree that is more in the spirit of C++, but part of the motivation for including C++ code that it mostly matches the exact code that has been used in the past two major Bitcoin Core releases (to interpret signatures as standard). -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
[Bitcoin-development] [softfork proposal] Strict DER signatures
Hello everyone, We've been aware of the risk of depending on OpenSSL for consensus rules for a while, and were trying to get rid of this as part of BIP 62 (malleability protection), which was however postponed due to unforeseen complexities. The recent evens (see the thread titled OpenSSL 1.0.0p / 1.0.1k incompatible, causes blockchain rejection. on this mailing list) have made it clear that the problem is very real, however, and I would prefer to have a fundamental solution for it sooner rather than later. I therefore propose a softfork to make non-DER signatures illegal (they've been non-standard since v0.8.0). A draft BIP text can be found on: https://gist.github.com/sipa/5d12c343746dad376c80 The document includes motivation and specification. In addition, an implementation (including unit tests derived from the BIP text) can be found on: https://github.com/sipa/bitcoin/commit/bipstrictder Comments/criticisms are very welcome, but I'd prefer keeping the discussion here on the mailinglist (which is more accessible than on the gist). -- Pieter -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
Pieter Wuille pieter.wui...@gmail.com writes: Hello everyone, We've been aware of the risk of depending on OpenSSL for consensus rules for a while, and were trying to get rid of this as part of BIP 62 (malleability protection), which was however postponed due to unforeseen complexities. The recent evens (see the thread titled OpenSSL 1.0.0p / 1.0.1k incompatible, causes blockchain rejection. on this mailing list) have made it clear that the problem is very real, however, and I would prefer to have a fundamental solution for it sooner rather than later. I therefore propose a softfork to make non-DER signatures illegal (they've been non-standard since v0.8.0). A draft BIP text can be found on: https://gist.github.com/sipa/5d12c343746dad376c80 Cut and paste bug in the last check: // Null bytes at the start of R are not allowed, unless it would otherwise be // interpreted as a negative number. if (lenS 1 (sig[lenR + 6] == 0x00) !(sig[lenR + 7] 0x80)) return false; You mean null bytes at the start of S. Cheers, Rusty. -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ Bitcoin-development mailing list Bitcoin-development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bitcoin-development
