Fix DER encoding.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

bitcoin_script.c

@@ -66,23 +66,115 @@ static void add_push_key(u8 **scriptp, const struct pubkey *key)
 	add_push_bytes(scriptp, key->key, pubkey_len(key));
 }
 
-/* Bitcoin wants DER encoding. */
+/* Stolen direct from bitcoin/src/script/sign.cpp:
-static void add_push_sig(u8 **scriptp, const struct signature *sig)
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2014 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+*/
+static bool IsValidSignatureEncoding(const unsigned char sig[], size_t len)
 {
-	u8 der[2 + 2 + sizeof(sig->r) + 2 + sizeof(sig->s)];
+    // 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)
 
-	der[0] = 0x30; /* Type */
+    // Minimum and maximum size constraints.
-	der[1] = sizeof(der) - 2; /* Total length */
+    if (len < 9) return false;
+    if (len > 73) return false;
 
-	der[2] = 0x2; /* r value type. */
+    // A signature is of type 0x30 (compound).
-	der[3] = sizeof(sig->r); /* r length */
+    if (sig[0] != 0x30) return false;
-	memcpy(der+4, sig->r, sizeof(sig->r));
 
-	der[4 + sizeof(sig->r)] = 0x2; /* s value type. */
+    // Make sure the length covers the entire signature.
-	der[4 + sizeof(sig->r) + 1] = sizeof(sig->s); /* s value length. */
+    if (sig[1] != len - 3) return false;
-	memcpy(der+4+sizeof(sig->r)+2, sig->s, sizeof(sig->s));
 
-	add_push_bytes(scriptp, der, sizeof(der));
+    // 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 >= len) 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) != len) 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;
+}
+
+/* DER encode a value, return length used. */
+static size_t der_encode_val(const u8 *val, u8 *der)
+{
+	size_t len = 0;
+
+	der[len++] = 0x2; /* value type. */
+	/* Add zero byte if it would otherwise be signed. */
+	if (val[0] & 0x80) {
+		der[len++] = 33; /* value length */
+		der[len++] = 0;
+	} else
+		der[len++] = 32; /* value length */
+
+	memcpy(der + len, val, 32);
+	return len + 32;
+}
+	
+/* Bitcoin wants DER encoding. */
+static void add_push_sig(u8 **scriptp, const struct bitcoin_signature *sig)
+{
+	u8 der[2 + 2 + 1 + sizeof(sig->sig.r) + 2 + 1 + sizeof(sig->sig.s) + 1];
+	size_t len = 0;
+
+	der[len++] = 0x30; /* Type */
+	der[len++] = 0; /* Total length after this: fill it at end. */
+
+	len += der_encode_val(sig->sig.r, der + len);
+	len += der_encode_val(sig->sig.s, der + len);
+
+	/* Fix up total length */
+	der[1] = len - 2;
+
+	/* Append sighash type */
+	der[len++] = sig->stype;
+
+	assert(IsValidSignatureEncoding(der, len));
+	add_push_bytes(scriptp, der, len);
 }
 
 /* FIXME: permute? */
