common: don't crash on bad sphinx payload.

It's cleanest to eliminate the SPHINX_INVALID_PAYLOAD altogether.

lightning_channeld: FATAL SIGNAL (version v0.7.3-242-gb1583bb-modded)
0x55a8169eed08 send_backtrace
	common/daemon.c:41
0x55a8169fc3eb status_failed
	common/status.c:206
0x55a8169fc657 status_backtrace_exit
	common/subdaemon.c:25
0x55a8169eedbb crashdump
	common/daemon.c:57
0x7f0eaff8446f ???
	???:0
0x7f0eaff843eb ???
	???:0
0x7f0eaff63898 ???
	???:0
0x55a8169fb29f route_step_decode
	common/sphinx.c:759
0x55a8169fb60a process_onionpacket
	common/sphinx.c:834
0x55a8169d9b34 get_shared_secret
	channeld/channeld.c:605
0x55a8169d9d35 handle_peer_add_htlc
	channeld/channeld.c:649
0x55a8169dd88d peer_in
	channeld/channeld.c:1838
0x55a8169e11a8 main
	channeld/channeld.c:3233
0x7f0eaff651e2 ???

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

common/sphinx.c

@@ -549,15 +549,13 @@ static bool sphinx_write_frame(u8 *dest, const struct sphinx_hop *hop)
 	return true;
 }
 
-static void sphinx_parse_payload(struct route_step *step, const u8 *src)
+static bool sphinx_parse_payload(struct route_step *step, const u8 *src)
 {
 	size_t hop_size, vsize;
 	bigsize_t raw_size;
 #if !EXPERIMENTAL_FEATURES
-	if (src[0] != 0x00) {
-		step->type = SPHINX_INVALID_PAYLOAD;
-		return;
-	}
+	if (src[0] != 0x00)
+		return false;
 #endif
 
 	/* BOLT #4:
@@ -583,8 +581,7 @@ static void sphinx_parse_payload(struct route_step *step, const u8 *src)
 		hop_size = raw_size + vsize + HMAC_SIZE;
 		step->type = SPHINX_TLV_PAYLOAD;
 	} else {
-		step->type = SPHINX_INVALID_PAYLOAD;
-		return;
+		return false;
 	}
 
 	/* Copy common pieces over */
@@ -607,10 +604,10 @@ static void sphinx_parse_payload(struct route_step *step, const u8 *src)
 
 		if (!fromwire_tlv_payload(&tlv, &max, step->payload.tlv)) {
 			/* FIXME: record offset of violation for error! */
-			step->type = SPHINX_INVALID_PAYLOAD;
-			return;
+			return false;
 		}
 	}
+	return true;
 }
 
 struct onionpacket *create_onionpacket(
@@ -754,7 +751,6 @@ static void route_step_decode(struct route_step *rs)
 		}
 #endif
 		break;
-	case SPHINX_INVALID_PAYLOAD:
 	case SPHINX_RAW_PAYLOAD:
 		abort();
 	}
@@ -803,7 +799,8 @@ struct route_step *process_onionpacket(
 	if (!blind_group_element(&step->next->ephemeralkey, &msg->ephemeralkey, blind))
 		return tal_free(step);
 
-	sphinx_parse_payload(step, paddedheader);
+	if (!sphinx_parse_payload(step, paddedheader))
+		return tal_free(step);
 
 	/* Extract how many bytes we need to shift away */
 	if (paddedheader[0] == 0x00) {

common/sphinx.h

@@ -71,7 +71,6 @@ struct hop_data_legacy {
 enum sphinx_payload_type {
 	SPHINX_V0_PAYLOAD = 0,
 	SPHINX_TLV_PAYLOAD = 1,
-	SPHINX_INVALID_PAYLOAD = 254,
 	SPHINX_RAW_PAYLOAD = 255,
 };
 

lightningd/peer_htlcs.c

@@ -962,7 +962,8 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id,
 
 	/* FIXME: Have channeld hand through just the route_step! */
 
-	/* channeld tests this, so it should pass. */
+	/* channeld calls both parse_onionpacket and process_onionpacket,
+	 * so they should succeed.. */
 	op = parse_onionpacket(tmpctx, hin->onion_routing_packet,
 			       sizeof(hin->onion_routing_packet),
 			       failcode);
@@ -974,7 +975,6 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id,
 		return false;
 	}
 
-	/* If it's crap, not channeld's fault, just fail it */
 	rs = process_onionpacket(tmpctx, op, hin->shared_secret->data,
 				 hin->payment_hash.u.u8,
 				 sizeof(hin->payment_hash));
@@ -986,12 +986,6 @@ static bool peer_accepted_htlc(struct channel *channel, u64 id,
 		return false;
 	}
 
-	/* Unknown realm isn't a bad onion, it's a normal failure. */
-	if (rs->type == SPHINX_INVALID_PAYLOAD) {
-		*failcode = WIRE_INVALID_REALM;
-		goto out;
-	}
-
 	hook_payload = tal(hin, struct htlc_accepted_hook_payload);
 
 	hook_payload->route_step = tal_steal(hook_payload, rs);