Security group names cannot contain spaces

[Rosario Di Somma]

The quantum security-group-create command allows Security group with spaces in the name to be created while Horizon produce an incorrect error message that says "The string may only contain ASCII characters and numbers.". The use of the validate_slug validator by the CreateGroup and UpdateGroup forms prevent the spaces to be added so we should either correct the misleading error message or consider a new validator to be coherent with the Quantum API.

[Gabriel Hurley (gabriel-hurley)]

This was based on a restriction in Nova. If the logic is different between Nova and Quantum we will need to customize the validator for each.

[Akihiro Motoki (amotoki)]

We need to be careful when allowing additional characters even when Neutron API allows it because nova has a proxy to neutron.
IMO, we need to ensure a security group can be used through both Neutron API directly and Nova Neutron security group proxy (before Nova v2 API becomes deprecated/removed) to avoid unnecessary confusion to users.

[Romain Hardouin (romain-hardouin)]

Gabriel, could you tell me more about Nova restriction? Is it obsolete?
I just tried to create a security group with a name containing spaces and it seems to work:

$ nova secgroup-create "A name with spaces" description
+--------------------------------------+--------------------+-------------+
| Id | Name | Description |
+--------------------------------------+--------------------+-------------+
| f9cbd4f0-aea5-41cc-8a14-1c945545acdb | A name with spaces | description |
+--------------------------------------+--------------------+-------------+

I want to fix the bug #1233501 (Security group names cannot contain at characters) and I've understood I could delete 'validate_slug'. Is it a mistake?

Akihiro, you're saying it's a bad idea to remove any validation?

[Akihiro Motoki (amotoki)]

I think it is no a good idea to remove a validation completely and some validation is required. We need to take care several cases. For example, does security group name with control characters/non-printable characters?

Regarding nova security group names, you need to investigate nova-network security group implementations. nova security group implementation adopts driver mechanism and there are two drivers at now (nova and neutron). Have you checked with both drivers? Horizon supports both drivers. Thus you need to take care both when relaxing the restrictions.

Of course it is a good direction to change the validation logic to more useful one.
Thanks for your work.

[Romain Hardouin (romain-hardouin)]

I have not checked with both drivers. I'll investigate nova-network as you mentioned it, and will try to set up a correct validation logic.
Thank you for these information.

[Matthew S (matts8484)]

Akihiro-san, if I understand correctly, you're saying that neither the Nova nor the Neutron APIs should allow someone to create a security group name that won't work via the other API, is that right?

In other words, it should not be possible for a user to use the Nova API to create a secgroup that has a name that is valid to Nova but is not valid to Neutron, then attempt to use the Neutron API to manage that secgroup, whereupon that user would see some failure or other.

Have I understood correctly? My apologies if not.

However, the topic of this ticket is the validation that's in Horizon, not the validation (if any) in Neutron and/or Nova.

I don't understand why there exists validation of security group names in Horizon, as well as the underlying layer(s). It introduces the potential for the validation to be performed differently, which is what we're seeing here - Horizon rejects @ characters and spaces, whereas Nova does not - 'nova secgroup-create' accepts both spaces and '@' characters.

Would it be possible to remove the validation in Horizon (and the error message when it fails, too), and simply have Horizon pass the values to Nova/Neutron, which do the validation, and then have Horizon report the validation failure (if any) to the user?

[Romain Hardouin (romain-hardouin)]

