Red Hat Satellite engineering is moving the tracking of its product development work on Satellite to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "Satellite project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs will be migrated starting at the end of May. If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "Satellite project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/SAT-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1191913 - user should not be allowed to create interface with invalid mac e.g. any random string
Summary: user should not be allowed to create interface with invalid mac e.g. any rand...
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: WebUI
Version: 6.1.0
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: Unspecified
Assignee: Marek Hulan
QA Contact: Tazim Kolhar
URL:
Whiteboard:
Depends On:
Blocks: 1193460 1193987 1194641
TreeView+ depends on / blocked
 
Reported: 2015-02-12 08:58 UTC by Sachin Ghai
Modified: 2017-02-23 20:32 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-08-12 13:58:07 UTC
Target Upstream Version:
Embargoed:

Attachments (Terms of Use)
see the mac field contains string "sachin" and form is saved. (33.34 KB, image/png)
2015-02-12 08:58 UTC, Sachin Ghai 		no flags Details
Mac address (49.36 KB, image/png)
2015-04-28 10:00 UTC, Tazim Kolhar 		no flags Details
foreman-debug attached (1.62 MB, application/x-xz)
2015-04-28 10:01 UTC, Tazim Kolhar 		no flags Details
invalid mac address (56.44 KB, image/png)
2015-05-11 10:53 UTC, Tazim Kolhar 		no flags Details
View All

Description Sachin Ghai 2015-02-12 08:58:54 UTC 
Created attachment 990803 [details]
see the mac field contains string "sachin" and form is saved.

Description of problem:
I created a new interface and used a random string for mac. It was accepted and form was saved. However user should n't be allowed to create interface with invalid mac. A proper validation needs to be set on mac field.

Version-Release number of selected component (if applicable):
Satellite-6.1.0-RHEL-6-20150210.0
Sat6.1.0 beta snap2

How reproducible:
always 

Steps to Reproduce:
1. New Host --> Add_interface
2.
3.

Actual results:
user can create interface with invalid mac

Expected results:
user should n't be allowed to create interface with invalid mac. A proper validation needs to be set on mac field.

Additional info:
production.log:

Processing by InterfacesController#new as JS
  Parameters: {"host"=>{"interfaces_attributes"=>{"new_1423727783901"=>{"type"=>"Nic::Managed", "mac"=>"sachin", "identifier"=>"", "name"=>"", "domain_id"=>"", "subnet_id"=>"", "ip"=>"", "managed"=>"1", "username"=>"", "password"=>"[FILTERED]", "provider"=>"IPMI"}}}, "fakepassword"=>"[FILTERED]", "_"=>"1423728170887"}
  Rendered nic/_base_form.html.erb (16.8ms)
  Rendered nic/_virtual_form.html.erb (175.9ms)
  Rendered nic/manageds/_managed.html.erb (196.8ms)
  Rendered nic/new.js.erb (199.6ms)
Completed 200 OK in 215ms (Views: 200.1ms | ActiveRecord: 2.5ms)


see mac set as  "mac"=>"sachin"
Comment 2 Marek Hulan 2015-02-17 15:05:08 UTC 
The request you attached is just JS call to update the UI. It does not save anything, it just syncs the modal window with the host form. I tested both host primary interface and other interfaces with invalid MAC address. Upon form submit, they MAC address fields are cleared and there's validation error that it can't be blank. Therefore I'm closing this. Feel free to ask if there are any questions.
Comment 3 Sachin Ghai 2015-02-18 07:10:09 UTC 
I didn't see if UI raising any validation error even on form submit.

I discovered a host and click on provision and define a new interface with any random text and submit the form. Form was submitted successfully and  Provisioning started and didn't get any validation error.

