Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Userdata in sets always set key to big endian #123

Open
geNAZt opened this issue May 17, 2021 · 5 comments
Open

Userdata in sets always set key to big endian #123

geNAZt opened this issue May 17, 2021 · 5 comments

Comments

@geNAZt
Copy link

geNAZt commented May 17, 2021

There is static userdata inside the set marshal:

	if s.Anonymous || s.Constant || s.Interval {
		tableInfo = append(tableInfo,
			// Semantically useless - kept for binary compatability with nft
			netlink.Attribute{Type: unix.NFTA_SET_USERDATA, Data: []byte("\x00\x04\x02\x00\x00\x00")})
	}

https://github.com/google/nftables/blob/master/set.go#L386

According to nftables code:

static int set_parse_udata_cb(const struct nftnl_udata *attr, void *data)
{
	unsigned char *value = nftnl_udata_get(attr);
	const struct nftnl_udata **tb = data;
	uint8_t type = nftnl_udata_type(attr);
	uint8_t len = nftnl_udata_len(attr);

	switch (type) {
	case NFTNL_UDATA_SET_KEYBYTEORDER:
	case NFTNL_UDATA_SET_DATABYTEORDER:
	case NFTNL_UDATA_SET_MERGE_ELEMENTS:
	case NFTNL_UDATA_SET_DATA_INTERVAL:
		if (len != sizeof(uint32_t))
			return -1;
		break;
	case NFTNL_UDATA_SET_KEY_TYPEOF:
	case NFTNL_UDATA_SET_DATA_TYPEOF:
		if (len < 3)
			return -1;
		break;
	case NFTNL_UDATA_SET_COMMENT:
		if (value[len - 1] != '\0')
			return -1;
		break;
	default:
		return 0;
	}
	tb[type] = attr;
	return 0;
}

https://git.netfilter.org/nftables/tree/src/netlink.c#n715

When i look up the enum in the nft lib i found that keybyteorder and databyteorder are values 0 and 1:

enum nftnl_udata_set_types {
	NFTNL_UDATA_SET_KEYBYTEORDER,
	NFTNL_UDATA_SET_DATABYTEORDER,
	NFTNL_UDATA_SET_MERGE_ELEMENTS,
	NFTNL_UDATA_SET_KEY_TYPEOF,
	NFTNL_UDATA_SET_DATA_TYPEOF,
	NFTNL_UDATA_SET_EXPR,
	NFTNL_UDATA_SET_DATA_INTERVAL,
	NFTNL_UDATA_SET_COMMENT,
	__NFTNL_UDATA_SET_MAX
};

https://git.netfilter.org/libnftnl/tree/include/libnftnl/udata.h#n40

I also found this byteoder enum: http://charette.no-ip.com:81/programming/doxygen/netfilter/datatype_8h.html#aec0612005abb2f2a88c2de638a442cf2

This would set the key data type to be big endian encoded. I can also observe that when i use this example code:

	workerSetElements := make([]nftables.SetElement, len(setElements))
	for i, v := range setElements {
		bs := make([]byte, 4)
		binary.LittleEndian.PutUint32(bs, uint32(i))
		workerSetElements[i] = nftables.SetElement{
			Key:         bs,
			Val:         ip2array(v),
			IntervalEnd: false,
		}
	}

This generates a netlink message like this:

May 17 06:44:01 ip-10-0-1-19 nftables-controller[31154]: nl: send msgs: {Header:{Length:116 Type:unknown(2572) Flags:request|acknowledge|0x400 Sequence:3804372827 PID:31154} Data:[2 0 0 0 12 0 2 0 95 95 109 97 112 37 100 0 8 0 4 0 0 0 0 1 14 0 1 0 115 116 97 116 101 108 101 115 115 0 0 0 60 0 3 128 28 0 1 128 12 0 1 128 8 0 1 0 0 0 0 0 12 0 2 128 8 0 1 0 10 0 1 225 28 0 2 128 12 0 1 128 8 0 1 0 1 0 0 0 12 0 2 128 8 0 1 0 10 0 1 82]}

When i look into how the second key is defined we can see that its correctly little endian: 8 0 1 0 1 0 0 0

Here is the userdata from nftables when i create the same set with it:

|00016|--|00013|        |len |flags| type|
| 00 04 01 00  |        |      data      |
| 00 00 01 04  |        |      data      |
| 02 00 00 00  |        |      data      |

I can observe that it sets key 0 (endianness for key) to 1 (LE) and for key 1 (data) to 2 (BE).

It would be awesome to control userdata in that case so that nft does show the correct output and not display LE uint32 as BE which confuses operations :D

Thanks

@stapelberg
Copy link
Collaborator

cc @sbezverk

@sbezverk
Copy link
Contributor

@geNAZt Since I did not have and still do not have any little endian platform to play with, the key in userdata has not been tested with it. As far as I remember go does not offer great runtime detection of the endianness so I suspect one needs to use a build-time variable to address the target endianness.

@stapelberg
Copy link
Collaborator

As far as I remember go does not offer great runtime detection of the endianness so I suspect one needs to use a build-time variable to address the target endianness.

Our github.com/google/nftables/binaryutil package uses github.com/koneu/natend for endianness detection. Thus far, I haven’t seen any problem reports about that :)

@geNAZt
Copy link
Author

geNAZt commented May 25, 2021

Ok, my expectation would be that i can set UserData (like any other argument) since this is a very low level lib. I would not want to have the library "guess" the correct UserData, i would like to tell the lib which values to use in that case.

@raphaelschnaitl
Copy link

Any idea how to overcome the display bug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants