On Wed, Apr 23, 2014 at 10:59:25AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
>
> An IP or IPv6 rule with port specification but without protocol
> specification cannot be instantiated by ebtables. The documentation
> points to 'protocol' being required but implementation does not
> enforce it to be given.
>
> Implement a rule validation function that checks whether the rule is
> valid when it is defined. This for example prevents the definition
> of rules like:
>
> <ip dstportstart='53'>
>
> where a protocol attribute would be required for it to be valid and for
> ebtables to be able to instantiate it. A valid rule then is:
>
> <ip protocol='udp' dstportstart='53'>
>
> Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
>
> Changes:
> v1->v2:
> - fixed access to ipv6 structures
>
> ---
> src/conf/nwfilter_conf.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
> index f5a75e4..69b1d97 100644
> --- a/src/conf/nwfilter_conf.c
> +++ b/src/conf/nwfilter_conf.c
> @@ -2093,6 +2093,66 @@ virNWFilterRuleDefFixupIPSet(ipHdrDataDefPtr ipHdr)
> }
> }
>
> +
> +/*
> + * virNWFilterRuleValidate
> + *
> + * Perform some basic rule validation to prevent rules from being
> + * defined that cannot be instantiated.
> + */
> +static int
> +virNWFilterRuleValidate(virNWFilterRuleDefPtr rule)
> +{
> + int ret = 0;
> + portDataDefPtr portData = NULL;
> + nwItemDescPtr dataProtocolID;
> + const char *protocol;
> +
> + switch (rule->prtclType) {
> + case VIR_NWFILTER_RULE_PROTOCOL_IP:
> + portData = &rule->p.ipHdrFilter.portData;
> + protocol = "IP";
> + dataProtocolID = &rule->p.ipHdrFilter.ipHdr.dataProtocolID;
> + /* fall through */
> + case VIR_NWFILTER_RULE_PROTOCOL_IPV6:
> + if (portData == NULL) {
> + portData = &rule->p.ipv6HdrFilter.portData;
> + protocol = "IPv6";
> + dataProtocolID = &rule->p.ipv6HdrFilter.ipHdr.dataProtocolID;
> + }
> + if (HAS_ENTRY_ITEM(&portData->dataSrcPortStart) ||
> + HAS_ENTRY_ITEM(&portData->dataDstPortStart) ||
> + HAS_ENTRY_ITEM(&portData->dataSrcPortEnd) ||
> + HAS_ENTRY_ITEM(&portData->dataDstPortEnd)) {
> + if (HAS_ENTRY_ITEM(dataProtocolID)) {
> + switch (dataProtocolID->u.u8) {
> + case 6: /* tcp */
> + case 17: /* udp */
> + case 33: /* dccp */
> + case 132: /* sctp */
> + break;
> + default:
> + ret = -1;
> + }
> + } else {
> + ret = -1;
> + }
> + if (ret < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("%s rule with port specification requires "
> + "protocol specification with protocol to be "
> + "either one of tcp(6), udp(17), dccp(33), or "
> + "sctp(132)"), protocol);
> + }
> + }
> + break;
> + default:
> + break;
Nitpick, the 'break' statements should be indented 4 more spaces.
> + }
> +
> + return ret;
> +}
> +
> static void
> virNWFilterRuleDefFixup(virNWFilterRuleDefPtr rule)
> {
> @@ -2389,6 +2449,8 @@ virNWFilterRuleParse(xmlNodePtr node)
> virAttr[i].att) < 0) {
> goto err_exit;
> }
> + if (virNWFilterRuleValidate(ret) < 0)
> + goto err_exit;
> break;
> }
> if (!found) {
ACK with the indentation fix. Fine to push in freeze IMHO.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list