[Crowbar] issues with short domain names in test/demo scenarios

Judd Maltin judd at newgoliath.com
Mon Jul 23 05:53:05 CDT 2012


Thanks for the patch, Adam.

We should definitely patch mainline Crowbar.  I'll open an internal bug
today.

Ideally, we should be using a well-known library to validate domain names,
like "require 'uri'" or 'addressable'.  I'll open a bug for that as well.

Thanks!
-judd

On Mon, Jul 23, 2012 at 6:06 AM, Adam Spiers <aspiers at suse.com> wrote:

> Hi all,
>
> Last week we encountered a nasty corner case where someone had
> configured their admin node with the FQDN "cloud-admin.fs" or similar.
> This causes schema validation of bc-template-dns.json to fail:
>
>   Jul 17 14:40:36 cloud-admin crowbar_app[4571]: Crowbar apply_role:
> creating dns.default
>   Jul 17 14:40:36 cloud-admin crowbar_app[4571]: Crowbar apply_role:
> Calling get to see if it already exists: dns.default
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: Crowbar apply_role:
> didn't already exist, creating proposal for dns.default
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: Could not recover Chef
> Crowbar Data on load bc-dns-default: #<Net::HTTPServerException: 404 "Not
> Found">
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: DNS create_proposal:
> entering
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: DNS create_proposal:
> exiting
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: validating proposal dns
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: validation errors in
> proposal dns
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: Failed to create
> dns.default: 400 : Should be a Domain Name: fs
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: Crowbar apply_role: check
> to see if it is already active: dns.default
>   Jul 17 14:40:37 cloud-admin crowbar_app[4571]: Crowbar apply_role:
> dns.default wasn't active: Activating
>   Jul 17 14:40:38 cloud-admin crowbar_app[4571]: Could not recover Chef
> Crowbar Data on load bc-dns-default: #<Net::HTTPServerException: 404 "Not
> Found">
>   Jul 17 14:40:38 cloud-admin crowbar_app[4571]: Failed to commit
> dns.default: 404 : Failed to find proposal: dns.default
>   Jul 17 14:40:38 cloud-admin crowbar_app[4571]: Crowbar apply_role: Done
> with creating: dns.default
>
> The biggest issue was that the code continued applying all the other
> proposals for the other barclamps, with no visible warning that
> anything had gone wrong.  It only became obvious when crowbar_join
> got stuck in a loop due to reverse DNS failing (since bind did not
> start on the admin node).
>
> So we thought it would be better to change the logic so that
> validation is performed on *all* barclamps before any proposals are
> committed.  Ralf has done this in our branch:
>
>
> https://github.com/SUSE-Cloud/barclamp-crowbar/commit/b50eee061c6ddba98792f97d546b7b5fd5a0f8b4
>
> Now let's get into the details of the validation code in question,
> which lives in
>
>   barclamps/crowbar/crowbar_framework/app/models/crowbar_validator.rb
>
> and looks like this:
>
>   when 'DomainName'
>     regex =
> /\A((((([a-z0-9]{1}[a-z0-9\-]{0,62}[a-z0-9]{1})|[a-z])\.)+[a-z]{2,6})|(\d{1,3}\.){3}\d{1,3}(\:\d{1,5})?)\z/
>
> There are a few issues with the regex:
>
>   1. It considers something like "192.168.0.1:3000" to be a valid
>      domain name!  Maybe an erroneous copy'n'paste extraction from
>      within an email or URL regular expression?  I see exactly the
>      same regex contained in the following page:
>
>        http://fightingforalostcause.net/misc/2006/compare-email-regex.php
>
>   2. It requires the final subdomain to match [a-z]{2,6} -
>      Presumably some assumption about the presence of a country code,
>      but that is not applicable in standalone test/demo networks, nor
>      is it even correct when matched against the official IANA list:
>
>        http://www.iana.org/domains/root/db/
>
>   3. It requires the domain name to have at least two components.
>      Again, that is not applicable in standalone test/demo networks.
>
>   4. It allows 64-character domain names, but 63 is the maximum
>      according to http://tools.ietf.org/html/rfc1034#section-3.5
>
>   5. It doesn't allow uppercase.
>
>   6. It allows illegal domains such as "12.34.com"
>
> Here's my suggestion for an improved version:
>
>   label = /[a-z]([a-z0-9\-]{0,62}[a-z0-9])?/i
>   regex = /\A#{label}(\.#{label})*\z/
>
> This is in our branch via:
>
>
> https://github.com/SUSE-Cloud/barclamp-crowbar/commit/0d1085ce69205841be202ce911c394e317814d63
>
> https://github.com/SUSE-Cloud/barclamp-crowbar/commit/f0639786f4a879a7cf5f04a402daa8ca54a943a7
>
> We noticed two other similar issues which we haven't addressed yet:
>
>   1. the 'FQDN' validation code in crowbar_validator.rb is apparently
>      never used - any thoughts on where this is supposed to be used?
>
>   2. NodeObject#alias= hardcodes a regular expression rather than
>      calling out to crowbar_validator.rb.
>
> Thanks,
> Adam (and Ralf)
>
> _______________________________________________
> Crowbar mailing list
> Crowbar at dell.com
> https://lists.us.dell.com/mailman/listinfo/crowbar
> For more information: https://github.com/dellcloudedge/crowbar/wiki
>



-- 
Judd Maltin
T: 917-882-1270
F: 501-694-7809
A loving heart is never wrong.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.us.dell.com/pipermail/crowbar/attachments/20120723/3771a356/attachment.html 


More information about the Crowbar mailing list