Processing by DiscoveredHostsController#update as */*
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"CkViytgWQfCS/GGRqyeML9iWFyRhMicfsPVehnMc7sU=", "host"=>{"name"=>"mac525400acef58", "hostgroup_id"=>"1", "environment_id"=>"5", "content_source_id"=>"1", "puppet_ca_proxy_id"=>"1", "puppet_proxy_id"=>"1", "puppetclass_ids"=>[""], "managed"=>"true", "progress_report_id"=>"[FILTERED]", "type"=>"Host::Managed", "domain_id"=>"1", "realm_id"=>"", "mac"=>"52:54:00:ac:ef:58", "subnet_id"=>"1", "ip"=>"192.168.100.107", "interfaces_attributes"=>{"new_1424242522520"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"sachin ghai", "identifier"=>"eth0:0", "name"=>"", "domain_id"=>"", "subnet_id"=>"1", "ip"=>"192.168.100.107", "managed"=>"1", "virtual"=>"1", "tag"=>"0", "attached_to"=>"eth0:0"}, "new_interfaces"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"", "identifier"=>"", "name"=>"", "domain_id"=>"", "subnet_id"=>"", "ip"=>"", "managed"=>"1", "virtual"=>"0", "tag"=>"", "attached_to"=>""}}, "architecture_id"=>"1", "operatingsystem_id"=>"1", "build"=>"1", "medium_id"=>"8", "ptable_id"=>"7", "disk"=>"", "root_pass"=>"[FILTERED]", "is_owned_by"=>"3-Users", "enabled"=>"1", "model_id"=>"2", "comment"=>"", "overwrite"=>"false"}, "hostgroup"=>{"lifecycle_environment_id"=>"1", "content_view_id"=>"5"}, "id"=>"mac525400acef58"}
Create DHCP reservation for mac525400acef58.englab.pnq.redhat.com-52:54:00:ac:ef:58/192.168.100.107
Add DNS A record for mac525400acef58.englab.pnq.redhat.com/192.168.100.107
Add DNS PTR record for 192.168.100.107/mac525400acef58.englab.pnq.redhat.com
Add the TFTP configuration for mac525400acef58.englab.pnq.redhat.com
Fetching required TFTP boot files for mac525400acef58.englab.pnq.redhat.com
Redirected to https://dhcp201-167.englab.pnq.redhat.com/hosts/mac525400acef58.englab.pnq.redhat.com
ForemanDiscovery: Rebooting mac525400acef58
ForemanDiscovery: reboot result: successful

See in above logs mac set as sachin ghai

Later, when I edit the host, the mac field was empty. Looks it just silently removes the invalid mac? is it the expected behavior ?
Comment 4 Marek Hulan 2015-02-18 14:09:00 UTC 
In discovered host it works in a same way in my instance. Invalid mac address gets cleared and if it's required for an interface I see validation issue upon host form submit. If it's not, then there's no validation error and the form saves (with blank mac, so the save is expected I think)
Comment 5 Marek Hulan 2015-02-25 10:23:58 UTC 
Sachin is there any other concern? I think this is NOTABUG and it does not block 1193987 in any way.
Comment 6 Sachin Ghai 2015-02-26 11:53:02 UTC 
Marek, My only concern is the way we are validating the mac is misleading. 

Case1: I discovered a host and click on provision. A new host edit form opened up. Then added a physical managed interface with mac = 'invalid_mac' and form raised error "mac can't be blank". Its misleading

see in below logs mac was set as "mac"=>"invalid_mac",

