* [Bitcoin-development] [softfork proposal] Strict DER signatures
@ 2015-01-21 0:35 Pieter Wuille
2015-01-21 4:45 ` Rusty Russell
` (7 more replies)
0 siblings, 8 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-01-21 0:35 UTC (permalink / raw)
To: Bitcoin Dev
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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
@ 2015-01-21 4:45 ` Rusty Russell
2015-01-21 16:49 ` Pieter Wuille
2015-01-21 19:10 ` Peter Todd
` (6 subsequent siblings)
7 siblings, 1 reply; 32+ messages in thread
From: Rusty Russell @ 2015-01-21 4:45 UTC (permalink / raw)
To: Pieter Wuille, Bitcoin Dev
Pieter Wuille <pieter.wuille@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 4:45 ` Rusty Russell
@ 2015-01-21 16:49 ` Pieter Wuille
0 siblings, 0 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-01-21 16:49 UTC (permalink / raw)
To: Rusty Russell; +Cc: Bitcoin Dev
On Tue, Jan 20, 2015 at 11:45 PM, Rusty Russell <rusty@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
2015-01-21 4:45 ` Rusty Russell
@ 2015-01-21 19:10 ` Peter Todd
2015-01-21 19:29 ` Douglas Roark
` (5 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: Peter Todd @ 2015-01-21 19:10 UTC (permalink / raw)
To: Pieter Wuille, Gregory Maxwell; +Cc: Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]
On Tue, Jan 20, 2015 at 07:35:49PM -0500, Pieter Wuille wrote:
I read this and it's boring, now that all my objections have been met. :)
I'll try get a chance to actually test/review this in detail; in SF for
the next three weeks with some ugly deadlines and a slow laptop. :(
> 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).
--
'peter'[:-1]@petertodd.org
00000000000000001a5e1dc75b28e8445c6e8a5c35c76637e33a3e96d487b74c
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 650 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
2015-01-21 4:45 ` Rusty Russell
2015-01-21 19:10 ` Peter Todd
@ 2015-01-21 19:29 ` Douglas Roark
2015-01-21 20:30 ` Pieter Wuille
2015-01-21 20:37 ` Gavin Andresen
2015-01-21 20:27 ` Andrew Poelstra
` (4 subsequent siblings)
7 siblings, 2 replies; 32+ messages in thread
From: Douglas Roark @ 2015-01-21 19:29 UTC (permalink / raw)
To: Bitcoin Dev
-----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.
doug@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-----
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
` (2 preceding siblings ...)
2015-01-21 19:29 ` Douglas Roark
@ 2015-01-21 20:27 ` Andrew Poelstra
2015-01-21 22:57 ` Dave Collins
` (3 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: Andrew Poelstra @ 2015-01-21 20:27 UTC (permalink / raw)
To: Pieter Wuille; +Cc: Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 2214 bytes --]
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
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 19:29 ` Douglas Roark
@ 2015-01-21 20:30 ` Pieter Wuille
2015-01-21 20:39 ` Douglas Roark
2015-01-21 20:37 ` Gavin Andresen
1 sibling, 1 reply; 32+ messages in thread
From: Pieter Wuille @ 2015-01-21 20:30 UTC (permalink / raw)
To: Douglas Roark; +Cc: Bitcoin Dev
On Wed, Jan 21, 2015 at 2:29 PM, Douglas Roark <doug@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 19:29 ` Douglas Roark
2015-01-21 20:30 ` Pieter Wuille
@ 2015-01-21 20:37 ` Gavin Andresen
2015-01-21 20:52 ` Douglas Roark
2015-01-21 21:22 ` Pieter Wuille
1 sibling, 2 replies; 32+ messages in thread
From: Gavin Andresen @ 2015-01-21 20:37 UTC (permalink / raw)
To: Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 771 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 1292 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 20:30 ` Pieter Wuille
@ 2015-01-21 20:39 ` Douglas Roark
0 siblings, 0 replies; 32+ messages in thread
From: Douglas Roark @ 2015-01-21 20:39 UTC (permalink / raw)
To: Bitcoin Dev
-----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.
doug@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-----
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 20:37 ` Gavin Andresen
@ 2015-01-21 20:52 ` Douglas Roark
2015-01-21 21:22 ` Pieter Wuille
1 sibling, 0 replies; 32+ messages in thread
From: Douglas Roark @ 2015-01-21 20:52 UTC (permalink / raw)
To: bitcoin-development
-----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.
doug@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-----
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 20:37 ` Gavin Andresen
2015-01-21 20:52 ` Douglas Roark
@ 2015-01-21 21:22 ` Pieter Wuille
1 sibling, 0 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-01-21 21:22 UTC (permalink / raw)
To: Gavin Andresen; +Cc: Bitcoin Dev
On Wed, Jan 21, 2015 at 3:37 PM, Gavin Andresen <gavinandresen@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
` (3 preceding siblings ...)
2015-01-21 20:27 ` Andrew Poelstra
@ 2015-01-21 22:57 ` Dave Collins
2015-01-22 0:32 ` Rusty Russell
` (2 subsequent siblings)
7 siblings, 0 replies; 32+ messages in thread
From: Dave Collins @ 2015-01-21 22:57 UTC (permalink / raw)
To: Pieter Wuille, Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 1828 bytes --]
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).
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 834 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
` (4 preceding siblings ...)
2015-01-21 22:57 ` Dave Collins
@ 2015-01-22 0:32 ` Rusty Russell
2015-01-22 3:12 ` David Vorick
` (2 more replies)
2015-01-22 22:41 ` Zooko Wilcox-OHearn
2015-01-26 5:14 ` Pieter Wuille
7 siblings, 3 replies; 32+ messages in thread
From: Rusty Russell @ 2015-01-22 0:32 UTC (permalink / raw)
To: Pieter Wuille, Bitcoin Dev
Pieter Wuille <pieter.wuille@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::vector<unsigned char> &sig, size_t &off,
unsigned int &val)
{
if (off >= sig.size()) return false;
val = sig[off++];
return true;
}
bool ConsumeTypeByte(const std::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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 what data is hashed (not part of the DER
// signature)
// Accept empty signature as correctly encoded (but invalid) signature,
// even though it is not strictly DER. This avoids needing full DER signatures
// in places where any invalid signature would do. Given that the empty string is
// always invalid as signature, this is safe.
if (sig.size() == 0) return true;
// Minimum and maximum size constraints.
if (sig.size() < 9) return false;
if (sig.size() > 73) return false;
// A signature is of type 0x30 (compound).
if (sig[0] != 0x30) return false;
// Make sure the length covers the entire signature.
if (sig[1] != sig.size() - 3) return false;
// Extract the length of the R element.
unsigned int lenR = sig[3];
// Make sure the length of the S element is still inside the signature.
if (5 + lenR >= sig.size()) return false;
// Extract the length of the S element.
unsigned int lenS = sig[5 + lenR];
// Verify that the length of the signature matches the sum of the length
// of the elements.
if ((size_t)(lenR + lenS + 7) != sig.size()) return false;
// Check whether the R element is an integer.
if (sig[2] != 0x02) return false;
// Zero-length integers are not allowed for R.
if (lenR == 0) return false;
// Negative numbers are not allowed for R.
if (sig[4] & 0x80) return false;
// Null bytes at the start of R are not allowed, unless R would
// otherwise be interpreted as a negative number.
if (lenR > 1 && (sig[4] == 0x00) && !(sig[5] & 0x80)) return false;
// Check whether the S element is an integer.
if (sig[lenR + 4] != 0x02) return false;
// Zero-length integers are not allowed for S.
if (lenS == 0) return false;
// Negative numbers are not allowed for S.
if (sig[lenR + 6] & 0x80) return false;
// Null bytes at the start of S are not allowed, unless S would otherwise be
// interpreted as a negative number.
if (lenS > 1 && (sig[lenR + 6] == 0x00) && !(sig[lenR + 7] & 0x80)) return false;
return true;
}
#endif
#define COMPOUND 0x30
#define NOT_COMPOUND 0x31
// Len gets adjusted by check() to be actual length with this offset.
#define LEN_OK 0
#define LEN_TOO_BIG 1
#define LEN_TOO_SMALL 0xff
#define INT 0x02
#define NOT_INT 0x03
#define MINIMAL_SIGLEN 1
#define MINIMAL_SIGVAL 0x0
#define NORMAL_SIGLEN 32
#define NORMAL_SIGVAL(S) S, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, \
0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, \
0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, \
0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f
// 33 bytes is possible, with 0 prepended.
#define MAXIMAL_SIGLEN 33
#define MAXIMAL_SIGVAL(S) NORMAL_SIGVAL(S), 0x20
#define OVERSIZE_SIGLEN 34
#define OVERSIZE_SIGVAL(S) MAXIMAL_SIGVAL(S), 0x21
#define ZEROPAD_SIGLEN (1 + NORMAL_SIGLEN)
#define ZEROPAD_SIGVAL(S) 00, NORMAL_SIGVAL(S)
#define SIGHASH 0xf0
static bool check(const std::vector<unsigned char> &sig)
{
std::vector<unsigned char> fixed = sig;
// Fixup length
if (fixed.size() > 1)
fixed[1] += fixed.size() - 3;
return IsValidSignatureEncoding(fixed);
}
#define good(arr) assert(check(std::vector<unsigned char>(arr, arr+sizeof(arr))))
#define bad(arr) assert(!check(std::vector<unsigned char>(arr, arr+sizeof(arr))))
// The OK cases.
static unsigned char zerolen[] = { };
static unsigned char normal[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char min_r[] = { COMPOUND, LEN_OK,
INT, MINIMAL_SIGLEN, MINIMAL_SIGVAL,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char min_s[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, MINIMAL_SIGLEN, MINIMAL_SIGVAL,
SIGHASH };
static unsigned char max_r[] = { COMPOUND, LEN_OK,
INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char max_s[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x2),
SIGHASH };
// As long as total size doesn't go over, a single sig is allowed > 33 bytes
static unsigned char wierd_s_len[] = { COMPOUND, LEN_OK,
INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char wierd_r_len[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x2),
SIGHASH };
static unsigned char zeropad_s[] = { COMPOUND, LEN_OK,
INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x81),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char zeropad_r[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x82),
SIGHASH };
// The fail cases.
static unsigned char not_compound[] = { NOT_COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char short_len[] = { COMPOUND, LEN_TOO_SMALL,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char long_len[] = { COMPOUND, LEN_TOO_BIG,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char r_notint[] = { COMPOUND, LEN_OK,
NOT_INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char s_notint[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
NOT_INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char s_oversig[] = { COMPOUND, LEN_OK,
INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x1),
INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char r_oversig[] = { COMPOUND, LEN_OK,
INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x1),
INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x2),
SIGHASH };
static unsigned char s_negative[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x81),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char r_negative[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x82),
SIGHASH };
static unsigned char zeropad_bad_s[] = { COMPOUND, LEN_OK,
INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char zeropad_bad_r[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x2),
SIGHASH };
static unsigned char missing_sighash[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2) };
static unsigned char extra_byte[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH, 0 };
// Bad signature lengths
static unsigned char zerolen_r[] = { COMPOUND, LEN_OK,
INT, 0,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char zerolen_s[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, 0,
SIGHASH };
static unsigned char overlen_r_by_1[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN + 1 + 1 + NORMAL_SIGLEN + 1 + 1, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char overlen_s_by_1[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN+1+1, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char underlen_r_by_1[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN-1, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
SIGHASH };
static unsigned char underlen_s_by_1[] = { COMPOUND, LEN_OK,
INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
INT, NORMAL_SIGLEN-1, NORMAL_SIGVAL(0x2),
SIGHASH };
int main()
{
good(zerolen);
good(normal);
good(min_r);
good(min_s);
good(max_r);
good(max_s);
good(wierd_s_len);
good(wierd_r_len);
good(zeropad_s);
good(zeropad_r);
// Try different amounts of truncation.
for (size_t i = 1; i < sizeof(normal)-1; i++)
assert(!check(std::vector<unsigned char>(normal, normal+i)));
bad(not_compound);
bad(short_len);
bad(long_len);
bad(r_notint);
bad(s_notint);
bad(s_oversig);
bad(r_oversig);
bad(s_negative);
bad(r_negative);
bad(s_negative);
bad(r_negative);
bad(zeropad_bad_s);
bad(zeropad_bad_r);
bad(zerolen_r);
bad(zerolen_s);
bad(overlen_r_by_1);
bad(overlen_s_by_1);
bad(underlen_r_by_1);
bad(underlen_s_by_1);
bad(missing_sighash);
bad(extra_byte);
return 0;
}
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-22 0:32 ` Rusty Russell
@ 2015-01-22 3:12 ` David Vorick
2015-01-22 4:18 ` Matt Whitlock
2015-01-25 14:34 ` Pieter Wuille
2 siblings, 0 replies; 32+ messages in thread
From: David Vorick @ 2015-01-22 3:12 UTC (permalink / raw)
Cc: Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 18350 bytes --]
Seems like a good change to me.
On Wed, Jan 21, 2015 at 7:32 PM, Rusty Russell <rusty@rustcorp•com.au>
wrote:
> Pieter Wuille <pieter.wuille@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::vector<unsigned char> &sig, size_t &off,
> unsigned int &val)
> {
> if (off >= sig.size()) return false;
>
> val = sig[off++];
> return true;
> }
>
> bool ConsumeTypeByte(const std::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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 what data is hashed (not part of
> the DER
> // signature)
>
> // Accept empty signature as correctly encoded (but invalid) signature,
> // even though it is not strictly DER. This avoids needing full DER
> signatures
> // in places where any invalid signature would do. Given that the
> empty string is
> // always invalid as signature, this is safe.
> if (sig.size() == 0) return true;
>
> // Minimum and maximum size constraints.
> if (sig.size() < 9) return false;
> if (sig.size() > 73) return false;
>
> // A signature is of type 0x30 (compound).
> if (sig[0] != 0x30) return false;
>
> // Make sure the length covers the entire signature.
> if (sig[1] != sig.size() - 3) return false;
>
> // Extract the length of the R element.
> unsigned int lenR = sig[3];
>
> // Make sure the length of the S element is still inside the signature.
> if (5 + lenR >= sig.size()) return false;
>
> // Extract the length of the S element.
> unsigned int lenS = sig[5 + lenR];
>
> // Verify that the length of the signature matches the sum of the
> length
> // of the elements.
> if ((size_t)(lenR + lenS + 7) != sig.size()) return false;
>
> // Check whether the R element is an integer.
> if (sig[2] != 0x02) return false;
>
> // Zero-length integers are not allowed for R.
> if (lenR == 0) return false;
>
> // Negative numbers are not allowed for R.
> if (sig[4] & 0x80) return false;
>
> // Null bytes at the start of R are not allowed, unless R would
> // otherwise be interpreted as a negative number.
> if (lenR > 1 && (sig[4] == 0x00) && !(sig[5] & 0x80)) return false;
>
> // Check whether the S element is an integer.
> if (sig[lenR + 4] != 0x02) return false;
>
> // Zero-length integers are not allowed for S.
> if (lenS == 0) return false;
>
> // Negative numbers are not allowed for S.
> if (sig[lenR + 6] & 0x80) return false;
>
> // Null bytes at the start of S are not allowed, unless S would
> otherwise be
> // interpreted as a negative number.
> if (lenS > 1 && (sig[lenR + 6] == 0x00) && !(sig[lenR + 7] & 0x80))
> return false;
>
> return true;
> }
> #endif
>
> #define COMPOUND 0x30
> #define NOT_COMPOUND 0x31
>
> // Len gets adjusted by check() to be actual length with this offset.
> #define LEN_OK 0
> #define LEN_TOO_BIG 1
> #define LEN_TOO_SMALL 0xff
>
> #define INT 0x02
> #define NOT_INT 0x03
>
> #define MINIMAL_SIGLEN 1
> #define MINIMAL_SIGVAL 0x0
>
> #define NORMAL_SIGLEN 32
> #define NORMAL_SIGVAL(S) S, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, \
> 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, \
> 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, \
> 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f
>
> // 33 bytes is possible, with 0 prepended.
> #define MAXIMAL_SIGLEN 33
> #define MAXIMAL_SIGVAL(S) NORMAL_SIGVAL(S), 0x20
>
> #define OVERSIZE_SIGLEN 34
> #define OVERSIZE_SIGVAL(S) MAXIMAL_SIGVAL(S), 0x21
>
> #define ZEROPAD_SIGLEN (1 + NORMAL_SIGLEN)
> #define ZEROPAD_SIGVAL(S) 00, NORMAL_SIGVAL(S)
>
> #define SIGHASH 0xf0
>
> static bool check(const std::vector<unsigned char> &sig)
> {
> std::vector<unsigned char> fixed = sig;
>
> // Fixup length
> if (fixed.size() > 1)
> fixed[1] += fixed.size() - 3;
> return IsValidSignatureEncoding(fixed);
> }
>
> #define good(arr) assert(check(std::vector<unsigned char>(arr,
> arr+sizeof(arr))))
> #define bad(arr) assert(!check(std::vector<unsigned char>(arr,
> arr+sizeof(arr))))
>
> // The OK cases.
> static unsigned char zerolen[] = { };
> static unsigned char normal[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char min_r[] = { COMPOUND, LEN_OK,
> INT, MINIMAL_SIGLEN, MINIMAL_SIGVAL,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char min_s[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, MINIMAL_SIGLEN, MINIMAL_SIGVAL,
> SIGHASH };
> static unsigned char max_r[] = { COMPOUND, LEN_OK,
> INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char max_s[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x2),
> SIGHASH };
> // As long as total size doesn't go over, a single sig is allowed > 33
> bytes
> static unsigned char wierd_s_len[] = { COMPOUND, LEN_OK,
> INT, OVERSIZE_SIGLEN,
> OVERSIZE_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char wierd_r_len[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, OVERSIZE_SIGLEN,
> OVERSIZE_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zeropad_s[] = { COMPOUND, LEN_OK,
> INT, ZEROPAD_SIGLEN,
> ZEROPAD_SIGVAL(0x81),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zeropad_r[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, ZEROPAD_SIGLEN,
> ZEROPAD_SIGVAL(0x82),
> SIGHASH };
>
>
> // The fail cases.
> static unsigned char not_compound[] = { NOT_COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char short_len[] = { COMPOUND, LEN_TOO_SMALL,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char long_len[] = { COMPOUND, LEN_TOO_BIG,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char r_notint[] = { COMPOUND, LEN_OK,
> NOT_INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char s_notint[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> NOT_INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char s_oversig[] = { COMPOUND, LEN_OK,
> INT, OVERSIZE_SIGLEN,
> OVERSIZE_SIGVAL(0x1),
> INT, MAXIMAL_SIGLEN,
> MAXIMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char r_oversig[] = { COMPOUND, LEN_OK,
> INT, MAXIMAL_SIGLEN,
> MAXIMAL_SIGVAL(0x1),
> INT, OVERSIZE_SIGLEN,
> OVERSIZE_SIGVAL(0x2),
> SIGHASH };
> static unsigned char s_negative[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x81),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char r_negative[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x82),
> SIGHASH };
> static unsigned char zeropad_bad_s[] = { COMPOUND, LEN_OK,
> INT, ZEROPAD_SIGLEN,
> ZEROPAD_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zeropad_bad_r[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, ZEROPAD_SIGLEN,
> ZEROPAD_SIGVAL(0x2),
> SIGHASH };
> static unsigned char missing_sighash[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2) };
> static unsigned char extra_byte[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH, 0 };
>
> // Bad signature lengths
> static unsigned char zerolen_r[] = { COMPOUND, LEN_OK,
> INT, 0,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zerolen_s[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, 0,
> SIGHASH };
> static unsigned char overlen_r_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN + 1 + 1 +
> NORMAL_SIGLEN + 1 + 1, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char overlen_s_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN+1+1,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char underlen_r_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN-1,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char underlen_s_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN,
> NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN-1,
> NORMAL_SIGVAL(0x2),
> SIGHASH };
>
> int main()
> {
> good(zerolen);
> good(normal);
> good(min_r);
> good(min_s);
> good(max_r);
> good(max_s);
> good(wierd_s_len);
> good(wierd_r_len);
> good(zeropad_s);
> good(zeropad_r);
>
> // Try different amounts of truncation.
> for (size_t i = 1; i < sizeof(normal)-1; i++)
> assert(!check(std::vector<unsigned char>(normal, normal+i)));
>
> bad(not_compound);
> bad(short_len);
> bad(long_len);
> bad(r_notint);
> bad(s_notint);
> bad(s_oversig);
> bad(r_oversig);
> bad(s_negative);
> bad(r_negative);
> bad(s_negative);
> bad(r_negative);
> bad(zeropad_bad_s);
> bad(zeropad_bad_r);
> bad(zerolen_r);
> bad(zerolen_s);
> bad(overlen_r_by_1);
> bad(overlen_s_by_1);
> bad(underlen_r_by_1);
> bad(underlen_s_by_1);
> bad(missing_sighash);
> bad(extra_byte);
>
> return 0;
> }
>
>
>
>
> ------------------------------------------------------------------------------
> 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
>
[-- Attachment #2: Type: text/html, Size: 22506 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-22 0:32 ` Rusty Russell
2015-01-22 3:12 ` David Vorick
@ 2015-01-22 4:18 ` Matt Whitlock
2015-01-22 4:20 ` Pieter Wuille
2015-01-25 14:34 ` Pieter Wuille
2 siblings, 1 reply; 32+ messages in thread
From: Matt Whitlock @ 2015-01-22 4:18 UTC (permalink / raw)
To: Rusty Russell; +Cc: bitcoin-development
To be more in the C++ spirit, I would suggest changing the (const std::vector<unsigned char> &sig, size_t &off) parameters to (std::vector<unsigned char>::const_iterator &itr, std::vector<unsigned char>::const_iterator end).
Example:
bool ConsumeNumber(std::vector<unsigned char>::const_iterator &itr, std::vector<unsigned 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.wuille@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::vector<unsigned char> &sig, size_t &off,
> unsigned int &val)
> {
> if (off >= sig.size()) return false;
>
> val = sig[off++];
> return true;
> }
>
> bool ConsumeTypeByte(const std::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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::vector<unsigned 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 what data is hashed (not part of the DER
> // signature)
>
> // Accept empty signature as correctly encoded (but invalid) signature,
> // even though it is not strictly DER. This avoids needing full DER signatures
> // in places where any invalid signature would do. Given that the empty string is
> // always invalid as signature, this is safe.
> if (sig.size() == 0) return true;
>
> // Minimum and maximum size constraints.
> if (sig.size() < 9) return false;
> if (sig.size() > 73) return false;
>
> // A signature is of type 0x30 (compound).
> if (sig[0] != 0x30) return false;
>
> // Make sure the length covers the entire signature.
> if (sig[1] != sig.size() - 3) return false;
>
> // Extract the length of the R element.
> unsigned int lenR = sig[3];
>
> // Make sure the length of the S element is still inside the signature.
> if (5 + lenR >= sig.size()) return false;
>
> // Extract the length of the S element.
> unsigned int lenS = sig[5 + lenR];
>
> // Verify that the length of the signature matches the sum of the length
> // of the elements.
> if ((size_t)(lenR + lenS + 7) != sig.size()) return false;
>
> // Check whether the R element is an integer.
> if (sig[2] != 0x02) return false;
>
> // Zero-length integers are not allowed for R.
> if (lenR == 0) return false;
>
> // Negative numbers are not allowed for R.
> if (sig[4] & 0x80) return false;
>
> // Null bytes at the start of R are not allowed, unless R would
> // otherwise be interpreted as a negative number.
> if (lenR > 1 && (sig[4] == 0x00) && !(sig[5] & 0x80)) return false;
>
> // Check whether the S element is an integer.
> if (sig[lenR + 4] != 0x02) return false;
>
> // Zero-length integers are not allowed for S.
> if (lenS == 0) return false;
>
> // Negative numbers are not allowed for S.
> if (sig[lenR + 6] & 0x80) return false;
>
> // Null bytes at the start of S are not allowed, unless S would otherwise be
> // interpreted as a negative number.
> if (lenS > 1 && (sig[lenR + 6] == 0x00) && !(sig[lenR + 7] & 0x80)) return false;
>
> return true;
> }
> #endif
>
> #define COMPOUND 0x30
> #define NOT_COMPOUND 0x31
>
> // Len gets adjusted by check() to be actual length with this offset.
> #define LEN_OK 0
> #define LEN_TOO_BIG 1
> #define LEN_TOO_SMALL 0xff
>
> #define INT 0x02
> #define NOT_INT 0x03
>
> #define MINIMAL_SIGLEN 1
> #define MINIMAL_SIGVAL 0x0
>
> #define NORMAL_SIGLEN 32
> #define NORMAL_SIGVAL(S) S, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, \
> 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, \
> 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, \
> 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f
>
> // 33 bytes is possible, with 0 prepended.
> #define MAXIMAL_SIGLEN 33
> #define MAXIMAL_SIGVAL(S) NORMAL_SIGVAL(S), 0x20
>
> #define OVERSIZE_SIGLEN 34
> #define OVERSIZE_SIGVAL(S) MAXIMAL_SIGVAL(S), 0x21
>
> #define ZEROPAD_SIGLEN (1 + NORMAL_SIGLEN)
> #define ZEROPAD_SIGVAL(S) 00, NORMAL_SIGVAL(S)
>
> #define SIGHASH 0xf0
>
> static bool check(const std::vector<unsigned char> &sig)
> {
> std::vector<unsigned char> fixed = sig;
>
> // Fixup length
> if (fixed.size() > 1)
> fixed[1] += fixed.size() - 3;
> return IsValidSignatureEncoding(fixed);
> }
>
> #define good(arr) assert(check(std::vector<unsigned char>(arr, arr+sizeof(arr))))
> #define bad(arr) assert(!check(std::vector<unsigned char>(arr, arr+sizeof(arr))))
>
> // The OK cases.
> static unsigned char zerolen[] = { };
> static unsigned char normal[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char min_r[] = { COMPOUND, LEN_OK,
> INT, MINIMAL_SIGLEN, MINIMAL_SIGVAL,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char min_s[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, MINIMAL_SIGLEN, MINIMAL_SIGVAL,
> SIGHASH };
> static unsigned char max_r[] = { COMPOUND, LEN_OK,
> INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char max_s[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x2),
> SIGHASH };
> // As long as total size doesn't go over, a single sig is allowed > 33 bytes
> static unsigned char wierd_s_len[] = { COMPOUND, LEN_OK,
> INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char wierd_r_len[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zeropad_s[] = { COMPOUND, LEN_OK,
> INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x81),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zeropad_r[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x82),
> SIGHASH };
>
>
> // The fail cases.
> static unsigned char not_compound[] = { NOT_COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char short_len[] = { COMPOUND, LEN_TOO_SMALL,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char long_len[] = { COMPOUND, LEN_TOO_BIG,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char r_notint[] = { COMPOUND, LEN_OK,
> NOT_INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char s_notint[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> NOT_INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char s_oversig[] = { COMPOUND, LEN_OK,
> INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x1),
> INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char r_oversig[] = { COMPOUND, LEN_OK,
> INT, MAXIMAL_SIGLEN, MAXIMAL_SIGVAL(0x1),
> INT, OVERSIZE_SIGLEN, OVERSIZE_SIGVAL(0x2),
> SIGHASH };
> static unsigned char s_negative[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x81),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char r_negative[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x82),
> SIGHASH };
> static unsigned char zeropad_bad_s[] = { COMPOUND, LEN_OK,
> INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zeropad_bad_r[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, ZEROPAD_SIGLEN, ZEROPAD_SIGVAL(0x2),
> SIGHASH };
> static unsigned char missing_sighash[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2) };
> static unsigned char extra_byte[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH, 0 };
>
> // Bad signature lengths
> static unsigned char zerolen_r[] = { COMPOUND, LEN_OK,
> INT, 0,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char zerolen_s[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, 0,
> SIGHASH };
> static unsigned char overlen_r_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN + 1 + 1 + NORMAL_SIGLEN + 1 + 1, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char overlen_s_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN+1+1, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char underlen_r_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN-1, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x2),
> SIGHASH };
> static unsigned char underlen_s_by_1[] = { COMPOUND, LEN_OK,
> INT, NORMAL_SIGLEN, NORMAL_SIGVAL(0x1),
> INT, NORMAL_SIGLEN-1, NORMAL_SIGVAL(0x2),
> SIGHASH };
>
> int main()
> {
> good(zerolen);
> good(normal);
> good(min_r);
> good(min_s);
> good(max_r);
> good(max_s);
> good(wierd_s_len);
> good(wierd_r_len);
> good(zeropad_s);
> good(zeropad_r);
>
> // Try different amounts of truncation.
> for (size_t i = 1; i < sizeof(normal)-1; i++)
> assert(!check(std::vector<unsigned char>(normal, normal+i)));
>
> bad(not_compound);
> bad(short_len);
> bad(long_len);
> bad(r_notint);
> bad(s_notint);
> bad(s_oversig);
> bad(r_oversig);
> bad(s_negative);
> bad(r_negative);
> bad(s_negative);
> bad(r_negative);
> bad(zeropad_bad_s);
> bad(zeropad_bad_r);
> bad(zerolen_r);
> bad(zerolen_s);
> bad(overlen_r_by_1);
> bad(overlen_s_by_1);
> bad(underlen_r_by_1);
> bad(underlen_s_by_1);
> bad(missing_sighash);
> bad(extra_byte);
>
> return 0;
> }
>
>
>
> ------------------------------------------------------------------------------
> 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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-22 4:18 ` Matt Whitlock
@ 2015-01-22 4:20 ` Pieter Wuille
0 siblings, 0 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-01-22 4:20 UTC (permalink / raw)
To: Matt Whitlock; +Cc: Bitcoin Dev
On Wed, Jan 21, 2015 at 11:18 PM, Matt Whitlock <bip@mattwhitlock•name> wrote:
> To be more in the C++ spirit, I would suggest changing the (const std::vector<unsigned char> &sig, size_t &off) parameters to (std::vector<unsigned char>::const_iterator &itr, std::vector<unsigned 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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
` (5 preceding siblings ...)
2015-01-22 0:32 ` Rusty Russell
@ 2015-01-22 22:41 ` Zooko Wilcox-OHearn
2015-01-25 16:57 ` Pieter Wuille
2015-01-26 5:14 ` Pieter Wuille
7 siblings, 1 reply; 32+ messages in thread
From: Zooko Wilcox-OHearn @ 2015-01-22 22:41 UTC (permalink / raw)
To: Pieter Wuille; +Cc: Bitcoin Dev
.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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-22 0:32 ` Rusty Russell
2015-01-22 3:12 ` David Vorick
2015-01-22 4:18 ` Matt Whitlock
@ 2015-01-25 14:34 ` Pieter Wuille
2015-01-25 14:48 ` Gregory Maxwell
2 siblings, 1 reply; 32+ messages in thread
From: Pieter Wuille @ 2015-01-25 14:34 UTC (permalink / raw)
To: Rusty Russell; +Cc: Bitcoin Dev
On Wed, Jan 21, 2015 at 8:32 PM, Rusty Russell <rusty@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-25 14:34 ` Pieter Wuille
@ 2015-01-25 14:48 ` Gregory Maxwell
2015-02-03 0:44 ` Pieter Wuille
0 siblings, 1 reply; 32+ messages in thread
From: Gregory Maxwell @ 2015-01-25 14:48 UTC (permalink / raw)
To: Pieter Wuille; +Cc: Bitcoin Dev
On Sun, Jan 25, 2015 at 2:34 PM, Pieter Wuille <pieter.wuille@gmail•com> wrote:
> * Add it to the softfork now, and be done with it.
Initially I was of the opinion that we couldn't do that, because
soft-forks which hit transactions many nodes would relay+mine creates
a forking risk... but with the realization that imbalanced R/S plus
checksig-not would only be work with 0.10rc/git changed my mind.
Unlike two years ago miners no longer appear to be racing the bleeding
edge, and it's never show up in a release. Obviously the next RC would
also make those non-standard. And then we'll have some non-trivial
amount of time before the soft-fork activates for whatever stragglers
there are on 0.10 prerelease code to update. The deployment of the
soft-fork rules themselves will already drive people to update.
In terms of being robust to implementation differences, not permitting
overlarge R/S is obviously prudent.
So I think we should just go ahead with R/S length upper bounds as
both IsStandard and in STRICTDER.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-22 22:41 ` Zooko Wilcox-OHearn
@ 2015-01-25 16:57 ` Pieter Wuille
0 siblings, 0 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-01-25 16:57 UTC (permalink / raw)
To: Zooko Wilcox-OHearn; +Cc: Bitcoin Dev
On Thu, Jan 22, 2015 at 6:41 PM, Zooko Wilcox-OHearn
<zooko@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
` (6 preceding siblings ...)
2015-01-22 22:41 ` Zooko Wilcox-OHearn
@ 2015-01-26 5:14 ` Pieter Wuille
2015-01-26 18:35 ` Gregory Maxwell
7 siblings, 1 reply; 32+ messages in thread
From: Pieter Wuille @ 2015-01-26 5:14 UTC (permalink / raw)
To: Bitcoin Dev, Gregory Maxwell
On Tue, Jan 20, 2015 at 8:35 PM, Pieter Wuille <pieter.wuille@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-26 5:14 ` Pieter Wuille
@ 2015-01-26 18:35 ` Gregory Maxwell
2015-01-28 6:24 ` Wladimir
2015-02-06 21:38 ` Pieter Wuille
0 siblings, 2 replies; 32+ messages in thread
From: Gregory Maxwell @ 2015-01-26 18:35 UTC (permalink / raw)
To: Pieter Wuille; +Cc: Bitcoin Dev
On Mon, Jan 26, 2015 at 5:14 AM, Pieter Wuille <pieter.wuille@gmail•com> wrote:
> On Tue, Jan 20, 2015 at 8:35 PM, Pieter Wuille <pieter.wuille@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-26 18:35 ` Gregory Maxwell
@ 2015-01-28 6:24 ` Wladimir
2015-02-06 21:38 ` Pieter Wuille
1 sibling, 0 replies; 32+ messages in thread
From: Wladimir @ 2015-01-28 6:24 UTC (permalink / raw)
To: Bitcoin Dev
On Mon, 26 Jan 2015, Gregory Maxwell wrote:
> On Mon, Jan 26, 2015 at 5:14 AM, Pieter Wuille <pieter.wuille@gmail•com> wrote:
>> On Tue, Jan 20, 2015 at 8:35 PM, Pieter Wuille <pieter.wuille@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-25 14:48 ` Gregory Maxwell
@ 2015-02-03 0:44 ` Pieter Wuille
2015-02-03 2:21 ` Gregory Maxwell
2015-02-03 12:00 ` Wladimir
0 siblings, 2 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-02-03 0:44 UTC (permalink / raw)
To: Gregory Maxwell; +Cc: Bitcoin Dev
On Sun, Jan 25, 2015 at 6:48 AM, Gregory Maxwell <gmaxwell@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-02-03 0:44 ` Pieter Wuille
@ 2015-02-03 2:21 ` Gregory Maxwell
2015-02-03 12:00 ` Wladimir
1 sibling, 0 replies; 32+ messages in thread
From: Gregory Maxwell @ 2015-02-03 2:21 UTC (permalink / raw)
To: Pieter Wuille; +Cc: Bitcoin Dev
On Tue, Feb 3, 2015 at 12:44 AM, Pieter Wuille <pieter.wuille@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.
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-02-03 0:44 ` Pieter Wuille
2015-02-03 2:21 ` Gregory Maxwell
@ 2015-02-03 12:00 ` Wladimir
2015-02-03 14:30 ` Alex Morcos
2015-02-03 18:15 ` Pieter Wuille
1 sibling, 2 replies; 32+ messages in thread
From: Wladimir @ 2015-02-03 12:00 UTC (permalink / raw)
To: Pieter Wuille; +Cc: Bitcoin Dev
> 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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-02-03 12:00 ` Wladimir
@ 2015-02-03 14:30 ` Alex Morcos
2015-02-03 18:15 ` Pieter Wuille
1 sibling, 0 replies; 32+ messages in thread
From: Alex Morcos @ 2015-02-03 14:30 UTC (permalink / raw)
To: Wladimir; +Cc: Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]
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 <laanwj@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
>
[-- Attachment #2: Type: text/html, Size: 2186 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-02-03 12:00 ` Wladimir
2015-02-03 14:30 ` Alex Morcos
@ 2015-02-03 18:15 ` Pieter Wuille
2015-02-03 18:19 ` Gavin Andresen
2015-02-03 23:38 ` Pieter Wuille
1 sibling, 2 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-02-03 18:15 UTC (permalink / raw)
To: Wladimir; +Cc: Bitcoin Dev
On Tue, Feb 3, 2015 at 4:00 AM, Wladimir <laanwj@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-02-03 18:15 ` Pieter Wuille
@ 2015-02-03 18:19 ` Gavin Andresen
2015-02-03 19:22 ` Jeff Garzik
2015-02-03 23:38 ` Pieter Wuille
1 sibling, 1 reply; 32+ messages in thread
From: Gavin Andresen @ 2015-02-03 18:19 UTC (permalink / raw)
To: Pieter Wuille; +Cc: Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 500 bytes --]
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.wuille@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
[-- Attachment #2: Type: text/html, Size: 1027 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-02-03 18:19 ` Gavin Andresen
@ 2015-02-03 19:22 ` Jeff Garzik
0 siblings, 0 replies; 32+ messages in thread
From: Jeff Garzik @ 2015-02-03 19:22 UTC (permalink / raw)
To: Gavin Andresen; +Cc: Bitcoin Dev
[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]
+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 <gavinandresen@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.wuille@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/
[-- Attachment #2: Type: text/html, Size: 2619 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-02-03 18:15 ` Pieter Wuille
2015-02-03 18:19 ` Gavin Andresen
@ 2015-02-03 23:38 ` Pieter Wuille
1 sibling, 0 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-02-03 23:38 UTC (permalink / raw)
To: Wladimir; +Cc: Bitcoin Dev
On Tue, Feb 3, 2015 at 10:15 AM, Pieter Wuille <pieter.wuille@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
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Bitcoin-development] [softfork proposal] Strict DER signatures
2015-01-26 18:35 ` Gregory Maxwell
2015-01-28 6:24 ` Wladimir
@ 2015-02-06 21:38 ` Pieter Wuille
1 sibling, 0 replies; 32+ messages in thread
From: Pieter Wuille @ 2015-02-06 21:38 UTC (permalink / raw)
To: Gregory Maxwell; +Cc: Bitcoin Dev
On Mon, Jan 26, 2015 at 10:35 AM, Gregory Maxwell <gmaxwell@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
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2015-02-06 21:38 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-21 0:35 [Bitcoin-development] [softfork proposal] Strict DER signatures Pieter Wuille
2015-01-21 4:45 ` Rusty Russell
2015-01-21 16:49 ` Pieter Wuille
2015-01-21 19:10 ` Peter Todd
2015-01-21 19:29 ` Douglas Roark
2015-01-21 20:30 ` Pieter Wuille
2015-01-21 20:39 ` Douglas Roark
2015-01-21 20:37 ` Gavin Andresen
2015-01-21 20:52 ` Douglas Roark
2015-01-21 21:22 ` Pieter Wuille
2015-01-21 20:27 ` Andrew Poelstra
2015-01-21 22:57 ` Dave Collins
2015-01-22 0:32 ` Rusty Russell
2015-01-22 3:12 ` David Vorick
2015-01-22 4:18 ` Matt Whitlock
2015-01-22 4:20 ` Pieter Wuille
2015-01-25 14:34 ` Pieter Wuille
2015-01-25 14:48 ` Gregory Maxwell
2015-02-03 0:44 ` Pieter Wuille
2015-02-03 2:21 ` Gregory Maxwell
2015-02-03 12:00 ` Wladimir
2015-02-03 14:30 ` Alex Morcos
2015-02-03 18:15 ` Pieter Wuille
2015-02-03 18:19 ` Gavin Andresen
2015-02-03 19:22 ` Jeff Garzik
2015-02-03 23:38 ` Pieter Wuille
2015-01-22 22:41 ` Zooko Wilcox-OHearn
2015-01-25 16:57 ` Pieter Wuille
2015-01-26 5:14 ` Pieter Wuille
2015-01-26 18:35 ` Gregory Maxwell
2015-01-28 6:24 ` Wladimir
2015-02-06 21:38 ` Pieter Wuille
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox