Setting passwords for openstack services in HEX string format breaks deployment: 'err: 12380245 is not a string. It looks to be a Fixnum'

Bug #1494779 reported by Artem Panchenko
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fuel for OpenStack
Fix Released
High
Maksim Malchuk
7.0.x
Fix Released
High
Maksim Malchuk

Bug Description

Fuel version info (7.0 build #289): http://paste.openstack.org/show/456829/

If password for admin tenant (set manually by cloud administrator) or some OpenStack service (generated automatically by Nailgun) looks like /0X[A-F0-9]+/ (HEX string) then environment deployment fails:

node-1 2015-09-10T05:10:06.037425 err: 12380245 is not a string. It looks to be a Fixnum at /etc/puppet/modules/neutron/manifests/db/mysql.pp:48 on node node-1.test.domain.local

Steps to reproduce:

1. Create environment
2. Add nodes
3. Set tenant password to '0Xbb44' (without quotes)
4. Deploy environment

Expected result: environment successfully deployed

Actual: deployment fails because password for tenant is not a string

This issue is caused by incompatibility of 'Psych' (default in both) YAML engines in Ruby version 2.1.1 (master node, Astute container) and 1.9.3 (slaves), see details here:

https://news.ycombinator.com/item?id=7788532

When Astute dumps string it uses Ruby 2.1.1 which save password as implicit (bare) string:

root@node-1:~# grep quantum_settings /etc/astute.yaml -A 2
quantum_settings:
  database:
    passwd: 0XbCe855

But Hiera on slaves uses Ruby 1.9.3 which tries to convert unquoted strings to YAML supported formats, so it parses the password as hex/int:

root@node-1:~# hiera quantum_settings | grep database
{"database"=>{"passwd"=>12380245},

I tried to use Ruby 2.1.1 to load the same YAML and it correctly parsed password as string:

irb(main):002:0> c = File.open('test.yaml').read()
=> "---\ndatabase:\n passwd: 0XbCe855\n"
irb(main):003:0> hash = YAML::load(c)
=> {"database"=>{"passwd"=>"0XbCe855"}}
irb(main):004:0> hash['database']['passwd'].class
=> String

So looks like we need to upgrade Ruby on slaves to >2.1 version in order to fix this bug.

Revision history for this message
Artem Panchenko (apanchenko-8) wrote :
Revision history for this message
Artem Panchenko (apanchenko-8) wrote :

If we want to fix this bug in 7.0 we need to use some workaround, because it's impossible to upgrade Ruby version after HCF. I tried to play with different options in Ruby 2.1.1 (both Psych and Syck modules) to configure Astute to always quote strings explicitly, but with no luck. However I've found another workaround - configure Hiera to use Syck module, which seems parses implicit strings as expected, here is a patch https://review.fuel-infra.org/#/c/11489/

Revision history for this message
Andrey Maximov (maximov) wrote :

guys, do you have some example of passwords which result to hashes started with 0x ?

Revision history for this message
Andrey Maximov (maximov) wrote :

sorry, I misunderstood the problem, so this is about passwords in plaintext.

Revision history for this message
Vladimir Kuklin (vkuklin) wrote :

I think that for 7.0 we can fix this issue by adding note into release-notes and limiting passowrd generator in nailgun in order to make it not to generate passwords starting with '[0][xX]'

Revision history for this message
Artem Panchenko (apanchenko-8) wrote :

Guys,

AFAIU you decided to fix this issue in 7.0 by patching Nailgun and adding warnings to docs. But JFYI I tested the patch for Hiera (mentioned above) and it fixes the issue. Also custom BVT passed with new package :)

Revision history for this message
Vladimir Sharshov (vsharshov) wrote :

Because of hard code freeze i set won't fix for 7.0

Revision history for this message
Dmitry Ilyin (idv1985) wrote :

There is almost nothing that can be done about this, it's how the old ruby yaml library behaves.
We should make Nailgun's password generator to never generate a password that starts with /^0x/i

Revision history for this message
Dmitry Ilyin (idv1985) wrote :

Or even just not start with 0 (number)

tags: added: release-notes
Dmitry Pyzhov (dpyzhov)
no longer affects: fuel/8.0.x
Changed in fuel:
assignee: Fedor Zhadaev (fzhadaev) → Fuel Python Team (fuel-python)
Dmitry Pyzhov (dpyzhov)
Changed in fuel:
assignee: Fuel Python Team (fuel-python) → Fedor Zhadaev (fzhadaev)
Revision history for this message
Fedor Zhadaev (fzhadaev) wrote :

Seems like patch for Hiera from comment #2 is better solution then workarounds in Nailgun.

Changed in fuel:
assignee: Fedor Zhadaev (fzhadaev) → Fuel Library Team (fuel-library)
Revision history for this message
Mike Scherbakov (mihgen) wrote :

Why don't we just create passwords in quotes, so it's always interpreted as a String.. ?

Revision history for this message
Matthew Mosesohn (raytrac3r) wrote :

Fedor, this affects 7.0 and earlier. Is there are strong reason why we can't approach this from nailgun in one of the two possible solutions?
Solution 1: Avoid starting passwords with '0'
Solution 2: Encapsulate passwords with double quotes

Solution in comment #2 is slow and we are likely to see many obstacles before it could be implemented.

Changed in fuel:
assignee: Fuel Library Team (fuel-library) → Fedor Zhadaev (fzhadaev)
Revision history for this message
Fedor Zhadaev (fzhadaev) wrote :

Matthew, I've investigated this problem and have the discussion with Vladimir Sharshov about proposed fixes. Since some of passwords may be set by user, it should be done a few dirty hacks in Nailgun to fix this bug (in generators and validators). Moreover, if we made such hacks in validators we'll have to show confusing messages to user like "You can't use this password, because it starts with '0'". That's why hiera solution looks less terrible.

Guys, please give your opinion about this point. If you think I'm wrong I'll make the fix for nailgun.

Dmitry Pyzhov (dpyzhov)
tags: added: area-python
Revision history for this message
Matthew Mosesohn (raytrac3r) wrote :

AFter some discussion this needs to be addressed on the Ruby side one way or another. Nailgun sends Astute (a ruby application) a JSON to generate the astute.yaml file. We should quote values when writing out the YAML file or fix the parser when reading it in via hiera.

Dmitry Pyzhov (dpyzhov)
Changed in fuel:
assignee: Fedor Zhadaev (fzhadaev) → Fuel Library Team (fuel-library)
Changed in fuel:
assignee: Fuel Library Team (fuel-library) → Maksim Malchuk (mmalchuk)
Changed in fuel:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to fuel-astute (stable/7.0)

Fix proposed to branch: stable/7.0
Review: https://review.openstack.org/239888

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to fuel-astute (master)

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

Dmitry Pyzhov (dpyzhov)
tags: added: tricky
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to fuel-astute (master)

Reviewed: https://review.openstack.org/239894
Committed: https://git.openstack.org/cgit/openstack/fuel-astute/commit/?id=ed8248db45a20d2e582e0660dc36234dc977e3a2
Submitter: Jenkins
Branch: master

commit ed8248db45a20d2e582e0660dc36234dc977e3a2
Author: Maksim Malchuk <email address hidden>
Date: Wed Oct 28 12:37:59 2015 +0300

    Quote "bad" passwords in 'astute.yaml' right before upload it to nodes

    Astute uses Ruby 2.1 with YAML 'Psych' engine which is not compatible
    with Ruby 1.9 'Psych' engine used by Hierra in Puppet to parse unquoted
    strings of passwords started with prefixed numeral values.

    This commit uses alternate version of 'YAML::dump' method with forced
    quotation of strings according its value and attributes.

    Also this commit should fix possible problem with unquoted strings
    containing bad mac-addresses which started with two zeros.

    This commit should be reverted after upgrading to Ruby 2.1 everywhere.

    Change-Id: I7435da01016c4b2266210eb9422ea1fae87de7f8
    Closes-Bug: #1494779

Changed in fuel:
status: In Progress → Fix Committed
Vladimir (vushakov)
tags: added: on-verification
Revision history for this message
Fuel Devops McRobotson (fuel-devops-robot) wrote : Change abandoned on packages/trusty/ruby-hiera (7.0)

Change abandoned by Artem Panchenko <email address hidden> on branch: 7.0
Review: https://review.fuel-infra.org/11489
Reason: fixed on astute side https://review.openstack.org/#/c/239894/

Revision history for this message
Vladimir (vushakov) wrote :

Verified on Fuel 8.0 build.
Environment created by system_tests.sh. Admin password was changed to '0Xbb44'. Environment successfully deployed.

  2015.1.0-8.0:
    VERSION:
      api: '1.0'
      astute_sha: 959b06c5ef8143125efd1727d350c050a922eb12
      build_id: '147'
      build_number: '147'
      feature_groups:
      - mirantis
      fuel-agent_sha: 07560a9fc3ce5301ace04d2d3e5d68db6ee4f8d5
      fuel-createmirror_sha: d3949f3094248b9686a4c20caea5ae13c9738a8d
      fuel-library_sha: a94bddc9ae6198510402a42ad78699cb9d70fda8
      fuel-nailgun-agent_sha: 00b4b11553c250f22c0079fb74c8b782dcb7b740
      fuel-nailgun_sha: fe7fe7d082b4045d34e9b2d9aac6766dbd6cd576
      fuel-ostf_sha: f771e23c5cbecbbb578ee4574ae2e51aa2e91819
      fuel-upgrade_sha: 1e894e26d4e1423a9b0d66abd6a79505f4175ff6
      fuelmain_sha: 3b64b95f6b513b0d46b57525a858eea7a9809da3
      fuelmenu_sha: f40c3ec0e0e1bdc499f4cb50d46aecefb4dbb7c7
      network-checker_sha: a57e1d69acb5e765eb22cab0251c589cd76f51da
      openstack_version: 2015.1.0-8.0
      production: docker
      python-fuelclient_sha: a88bc94b562ee511eb277183d0a750a1e3312662
      release: '8.0'
      shotgun_sha: 25dd78a3118267e3616df0727ce746e7dead2d67

tags: removed: on-verification
Changed in fuel:
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to fuel-astute (stable/7.0)

Reviewed: https://review.openstack.org/239888
Committed: https://git.openstack.org/cgit/openstack/fuel-astute/commit/?id=44fa6088bba6f1d9ba3cae264bcd727c40719976
Submitter: Jenkins
Branch: stable/7.0

commit 44fa6088bba6f1d9ba3cae264bcd727c40719976
Author: Maksim Malchuk <email address hidden>
Date: Wed Oct 28 12:37:59 2015 +0300

    Quote "bad" passwords in 'astute.yaml' right before upload it to nodes

    Astute uses Ruby 2.1 with YAML 'Psych' engine which is not compatible
    with Ruby 1.9 'Psych' engine used by Hierra in Puppet to parse unquoted
    strings of passwords started with prefixed numeral values.

    This commit uses alternate version of 'YAML::dump' method with forced
    quotation of strings according its value and attributes.

    Also this commit should fix possible problem with unquoted strings
    containing bad mac-addresses which started with two zeros.

    This commit should be reverted after upgrading to Ruby 2.1 everywhere.

    Change-Id: I7435da01016c4b2266210eb9422ea1fae87de7f8
    Closes-Bug: #1494779

tags: added: 8.0 release-notes-notes
removed: release-notes
tags: added: release-notes-done
removed: release-notes-notes
tags: added: on-verification
Revision history for this message
Alexander Gromov (agromov) wrote :

Verified on 7.0-mu-3.

Environment was prepared by system_tests.sh. Admin password was changed to '0Xbb44'.
Environment was successfully deployed.

tags: removed: on-verification
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.