@@ -155,7 +247,7 @@ u8 *scriptpubkey_pay_to_pubkeyhash(const tal_t *ctx,
 
 u8 *scriptsig_pay_to_pubkeyhash(const tal_t *ctx,
 				const struct pubkey *key,
-				const struct signature *sig)
+				const struct bitcoin_signature *sig)
 {
 	u8 *script = tal_arr(ctx, u8, 0);
 
@@ -166,8 +258,8 @@ u8 *scriptsig_pay_to_pubkeyhash(const tal_t *ctx,
 }
 
 u8 *scriptsig_p2sh_2of2(const tal_t *ctx,
-			const struct signature *sig1,
+			const struct bitcoin_signature *sig1,
-			const struct signature *sig2,
+			const struct bitcoin_signature *sig2,
 			const struct pubkey *key1,
 			const struct pubkey *key2)
 {

bitcoin_script.h

@@ -2,12 +2,18 @@
 #define LIGHTNING_BITCOIN_SCRIPT_H
 #include <ccan/short_types/short_types.h>
 #include <ccan/tal/tal.h>
+#include "signature.h"
 
 struct bitcoin_address;
 struct pubkey;
-struct signature;
 struct sha256;
 
+/* A bitcoin signature includes one byte for the type. */
+struct bitcoin_signature {
+	struct signature sig;
+	enum sighash_type stype;
+};
+
 /* tal_count() gives the length of the script. */
 u8 *bitcoin_redeem_2of2(const tal_t *ctx,
 			const struct pubkey *key1,
@@ -36,12 +42,12 @@ u8 *scriptpubkey_pay_to_pubkeyhash(const tal_t *ctx,
 /* Create an input script to accept pay to pubkey */
 u8 *scriptsig_pay_to_pubkeyhash(const tal_t *ctx,
 				const struct pubkey *key,
-				const struct signature *sig);
+				const struct bitcoin_signature *sig);
 
 /* Create an input script to accept pay to pubkey */
 u8 *scriptsig_p2sh_2of2(const tal_t *ctx,
-			const struct signature *sig1,
+			const struct bitcoin_signature *sig1,
-			const struct signature *sig2,
+			const struct bitcoin_signature *sig2,
 			const struct pubkey *key1,
 			const struct pubkey *key2);
 

check-commit-sig.c

@@ -31,7 +31,7 @@ int main(int argc, char *argv[])
 	u8 *tx_arr;
 	size_t *inmap, *outmap;
 	struct pubkey pubkey1, pubkey2;
-	struct signature *sig1, sig2;
+	struct bitcoin_signature sig1, sig2;
 	char *tx_hex;
 	EC_KEY *privkey;
 	bool testnet;
@@ -78,22 +78,25 @@ int main(int argc, char *argv[])
 	/* FIXME: Creating out signature just to check the script we create
 	 * is overkill: if their signature and pubkey signed the commit txin,
 	 * we're happy. */
-	sig1 = sign_tx_input(ctx, commit, 0, anchor->output[outmap[0]].script,
+	sig1.stype = SIGHASH_ALL;
-			     anchor->output[outmap[0]].script_length, privkey);
+	sign_tx_input(ctx, commit, 0, anchor->output[outmap[0]].script,
+		      anchor->output[outmap[0]].script_length, privkey,
+		      &sig1.sig);
 
 	/* Signatures and pubkeys well-formed? */
-	if (!proto_to_signature(cs2->sig, &sig2))
+	if (!proto_to_signature(cs2->sig, &sig2.sig))
 		errx(1, "Invalid commit-sig-2");
+	sig2.stype = SIGHASH_ALL;
 	if (!proto_to_pubkey(o2->anchor->pubkey, &pubkey2))
 		errx(1, "Invalid anchor-2 key");
 
 	/* Combined signatures must validate correctly. */
 	if (!check_2of2_sig(commit, 0, &anchor->output[outmap[0]],
-			    &pubkey1, &pubkey2, sig1, &sig2))
+			    &pubkey1, &pubkey2, &sig1, &sig2))
 		errx(1, "Signature failed");
 
 	/* Create p2sh input for commit */
-	commit->input[0].script = scriptsig_p2sh_2of2(commit, sig1, &sig2,
+	commit->input[0].script = scriptsig_p2sh_2of2(commit, &sig1, &sig2,
 						      &pubkey1, &pubkey2);
 	commit->input[0].script_length = tal_count(commit->input[0].script);
 

open-anchor-scriptsigs.c

@@ -26,17 +26,17 @@ static u8 *tx_scriptsig(const tal_t *ctx,
 			EC_KEY *privkey,
 			const struct pubkey *pubkey)
 {
-	struct signature *sig;
+	struct bitcoin_signature sig;
 
-	sig = sign_tx_input(ctx, tx, i,
+	sig.stype = SIGHASH_ALL;
-			    input->subscript.data, input->subscript.len,
+	if (!sign_tx_input(ctx, tx, i,
-			    privkey);
+			   input->subscript.data, input->subscript.len,
-	if (!sig)
+			   privkey, &sig.sig))
 		return NULL;
 
 	if (!is_pay_to_pubkey_hash(input->subscript.data, input->subscript.len))
 		errx(1, "FIXME: Don't know how to handle input");
-	return scriptsig_pay_to_pubkeyhash(ctx, pubkey, sig);
+	return scriptsig_pay_to_pubkeyhash(ctx, pubkey, &sig);
 }
 	
 int main(int argc, char *argv[])

open-commit-sig.c

@@ -28,7 +28,7 @@ int main(int argc, char *argv[])
 	struct bitcoin_tx *anchor, *commit;
 	struct sha256_double txid;
 	struct pkt *pkt;
-	struct signature *sig;
+	struct signature sig;
 	size_t *inmap, *outmap;
 	EC_KEY *privkey;
 	bool testnet;
@@ -75,10 +75,10 @@ int main(int argc, char *argv[])
 		     (long long)o2->commitment_fee);
 
 	/* Sign it for them. */
-	sig = sign_tx_input(ctx, commit, 0, anchor->output[outmap[0]].script,
+	sign_tx_input(ctx, commit, 0, anchor->output[outmap[0]].script,
-			    anchor->output[outmap[0]].script_length, privkey);
+		      anchor->output[outmap[0]].script_length, privkey, &sig);
 
-	pkt = open_commit_sig_pkt(ctx, sig);
+	pkt = open_commit_sig_pkt(ctx, &sig);
 	if (!write_all(STDOUT_FILENO, pkt,
 		       sizeof(pkt->len) + le32_to_cpu(pkt->len)))
 		err(1, "Writing out packet");

signature.c

@@ -8,16 +8,16 @@
 #include <assert.h>
 #include <ccan/cast/cast.h>
 
-struct signature *sign_hash(const tal_t *ctx, EC_KEY *private_key,
+bool sign_hash(const tal_t *ctx, EC_KEY *private_key,
-			    const struct sha256_double *h)
+	       const struct sha256_double *h,
+	       struct signature *s)
 {
 	ECDSA_SIG *sig;
 	int len;
-	struct signature *s;
 	
 	sig = ECDSA_do_sign(h->sha.u.u8, sizeof(*h), private_key);
 	if (!sig)
-		return NULL;
+		return false;
 
 	/* See https://github.com/sipa/bitcoin/commit/a81cd9680.
 	 * There can only be one signature with an even S, so make sure we
@@ -35,7 +35,8 @@ struct signature *sign_hash(const tal_t *ctx, EC_KEY *private_key,
 		assert(!BN_is_odd(sig->s));
         }
 
-	s = talz(ctx, struct signature);
+	/* In case numbers are small. */
+	memset(s, 0, sizeof(*s));
 
 	/* Pack r and s into signature, 32 bytes each. */
 	len = BN_num_bytes(sig->r);
@@ -46,7 +47,7 @@ struct signature *sign_hash(const tal_t *ctx, EC_KEY *private_key,
 	BN_bn2bin(sig->s, s->s + sizeof(s->s) - len);
 
 	ECDSA_SIG_free(sig);
-	return s;
+	return true;
 }
 
 /* Only does SIGHASH_ALL */
@@ -77,15 +78,16 @@ static void sha256_tx_one_input(struct bitcoin_tx *tx,
 	tx->input[input_num].script = NULL;
 }
 
-struct signature *sign_tx_input(const tal_t *ctx, struct bitcoin_tx *tx,
+/* Only does SIGHASH_ALL */
-				unsigned int in,
+bool sign_tx_input(const tal_t *ctx, struct bitcoin_tx *tx,
-				const u8 *subscript, size_t subscript_len,
+		   unsigned int in,
-				EC_KEY *privkey)
+		   const u8 *subscript, size_t subscript_len,
+		   EC_KEY *privkey, struct signature *sig)
 {
 	struct sha256_double hash;
 
 	sha256_tx_one_input(tx, in, subscript, subscript_len, &hash);
-	return sign_hash(ctx, privkey, &hash);
+	return sign_hash(ctx, privkey, &hash, sig);
 }
 
 static bool check_signed_hash(const struct sha256_double *hash,
@@ -137,7 +139,8 @@ out:
 bool check_2of2_sig(struct bitcoin_tx *tx, size_t input_num,
 		    const struct bitcoin_tx_output *output,
 		    const struct pubkey *key1, const struct pubkey *key2,
-		    const struct signature *sig1, const struct signature *sig2)
+		    const struct bitcoin_signature *sig1,
+		    const struct bitcoin_signature *sig2)
 {
 	struct sha256_double hash;
 	assert(input_num < tx->input_count);
@@ -146,8 +149,12 @@ bool check_2of2_sig(struct bitcoin_tx *tx, size_t input_num,
 	sha256_tx_one_input(tx, input_num,
 			    output->script, output->script_length, &hash);
 
-	return check_signed_hash(&hash, sig1, key1)
+	/* We only use SIGHASH_ALL for the moment. */
-		&& check_signed_hash(&hash, sig2, key2);
+	if (sig1->stype != SIGHASH_ALL || sig2->stype != SIGHASH_ALL)
+		return false;
+	
+	return check_signed_hash(&hash, &sig1->sig, key1)
+		&& check_signed_hash(&hash, &sig2->sig, key2);
 }
 
 Signature *signature_to_proto(const tal_t *ctx, const struct signature *sig)

signature.h

@@ -22,20 +22,23 @@ struct sha256_double;
 struct bitcoin_tx;
 struct pubkey;
 struct bitcoin_tx_output;
+struct bitcoin_signature;
 
-struct signature *sign_hash(const tal_t *ctx, EC_KEY *private_key,
+bool sign_hash(const tal_t *ctx, EC_KEY *private_key,
-			    const struct sha256_double *h);
+	       const struct sha256_double *h,
+	       struct signature *s);
 
 /* All tx input scripts must be set to 0 len. */
-struct signature *sign_tx_input(const tal_t *ctx,
+bool sign_tx_input(const tal_t *ctx, struct bitcoin_tx *tx,
-				struct bitcoin_tx *tx, unsigned int in,
+		   unsigned int in,
-				const u8 *subscript, size_t subscript_len,
+		   const u8 *subscript, size_t subscript_len,
-				EC_KEY *privkey);
+		   EC_KEY *privkey, struct signature *sig);
 
 bool check_2of2_sig(struct bitcoin_tx *tx, size_t input_num,
 		    const struct bitcoin_tx_output *spending,
 		    const struct pubkey *key1, const struct pubkey *key2,
-		    const struct signature *sig1, const struct signature *sig2);
+		    const struct bitcoin_signature *sig1,
+		    const struct bitcoin_signature *sig2);
 
 /* Convert to-from protobuf to internal representation. */
 Signature *signature_to_proto(const tal_t *ctx, const struct signature *sig);