Merge ~jasonzio/cloud-init:randomSeed into cloud-init:master

[Jason Zions]

Azure: Ensure platform random_seed is always serializable as JSON.

The Azure platform surfaces random bytes into /sys via Hyper-V.
Python 2.7 json.dump() raises an exception if asked to convert
a str with non-character content, and python 3.0 json.dump()
won't serialize a "bytes" value. As a result, c-i instance
data is often not written by Azure, making reboots slower (c-i
has to repeat work).

The random data is base64-encoded and then decoded into a string
(str or unicode depending on the version of Python in use). The
base64 string has just as many bits of entropy, so we're not
throwing away useful "information", but we can be certain
json.dump() will correctly serialize the bits.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:87a6981b6caf9dbe049c6924a13d83597bf73c9a
https://jenkins.ubuntu.com/server/job/cloud-init-ci/650/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/650/rebuild

review: Needs Fixing (continuous-integration)

[Dan Watkins (oddbloke)]

Hi Jason, thanks for the MP! I've left some inline comments (including pointing out what's causing the CI failure).

review: Needs Fixing

[Jason Zions (jasonzio)]

Responses in-line. I'll update the MP.

7fbdb3c... by "Jason Zions (MSFT)" <email address hidden> on 2019-03-25

Always b64-encode. Move poison seed to data file.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:7fbdb3cc10a0a37e856eb85229dcad8130456408
https://jenkins.ubuntu.com/server/job/cloud-init-ci/651/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/651/rebuild

review: Needs Fixing (continuous-integration)

80e0c63... by "Jason Zions (MSFT)" <email address hidden> on 2019-03-25

Tell load_json() to expect to produce something other than a dict

[Dan Watkins (oddbloke)]

Thanks for the swift response Jason!

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:80e0c63e21b7ac3a64bb4f2977993368032b50ec
https://jenkins.ubuntu.com/server/job/cloud-init-ci/652/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/652/rebuild

review: Needs Fixing (continuous-integration)

02e2aef... by "Jason Zions (MSFT)" <email address hidden> on 2019-03-25

load_json is happiest with top-level dict

[Jason Zions (jasonzio)]

This is why I wrapped the test string in a dict. util.load_json() assumes the result is a dict unless told otherwise, and the type that comes back is either unicode in python2 and str in python3 but there's no immediately obvious way to tell load_json() that it's one of those two. If I wrap the string in a dict, util.load_json() is happy, and the code is clear.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:02e2aefa74abd14bde7aeefc5b7d17229a7842e1
https://jenkins.ubuntu.com/server/job/cloud-init-ci/653/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/653/rebuild

review: Needs Fixing (continuous-integration)

1d44d97... by "Jason Zions (MSFT)" <email address hidden> on 2019-03-25

Handle bytes() object correctly for python3

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:1d44d97034d0fcc65ba481b1fafd5e781e99e18e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/654/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/654/rebuild

review: Needs Fixing (continuous-integration)

335ec2c... by "Jason Zions (MSFT)" <email address hidden> on 2019-03-26

Remove unneeded import

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:335ec2cdc09995ecbb90e5d1aa5622ea6c6a958e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/655/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/655/rebuild

review: Approve (continuous-integration)

[Dan Watkins (oddbloke)]

This looks great, thanks for iterating on it, Jason!

(I'm removing the Description because it tickles a bug in our auto-lander; I've just appended it to the commit message.)

review: Approve

[Scott Moser (smoser)]

There was discussion on this issue/fix in #cloud-init yesterday.
It is really just a specific case of the general python 2 only issue described in bug 1801364

[Jason Zions (jasonzio)]

You're correct, Scott, this is a specific fix in Azure for the python2 issue. The true heart of the issue is "JSON cannot store a random sequence of octets that do not form a legal UTF-8 string", but that is precisely the desired contract for random_seed.

I suggest cloud-init change its internal contract for random_seed to be "datasource provides a base64-encoded string of random octets". cc_random_seed.py should rely upon that and do a base64.b64decode on that value before writing it to /dev/urandom. Instance data persist/restore would work unchanged, since they'd now be dealing with a JSON-safe string. Only the two datasources that provide random_seed would have to change, and that change would look almost exactly like the one I made to the Azure source.

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index eccbee5..b4e3f06 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -54,6 +54,7 @@ REPROVISION_MARKER_FILE = "/var/lib/cloud/data/poll_imds"
 REPORTED_READY_MARKER_FILE = "/var/lib/cloud/data/reported_ready"
 AGENT_SEED_DIR = '/var/lib/waagent'
 IMDS_URL = "http://169.254.169.254/metadata/"
+PLATFORM_ENTROPY_SOURCE = "/sys/firmware/acpi/tables/OEM0"
 
 # List of static scripts and network config artifacts created by
 # stock ubuntu suported images.
@@ -195,6 +196,8 @@ if util.is_FreeBSD():
         RESOURCE_DISK_PATH = "/dev/" + res_disk
     else:
         LOG.debug("resource disk is None")
+    # TODO Find where platform entropy data is surfaced
+    PLATFORM_ENTROPY_SOURCE = None
 
 BUILTIN_DS_CONFIG = {
     'agent_command': AGENT_START_BUILTIN,
@@ -1100,16 +1103,27 @@ def _check_freebsd_cdrom(cdrom_dev):
     return False
 
 
-def _get_random_seed():
+def _get_random_seed(source=PLATFORM_ENTROPY_SOURCE):
     """Return content random seed file if available, otherwise,
        return None."""
     # azure / hyper-v provides random data here
-    # TODO. find the seed on FreeBSD platform
     # now update ds_cfg to reflect contents pass in config
-    if util.is_FreeBSD():
+    if source is None:
         return None
-    return util.load_file("/sys/firmware/acpi/tables/OEM0",
-                          quiet=True, decode=False)
+    seed = util.load_file(source, quiet=True, decode=False)
+
+    # The seed generally contains non-Unicode characters. load_file puts
+    # them into a str (in python 2) or bytes (in python 3). In python 2,
+    # bad octets in a str cause util.json_dumps() to throw an exception. In
+    # python 3, bytes is a non-serializable type, and the handler load_file
+    # uses applies b64 encoding *again* to handle it. The simplest solution
+    # is to just b64encode the data and then decode it to a serializable
+    # string. Same number of bits of entropy, just with 25% more zeroes.
+    # There's no need to undo this base64-encoding when the random seed is
+    # actually used in cc_seed_random.py.
+    seed = base64.b64encode(seed).decode()
+
+    return seed
 
 
 def list_possible_azure_ds_devs():
diff --git a/tests/data/azure/non_unicode_random_string b/tests/data/azure/non_unicode_random_string
new file mode 100644
index 0000000..b9ecefb
--- /dev/null
+++ b/tests/data/azure/non_unicode_random_string
@@ -0,0 +1 @@
+OEM0d\x00\x00\x00\x01\x80VRTUALMICROSFT\x02\x17\x00\x06MSFT\x97\x00\x00\x00C\xb4{V\xf4X%\x061x\x90\x1c\xfen\x86\xbf~\xf5\x8c\x94&\x88\xed\x84\xf9B\xbd\xd3\xf1\xdb\xee:\xd9\x0fc\x0e\x83(\xbd\xe3'\xfc\x85,\xdf\xf4\x13\x99N\xc5\xf3Y\x1e\xe3\x0b\xa4H\x08J\xb9\xdcdb$
\ No newline at end of file
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 6b05b8f..53c56cd 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -7,11 +7,11 @@ from cloudinit.sources import (
     UNSET, DataSourceAzure as dsaz, InvalidMetaDataException)
 from cloudinit.util import (b64e, decode_binary, load_file, write_file,
                             find_freebsd_part, get_path_dev_freebsd,
-                            MountFailedError)
+                            MountFailedError, json_dumps, load_json)
 from cloudinit.version import version_string as vs
 from cloudinit.tests.helpers import (
     HttprettyTestCase, CiTestCase, populate_dir, mock, wrap_and_call,
-    ExitStack)
+    ExitStack, resourceLocation)
 
 import crypt
 import httpretty
@@ -1923,4 +1923,24 @@ class TestWBIsPlatformViable(CiTestCase):
             self.logs.getvalue())
 
 
+class TestRandomSeed(CiTestCase):
+    """Test proper handling of random_seed"""
+
+    def test_non_ascii_seed_is_serializable(self):
+        """Pass if a random string from the Azure infrastructure which
+        contains at least one non-Unicode character can be converted to/from
+        JSON without alteration and without throwing an exception.
+        """
+        path = resourceLocation("azure/non_unicode_random_string")
+        result = dsaz._get_random_seed(path)
+
+        obj = {'seed': result}
+        try:
+            serialized = json_dumps(obj)
+            deserialized = load_json(serialized)
+        except UnicodeDecodeError:
+            self.fail("Non-serializable random seed returned")
+
+        self.assertEqual(deserialized['seed'], result)
+
 # vi: ts=4 expandtab