Akihiro, I tested with Nova-network and Neutron backends.
Spaces are allowed in both cases (as well as "@" bug #1233501).

My previous example (comments #3) used Neutron, proxied by Nova-net (hence UUID as ID).
As you can see below, I also tested with Nova-net (ID are integers):

$ nova secgroup-list
+----+--------------------+----------------------+
| Id | Name | Description |
+----+--------------------+----------------------+
| 2 | @test | dfgfd |
| 3 | A name with spaces | 3 spaces in the name |
| 1 | default | default |
+----+--------------------+----------------------+

In the dashboard, if we rely on backends validation as Matthew suggests,
and if we enter invalid data in the form -- e.g. spaces as group name --
an error pop-up appears: "Unable to create security group".

We can enhance this error message by adding the cause.
novaclient.ClientException has a message attribute therefore we can concatenate it:

except Exception as e:
    cause = getattr(e, 'message', str(e))
    error_msg = _('Unable to create security group: %s') % cause

Of course, we can still validate the name.
But in the end, probabilities to harm the user wouldn't be larger than probabilites to help him?
That's the case today: neither space nor @ are allowed.

What do you think?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/54007

[Romain Hardouin (romain-hardouin)]

The validation that I've submitted consists of:
 * A [:print:] regex.
 * A length limited to 255 characters in accordance with Nova and Neutron back ends.

Feedbacks are welcome :)

[Akihiro Motoki (amotoki)]

Sorry for my very late response.

My original and only concern is that if Horizon enforces more strict validation and a user first creates a security group with a name which is invalid as Horizon validator, when a user tries to update security group through Horizon the user is forced to change the name. However, this is a limited case and most users use Horizon for managing security groups if they create security groups in Horizon and vice versa. Thus I think it doesn't matter much even if Horizon has a validator as proposed in the review.

I still think that ideally Nova and Neutron API should use same validation rule, but it is not now.
In my understanding, nova only rejects empty or whitespace only names.
Neutron accepts all strings as name (because "name" is not used internally and just stored in the database).

In general, I am okay that Horizon only allows ASCII printable strings.

[Matthew S (matts8484)]

Akihiro, I think the simplest way to address your concern would be to remove all validation from Horizon. Then Horizon would never force the user to change the name in the way you describe (because absolutely all names would be valid to Horizon).

It's unfortunate that nova-network allows names with non-printing characters, control characters, whitespace etc. It's also unfortunate that Neutron allows empty names. These sorts of names could be very confusing to a Horizon user. However, I think that is a matter for nova-network and Neutron, so is nothing directly to do with Horizon.

If Horizon does not gracefully handle name validation failures from the networking back-ends, then I think that would be a separate bug in Horizon. Maybe it's a bug that's uncovered by fixing this bug.

In addition, I'm slighly concerned about XSS attacks if we allow HTML and Javascript special characters. But again, I think would be a separate bug (probably in Django or Horizon) that got uncovered by this one.

I personally am *not* okay with Horizon only allowing ASCII strings. I believe internationalisation of software is important, and will get even more important as time goes on. It is not the 1960s any more! :-)

So, I intend to submit a patch for your review which:
1. Removes security group name validation from Horizon; and
2. Ensures that when the back-end rejects a security group name, Horizon properly explains the exact reason for the rejection to the user.

If you think this is such a bad idea that I shouldn't even bother writing/submitting a patch, please tell me so.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/54007
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=80d9049fb6e6c7bf5eb262612e6ad3e8a541c051
Submitter: Jenkins
Branch: master

commit 80d9049fb6e6c7bf5eb262612e6ad3e8a541c051
Author: Romain Hardouin <email address hidden>
Date: Thu Apr 3 23:52:13 2014 +0200

    Allow all printable ASCII characters in security group names

    Thus far, the security group name was checked with Django's validate_slug.
    Consequently, some characters like spaces (bug #1224576) and @ (bug #1233501) were forbidden.
    This fix removes validate_slug and provides a less restrictive validation.
    Now, all printable ASCII characters -- i.e. from 0x20 to 0x7E -- are valid.
    Note that diacritics are disallowed.
    On top of that the length is now checked and limited to 255 characters in accordance with Nova.

    Security group forms have been factorized in one abstract base class: GroupBase.
    CreateGroup and UpdateGroup subclass GroupBase in order to be more DRY.

    Change-Id: Ifc8e5f75419a73a353b2138641773d36138ecea8
    Closes-Bug: #1224576
    Closes-Bug: #1233501