Processing by DiscoveredHostsController#update as */*
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"s4LVwIN5a8iAOOmYlEDFI9ezLaGvLTdihpUIVIdlVUs=", "host"=>{"name"=>"mac5254000d6420", "hostgroup_id"=>"1", "lifecycle_environment_id"=>"1", "content_view_id"=>"2", "environment_id"=>"2", "content_source_id"=>"1", "puppet_ca_proxy_id"=>"1", "puppet_proxy_id"=>"1", "managed"=>"true", "progress_report_id"=>"[FILTERED]", "type"=>"Host::Managed", "domain_id"=>"1", "realm_id"=>"", "mac"=>"52:54:00:0d:64:20", "subnet_id"=>"1", "ip"=>"192.168.100.15", "interfaces_attributes"=>{"0"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"invalid_mac", "identifier"=>"eth1", "name"=>"", "domain_id"=>"1", "subnet_id"=>"1", "ip"=>"192.168.100.16", "managed"=>"1", "virtual"=>"0", "tag"=>"", "attached_to"=>""}, "new_interfaces"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"", "identifier"=>"", "name"=>"", "domain_id"=>"", "subnet_id"=>"", "ip"=>"", "managed"=>"1", "virtual"=>"0", "tag"=>"", "attached_to"=>""}}, "architecture_id"=>"1", "build"=>"1", "disk"=>"", "root_pass"=>"[FILTERED]", "is_owned_by"=>"3-Users", "enabled"=>"1", "model_id"=>"2", "comment"=>"", "overwrite"=>"false"}, "discovered_host"=>{"puppetclass_ids"=>[""], "operatingsystem_id"=>"1", "medium_id"=>"7", "ptable_id"=>"7"}, "id"=>"mac5254000d6420"}
DHCP records -52:54:00:0d:64:20/192.168.100.14 already exists

Failed to save: Mac can't be blank, Conflict DHCP records -52:54:00:0d:64:20/192.168.100.14 already exists
Comment 7 Sachin Ghai 2015-02-26 11:57:20 UTC 
It would be better if we validate add_interface form as soon as user update the field instead of on form submit.
Comment 9 Sachin Ghai 2015-02-26 12:04:47 UTC 
In short, my only concern is we should raise validation errors instead of silently failing them. Comment8 clearly state the wrong configuration for eth0:0(grep without mac). 

We should really raise validation error. Please let me know what do you think ?
Comment 10 Marek Hulan 2015-02-26 14:10:31 UTC 
Sachin, change to run validation before the form is submit to the server is quite big and should be systematic, not implemented just on one form. It would require JS based validations generated based on server side validations. From your last comment I understood that the issue is that managed Alias can be created without specifying MAC. This results in wrong DEVICE name in interface config file. 

I agree that this is a bug (which probably applies to VLAN as well). But it's already reported as https://bugzilla.redhat.com/show_bug.cgi?id=1193460 so I think this one should be closed.

For the record, this is not related to discovery in any way as the validation rules are same.
Comment 12 Tazim Kolhar 2015-04-28 09:59:09 UTC 
FAILEDQA:
# rpm -qa | grep foreman
ruby193-rubygem-foreman_hooks-0.3.7-2.el7sat.noarch
foreman-selinux-1.7.2.13-1.el7sat.noarch
ruby193-rubygem-foreman_docker-1.2.0.9-1.el7sat.noarch
ruby193-rubygem-foreman-tasks-0.6.12.3-1.el7sat.noarch
rubygem-hammer_cli_foreman_tasks-0.0.3.3-1.el7sat.noarch
foreman-debug-1.7.2.17-1.el7sat.noarch
foreman-libvirt-1.7.2.17-1.el7sat.noarch
ruby193-rubygem-foreman_bootdisk-4.0.2.10-1.el7sat.noarch
foreman-compute-1.7.2.17-1.el7sat.noarch
foreman-ovirt-1.7.2.17-1.el7sat.noarch
rubygem-hammer_cli_foreman_bootdisk-0.1.2.5-1.el7sat.noarch
foreman-postgresql-1.7.2.17-1.el7sat.noarch
qe-sat6-rhel71.usersys.redhat.com-foreman-client-1.0-1.noarch
qe-sat6-rhel71.usersys.redhat.com-foreman-proxy-1.0-1.noarch
ruby193-rubygem-foreman_gutterball-0.0.1.9-1.el7sat.noarch
foreman-1.7.2.17-1.el7sat.noarch
foreman-gce-1.7.2.17-1.el7sat.noarch
ruby193-rubygem-foreman_discovery-2.0.0.9-1.el7sat.noarch
rubygem-hammer_cli_foreman-0.1.4.9-1.el7sat.noarch
foreman-vmware-1.7.2.17-1.el7sat.noarch
rubygem-hammer_cli_foreman_discovery-0.0.1.7-1.el7sat.noarch
foreman-proxy-1.7.2.4-1.el7sat.noarch
qe-sat6-rhel71.usersys.redhat.com-foreman-proxy-client-1.0-1.noarch
ruby193-rubygem-foreman-redhat_access-0.1.0-1.el7sat.noarch

