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

add interface name validation #712

Merged

Conversation

mars1024
Copy link
Member

@mars1024 mars1024 commented Sep 18, 2019

Interface name in linux is not unrestricted, so bad interface name will break CNI plugin.
This pr will add interface name validation to libcni and skel.

validation rules ref to https://github.com/torvalds/linux/blob/master/net/core/dev.c#L1024

libcni/api_test.go Outdated Show resolved Hide resolved
pkg/skel/skel_test.go Outdated Show resolved Hide resolved
pkg/skel/skel_test.go Outdated Show resolved Hide resolved
Signed-off-by: Bruce Ma <brucema19901024@gmail.com>
Signed-off-by: Bruce Ma <brucema19901024@gmail.com>
@mars1024 mars1024 force-pushed the modify/ifname_validation branch from efc4476 to d8845e8 Compare September 20, 2019 03:34
@mars1024
Copy link
Member Author

@jellonek all updated, ptal, thanks ~

pkg/utils/utils.go Outdated Show resolved Hide resolved
@mars1024 mars1024 force-pushed the modify/ifname_validation branch from d8845e8 to 71bebbc Compare September 23, 2019 06:13
pkg/utils/utils.go Outdated Show resolved Hide resolved
@mars1024 mars1024 force-pushed the modify/ifname_validation branch from 71bebbc to b9917fa Compare September 25, 2019 09:46
Copy link
Member

@jellonek jellonek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@dcbw
Copy link
Member

dcbw commented Oct 2, 2019

For reference, from net/core/dev.c:

/**
 *	dev_valid_name - check if name is okay for network device
 *	@name: name string
 *
 *	Network device names need to be valid file names to
 *	to allow sysfs to work.  We also disallow any kind of
 *	whitespace.
 */
bool dev_valid_name(const char *name)
{
	if (*name == '\0')
		return false;
	if (strnlen(name, IFNAMSIZ) == IFNAMSIZ)
		return false;
	if (!strcmp(name, ".") || !strcmp(name, ".."))
		return false;

	while (*name) {
		if (*name == '/' || *name == ':' || isspace(*name))
			return false;
		name++;
	}
	return true;
}

@squeed
Copy link
Member

squeed commented Oct 2, 2019

Aha, interesting. @mars1024 can you block "." and ".." too?

@jellonek
Copy link
Member

jellonek commented Oct 2, 2019

If we want to follow this kernel code - we should also block ":".

@mars1024
Copy link
Member Author

mars1024 commented Oct 8, 2019

@dcbw @squeed @jellonek Thanks for all your suggestions, I'll update this soon ~

@mars1024 mars1024 force-pushed the modify/ifname_validation branch from b9917fa to 5db3111 Compare October 8, 2019 03:43
@mars1024
Copy link
Member Author

mars1024 commented Oct 8, 2019

Updated!

…some comments

Signed-off-by: Bruce Ma <brucema19901024@gmail.com>
@mars1024 mars1024 force-pushed the modify/ifname_validation branch from 5db3111 to eefc069 Compare October 8, 2019 04:01
Copy link
Member

@jellonek jellonek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

5 participants