On Sat, Jan 08, 2011 at 08:04:33PM +0100, Marcel Wiget wrote:
> On Jan 8, 2011, at 4:20 PM, Claudio Jeker wrote:
>
> > Commited the diff. I guess there is a bit more needed so that we handle
> > various unknown TLVs correctly in hello and initializaion. I hope I can
> > provide a diff for this soon. Btw. I would be interested in the ldpd -dv
> > output of the failures you get when the JUNOS has RFC 3479 enabled or when
> > a different transport addr is used.
>
> Thanks! Without 'set protocols ldp graceful-restart helper-disable' on junos,
> I get the following output from ldpd -dv. Packet dump of the LDP
> initialization message and openbsd's response further down:
>
Thanks for all the info. Can you try the following diff?
This should allow unkown TLVs in hello and initialization messages.
This should signal the JUNOS neighbor that it needs to disable FT.
--
:wq Claudio
Index: hello.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldpd/hello.c,v
retrieving revision 1.8
diff -u -p -r1.8 hello.c
--- hello.c 8 Jan 2011 14:50:29 -0000 1.8
+++ hello.c 9 Jan 2011 14:55:12 -0000
@@ -202,32 +202,37 @@ int
tlv_decode_opt_hello_prms(char *buf, u_int16_t len, struct in_addr *addr,
u_int32_t *conf_number)
{
- struct hello_opt_parms_tlv tlv;
- int cons = 0;
+ struct tlv tlv;
+ int cons = 0;
+ u_int16_t tlv_len;
bzero(addr, sizeof(*addr));
*conf_number = 0;
while (len >= sizeof(tlv)) {
bcopy(buf, &tlv, sizeof(tlv));
-
- if (ntohs(tlv.length) < sizeof(u_int32_t))
- return (-1);
-
+ tlv_len = ntohs(tlv.length);
switch (ntohs(tlv.type)) {
case TLV_TYPE_IPV4TRANSADDR:
- addr->s_addr = tlv.value;
+ if (tlv_len < sizeof(u_int32_t))
+ return (-1);
+ bcopy(buf + TLV_HDR_LEN, addr, sizeof(u_int32_t));
break;
case TLV_TYPE_CONFIG:
- *conf_number = ntohl(tlv.value);
+ if (tlv_len < sizeof(u_int32_t))
+ return (-1);
+ bcopy(buf + TLV_HDR_LEN, conf_number,
+ sizeof(u_int32_t));
break;
default:
- return (-1);
+ /* if unknown flag set, ignore TLV */
+ if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
+ return (-1);
+ break;
}
-
- len -= sizeof(tlv);
- buf += sizeof(tlv);
- cons += sizeof(tlv);
+ buf += TLV_HDR_LEN + tlv_len;
+ len -= TLV_HDR_LEN + tlv_len;
+ cons += TLV_HDR_LEN + tlv_len;
}
return (cons);
Index: init.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldpd/init.c,v
retrieving revision 1.6
diff -u -p -r1.6 init.c
--- init.c 4 Nov 2010 09:52:16 -0000 1.6
+++ init.c 9 Jan 2011 14:55:35 -0000
@@ -38,6 +38,7 @@
#include "ldpe.h"
int gen_init_prms_tlv(struct ibuf *, struct nbr *, u_int16_t);
+int tlv_decode_opt_init_prms(char *, u_int16_t);
void
send_init(struct nbr *nbr)
@@ -88,12 +89,19 @@ recv_init(struct nbr *nbr, char *buf, u_
bcopy(buf, &sess, sizeof(sess));
if (ntohs(sess.length) != SESS_PRMS_SIZE - TLV_HDR_LEN ||
- ntohs(sess.length) != len - TLV_HDR_LEN) {
+ ntohs(sess.length) > len - TLV_HDR_LEN) {
session_shutdown(nbr, S_BAD_TLV_LEN, init.msgid, init.type);
return (-1);
}
- /* ATM and Frame Relay optional attributes not supported */
+ buf += SESS_PRMS_SIZE;
+ len -= SESS_PRMS_SIZE;
+
+ /* just ignore all optional TLVs for now */
+ if (tlv_decode_opt_init_prms(buf, len) == -1) {
+ session_shutdown(nbr, S_BAD_TLV_VAL, init.msgid, init.type);
+ return (-1);
+ }
if (nbr->iface->keepalive < ntohs(sess.keepalive_time))
nbr->keepalive = nbr->iface->keepalive;
@@ -126,4 +134,35 @@ gen_init_prms_tlv(struct ibuf *buf, stru
parms.lspace_id = 0;
return (ibuf_add(buf, &parms, SESS_PRMS_SIZE));
+}
+
+int
+tlv_decode_opt_init_prms(char *buf, u_int16_t len)
+{
+ struct tlv tlv;
+ int cons = 0;
+ u_int16_t tlv_len;
+
+ while (len >= sizeof(tlv)) {
+ bcopy(buf, &tlv, sizeof(tlv));
+ tlv_len = ntohs(tlv.length);
+ switch (ntohs(tlv.type)) {
+ case TLV_TYPE_ATMSESSIONPAR:
+ log_warnx("ATM session parameter present");
+ return (-1);
+ case TLV_TYPE_FRSESSION:
+ log_warnx("FR session parameter present");
+ return (-1);
+ default:
+ /* if unknown flag set, ignore TLV */
+ if (!(ntohs(tlv.type) & UNKNOWN_FLAG))
+ return (-1);
+ break;
+ }
+ buf += TLV_HDR_LEN + tlv_len;
+ len -= TLV_HDR_LEN + tlv_len;
+ cons += TLV_HDR_LEN + tlv_len;
+ }
+
+ return (cons);
}
Index: labelmapping.c
===================================================================
RCS file: /cvs/src/usr.sbin/ldpd/labelmapping.c,v
retrieving revision 1.18
diff -u -p -r1.18 labelmapping.c
--- labelmapping.c 31 Dec 2010 21:22:42 -0000 1.18
+++ labelmapping.c 9 Jan 2011 14:33:51 -0000
@@ -93,7 +93,7 @@ int
recv_labelmapping(struct nbr *nbr, char *buf, u_int16_t len)
{
struct ldp_msg lm;
- struct fec_tlv ft;
+ struct tlv ft;
struct map map;
u_int32_t label;
int feclen, lbllen, tlen;
@@ -202,7 +202,7 @@ int
recv_labelrequest(struct nbr *nbr, char *buf, u_int16_t len)
{
struct ldp_msg lr;
- struct fec_tlv ft;
+ struct tlv ft;
struct map map;
int feclen, tlen;
u_int8_t addr_type;
@@ -308,7 +308,7 @@ recv_labelwithdraw(struct nbr *nbr, char
{
struct map map;
struct ldp_msg lw;
- struct fec_tlv ft;
+ struct tlv ft;
u_int32_t label = NO_LABEL;
int feclen, tlen, numfec = 0;
u_int8_t addr_type;
@@ -449,7 +449,7 @@ recv_labelrelease(struct nbr *nbr, char
{
struct map map;
struct ldp_msg lr;
- struct fec_tlv ft;
+ struct tlv ft;
u_int32_t label = NO_LABEL;
int feclen, tlen, numfec = 0;
u_int8_t addr_type;
@@ -568,7 +568,7 @@ recv_labelabortreq(struct nbr *nbr, char
{
struct map map;
struct ldp_msg la;
- struct fec_tlv ft;
+ struct tlv ft;
int feclen, tlen;
u_int8_t addr_type;
@@ -707,7 +707,7 @@ tlv_decode_reqid(char *buf, u_int16_t le
void
gen_fec_tlv(struct ibuf *buf, struct in_addr prefix, u_int8_t prefixlen)
{
- struct fec_tlv ft;
+ struct tlv ft;
u_int8_t type;
u_int16_t family;
u_int8_t len;
Index: ldp.h
===================================================================
RCS file: /cvs/src/usr.sbin/ldpd/ldp.h,v
retrieving revision 1.7
diff -u -p -r1.7 ldp.h
--- ldp.h 4 Nov 2010 09:52:16 -0000 1.7
+++ ldp.h 9 Jan 2011 14:34:20 -0000
@@ -97,6 +97,12 @@ struct ldp_hdr {
#define INFINITE_HOLDTIME 0xffff
/* TLV record */
+struct tlv {
+ u_int16_t type;
+ u_int16_t length;
+};
+#define TLV_HDR_LEN 4
+
struct ldp_msg {
u_int16_t type;
u_int16_t length;
@@ -106,7 +112,6 @@ struct ldp_msg {
} __packed;
#define LDP_MSG_LEN 8
-#define TLV_HDR_LEN 4
#define UNKNOWN_FLAGS_MASK 0xc000
#define UNKNOWN_FLAG 0x8000
@@ -189,12 +194,6 @@ struct address_list_tlv {
#define ADDR_IPV4 0x1
#define ADDR_IPV6 0x2
-struct fec_tlv {
- u_int16_t type;
- u_int16_t length;
- /* fec elm entries */
-};
-
/* This struct is badly aligned so use two 32 bit fields */
struct fec_elm {
u_int32_t hdr;
@@ -202,10 +201,8 @@ struct fec_elm {
};
#define FEC_ELM_MIN_LEN 4
-
#define FEC_WILDCARD 0x01
#define FEC_PREFIX 0x02
-
#define FEC_IPV4 0x0001
struct label_tlv {
@@ -223,12 +220,6 @@ struct reqid_tlv {
};
#define REQID_TLV_LEN 8
-
-struct hello_opt_parms_tlv {
- u_int16_t type;
- u_int16_t length;
- u_int32_t value;
-};
#define NO_LABEL UINT_MAX