steps:
1. New Host --> Add_interface
add any value as mac address

screen shot attached
foreman-debug attached
Comment 13 Tazim Kolhar 2015-04-28 10:00:23 UTC 
Created attachment 1019603 [details]
Mac address
Comment 14 Tazim Kolhar 2015-04-28 10:01:08 UTC 
Created attachment 1019611 [details]
foreman-debug attached
Comment 15 Marek Hulan 2015-04-29 08:09:44 UTC 
This was tested on old version that does not include the fix. Moving back to POST, I think it should be moved to MODIFIED when there's a compose with the patch. If there is such compose, please move to MODIFIED.
Comment 17 Tazim Kolhar 2015-05-11 10:52:48 UTC 
VERIFIED:
# rpm -qa | grep foreman
ruby193-rubygem-foreman_docker-1.2.0.11-1.el7sat.noarch
rubygem-hammer_cli_foreman-0.1.4.11-1.el7sat.noarch
foreman-proxy-1.7.2.4-1.el7sat.noarch
ruby193-rubygem-foreman_bootdisk-4.0.2.13-1.el7sat.noarch
foreman-1.7.2.19-1.el7sat.noarch
ruby193-rubygem-foreman_discovery-2.0.0.13-1.el7sat.noarch
rubygem-hammer_cli_foreman_tasks-0.0.3.4-1.el7sat.noarch
foreman-libvirt-1.7.2.19-1.el7sat.noarch
foreman-postgresql-1.7.2.19-1.el7sat.noarch
ibm-hs22-05.rhts.eng.brq.redhat.com-foreman-proxy-client-1.0-1.noarch
foreman-debug-1.7.2.19-1.el7sat.noarch
foreman-compute-1.7.2.19-1.el7sat.noarch
foreman-vmware-1.7.2.19-1.el7sat.noarch
rubygem-hammer_cli_foreman_bootdisk-0.1.2.7-1.el7sat.noarch
foreman-ovirt-1.7.2.19-1.el7sat.noarch
ruby193-rubygem-foreman-redhat_access-0.1.0-1.el7sat.noarch
ruby193-rubygem-foreman-tasks-0.6.12.5-1.el7sat.noarch
rubygem-hammer_cli_foreman_discovery-0.0.1.9-1.el7sat.noarch
ruby193-rubygem-foreman_hooks-0.3.7-2.el7sat.noarch
ruby193-rubygem-foreman_gutterball-0.0.1.9-1.el7sat.noarch
foreman-selinux-1.7.2.13-1.el7sat.noarch
ibm-hs22-05.rhts.eng.brq.redhat.com-foreman-client-1.0-1.noarch
ibm-hs22-05.rhts.eng.brq.redhat.com-foreman-proxy-1.0-2.noarch
foreman-gce-1.7.2.19-1.el7sat.noarch

steps:
1. New Host --> Add_interface
 Add invalid mac
Error displayed

screenshot attached
Comment 18 Tazim Kolhar 2015-05-11 10:53:34 UTC 
Created attachment 1024193 [details]
invalid mac address
Comment 19 Bryan Kearney 2015-08-11 13:32:33 UTC 
This bug is slated to be released with Satellite 6.1.
Comment 20 Bryan Kearney 2015-08-12 13:58:07 UTC 
This bug was fixed in version 6.1.1 of Satellite which was released on 12 August, 2015.
Note You need to log in before you can comment on or make changes to this bug.