Merge lp:~mpontillo/maas/beaconing-packet-format into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Add methods to encode and decode beacon packets.

Also, change the name of the Fernet methods to make it clear that the encryption is done using a pre-shared key (the MAAS shared secret).

Minor test suite refactoring for SharedSecretTestCase to make it more general.

[Данило Шеган (danilo)]

I do have a few questions inline. Looks generally good, though.

review: Needs Information

[Mike Pontillo (mpontillo)]

Thanks for the review. Some replies below. You've given me some things to think about, so I'll make this WIP for now.

[Данило Шеган (danilo)]

Looks great, thanks for the changes!

A few more things that you probably forgot to complete with it being past 1am :-)

Not blocking since the gist of the stuff is there (basically, you are not making use of the new exception and raw=True parameter).

review: Approve

[Mike Pontillo (mpontillo)]

Thanks much for the review; some replies below.

=== modified file 'src/provisioningserver/security.py'
--- src/provisioningserver/security.py	2017-06-02 17:28:04 +0000
+++ src/provisioningserver/security.py	2017-06-21 15:18:45 +0000
@@ -9,7 +9,10 @@
     "get_shared_secret_from_filesystem",
 ]
 
-from base64 import urlsafe_b64encode
+from base64 import (
+    urlsafe_b64decode,
+    urlsafe_b64encode,
+)
 import binascii
 from binascii import (
     a2b_hex,
@@ -41,6 +44,10 @@
 )
 
 
+class MissingSharedSecret(RuntimeError):
+    """Raised when the MAAS shared secret is missing."""
+
+
 def to_hex(b):
     """Convert byte string to hex encoding."""
     assert isinstance(b, bytes), "%r is not a byte string" % (b,)
@@ -140,7 +147,8 @@
         global _fernet_psk
         if _fernet_psk is None:
             secret = get_shared_secret_from_filesystem()
-            assert secret is not None, "MAAS shared secret not found."
+            if secret is None:
+                raise MissingSharedSecret("MAAS shared secret not found.")
             # Keying material is required by PBKDF2 to be a byte string.
             kdf = PBKDF2HMAC(
                 algorithm=hashes.SHA256(),
@@ -171,7 +179,7 @@
     return f
 
 
-def fernet_encrypt(message):
+def fernet_encrypt_psk(message, raw=False):
     """Encrypts the specified message using the Fernet format.
 
     Returns the encrypted token, as a byte string.
@@ -183,29 +191,43 @@
 
     :param message: The message to encrypt.
     :type message: Must be of type 'bytes' or a UTF-8 'str'.
+    :param raw: if True, returns the decoded base64 bytes representing the
+        Fernet token. The bytes must be converted back to base64 to be
+        decrypted. (Or the 'raw' argument on the corresponding
+        fernet_decrypt_psk() function can be used.)
     :return: the encryption token, as a base64-encoded byte string.
     """
-    f = _get_fernet_context()
+    fernet = _get_fernet_context()
     if isinstance(message, str):
         message = message.encode("utf-8")
-    return f.encrypt(message)
-
-
-def fernet_decrypt(token, ttl=None):
+    token = fernet.encrypt(message)
+    if raw is True:
+        token = urlsafe_b64decode(token)
+    return token
+
+
+def fernet_decrypt_psk(token, ttl=None, raw=False):
     """Decrypts the specified Fernet token using the MAAS secret.
 
     Returns the decrypted token as a byte string; the user is responsible for
     converting it to the correct format or encoding.
 
     :param message: The token to decrypt.
-    :type token: bytes
+    :type token: Must be of type 'bytes', or an ASCII base64 string.
     :param ttl: Optional amount of time (in seconds) allowed to have elapsed
         before the message is rejected upon decryption. Note that the Fernet
         library considers times up to 60 seconds into the future (beyond the
         TTL) to be valid.
+    :param raw: if True, treats the string as the decoded base64 bytes of a
+        Fernet token, and attempts to encode them (as expected by the Fernet
+        APIs) before decrypting.
     :return: bytes
     """
+    if raw is True:
+        token = urlsafe_b64encode(token)
     f = _get_fernet_context()
+    if isinstance(token, str):
+        token = token.encode("ascii")
     return f.decrypt(token, ttl=ttl)
 
 

=== modified file 'src/provisioningserver/tests/test_security.py'
--- src/provisioningserver/tests/test_security.py	2017-06-02 17:28:04 +0000
+++ src/provisioningserver/tests/test_security.py	2017-06-21 15:18:45 +0000
@@ -28,8 +28,9 @@
 from provisioningserver import security
 from provisioningserver.path import get_data_path
 from provisioningserver.security import (
-    fernet_decrypt,
-    fernet_encrypt,
+    fernet_decrypt_psk,
+    fernet_encrypt_psk,
+    MissingSharedSecret,
 )
 from provisioningserver.utils.fs import (
     FileLock,
@@ -37,7 +38,10 @@
     write_text_file,
 )
 from testtools import ExpectedException
-from testtools.matchers import Equals
+from testtools.matchers import (
+    Equals,
+    IsInstance,
+)
 
 
 class SharedSecretTestCase(MAASTestCase):
@@ -49,17 +53,24 @@
         # so that tests cannot interfere with each other.
         get_secret.return_value = get_data_path(
             "var", "lib", "maas", "secret-%s" % factory.make_string(16))
-        secret_file = security.get_shared_secret_filesystem_path()
         # Extremely unlikely, but just in case.
-        if os.path.isfile(secret_file):
-            os.remove(secret_file)
+        self.delete_secret()
+        self.addCleanup(
+            setattr, security, "DEFAULT_ITERATION_COUNT",
+            security.DEFAULT_ITERATION_COUNT)
+        # The default high iteration count would make the tests very slow.
+        security.DEFAULT_ITERATION_COUNT = 2
         super().setUp()
 
     def tearDown(self):
+        self.delete_secret()
+        super().tearDown()
+
+    def delete_secret(self):
+        security._fernet_psk = None
         secret_file = security.get_shared_secret_filesystem_path()
         if os.path.isfile(secret_file):
             os.remove(secret_file)
-        super().tearDown()
 
     def write_secret(self):
         secret = factory.make_bytes()
@@ -121,6 +132,12 @@
 
 class TestSetSharedSecretOnFilesystem(MAASTestCase):
 
+    def test__default_iteration_count_is_reasonably_large(self):
+        # Ensure that the iteration count is high by default. This is very
+        # important so that the MAAS secret cannot be determined by
+        # brute-force.
+        self.assertThat(security.DEFAULT_ITERATION_COUNT, Equals(100000))
+
     def read_secret(self):
         secret_path = security.get_shared_secret_filesystem_path()
         secret_hex = read_text_file(secret_path)
@@ -299,40 +316,23 @@
 
 class TestFernetEncryption(SharedSecretTestCase):
 
-    def setUp(self):
-        security._fernet_psk = None
-        # Ensure that the iteration count is high by default. This is very
-        # important so that the MAAS secret cannot be determined by
-        # brute-force. As a side effect, this ensures our tearDown (which
-        # resets the iteration count to its default) works properly.
-        self.assertThat(security.DEFAULT_ITERATION_COUNT, Equals(100000))
-        self._previous_iteration_count = security.DEFAULT_ITERATION_COUNT
-        # The default high iteration count would make the tests very slow.
-        security.DEFAULT_ITERATION_COUNT = 2
-        super().setUp()
-
-    def tearDown(self):
-        security._fernet_psk = None
-        security.DEFAULT_ITERATION_COUNT = self._previous_iteration_count
-        super().tearDown()
-
     def test__first_encrypt_caches_psk(self):
         self.write_secret()
         self.assertIsNone(security._fernet_psk)
         testdata = factory.make_string()
-        fernet_encrypt(testdata)
+        fernet_encrypt_psk(testdata)
         self.assertIsNotNone(security._fernet_psk)
 
     def test__derives_identical_key_on_decrypt(self):
         self.write_secret()
         self.assertIsNone(security._fernet_psk)
         testdata = factory.make_bytes()
-        token = fernet_encrypt(testdata)
+        token = fernet_encrypt_psk(testdata)
         first_key = security._fernet_psk
         # Make it seem like we're decrypting something without ever encrypting
         # anything first.
         security._fernet_psk = None
-        decrypted = fernet_decrypt(token)
+        decrypted = fernet_decrypt_psk(token)
         second_key = security._fernet_psk
         self.assertEqual(first_key, second_key)
         self.assertEqual(testdata, decrypted)
@@ -340,29 +340,40 @@
     def test__can_encrypt_and_decrypt_string(self):
         self.write_secret()
         testdata = factory.make_string()
-        token = fernet_encrypt(testdata)
-        decrypted = fernet_decrypt(token)
-        decrypted = decrypted.decode("utf-8")
+        token = fernet_encrypt_psk(testdata)
+        # Round-trip this to a string, since Fernet tokens are used inside
+        # strings (such as JSON objects) typically.
+        token = token.decode("ascii")
+        decrypted = fernet_decrypt_psk(token)
+        decrypted = decrypted.decode("ascii")
+        self.assertThat(decrypted, Equals(testdata))
+
+    def test__can_encrypt_and_decrypt_with_raw_bytes(self):
+        self.write_secret()
+        testdata = factory.make_bytes()
+        token = fernet_encrypt_psk(testdata, raw=True)
+        self.assertThat(token, IsInstance(bytes))
+        decrypted = fernet_decrypt_psk(token, raw=True)
         self.assertThat(decrypted, Equals(testdata))
 
     def test__can_encrypt_and_decrypt_bytes(self):
         self.write_secret()
         testdata = factory.make_bytes()
-        token = fernet_encrypt(testdata)
-        decrypted = fernet_decrypt(token)
+        token = fernet_encrypt_psk(testdata)
+        decrypted = fernet_decrypt_psk(token)
         self.assertThat(decrypted, Equals(testdata))
 
     def test__raises_when_no_secret_exists(self):
         testdata = factory.make_bytes()
-        with ExpectedException(AssertionError):
-            fernet_encrypt(testdata)
-        with ExpectedException(AssertionError):
-            fernet_decrypt(b"")
+        with ExpectedException(MissingSharedSecret):
+            fernet_encrypt_psk(testdata)
+        with ExpectedException(MissingSharedSecret):
+            fernet_decrypt_psk(b"")
 
     def test__assures_data_integrity(self):
         self.write_secret()
         testdata = factory.make_bytes(size=10)
-        token = fernet_encrypt(testdata)
+        token = fernet_encrypt_psk(testdata)
         bad_token = bytearray(token)
         # Flip a bit in the token, so we can ensure it won't decrypt if it
         # has been corrupted. Subtract 4 to avoid the end of the token; that
@@ -374,30 +385,30 @@
         test_description = ("token=%s; token[%d] ^= 0x%02x" % (
             token.decode("utf-8"), byte_to_flip, bit_to_flip))
         with ExpectedException(InvalidToken, msg=test_description):
-            fernet_decrypt(bad_token)
+            fernet_decrypt_psk(bad_token)
 
     def test__messages_from_up_to_a_minute_in_the_future_accepted(self):
         self.write_secret()
         testdata = factory.make_bytes()
         now = time.time()
         self.patch(time, "time").side_effect = [now + 60, now]
-        token = fernet_encrypt(testdata)
-        fernet_decrypt(token, ttl=1)
+        token = fernet_encrypt_psk(testdata)
+        fernet_decrypt_psk(token, ttl=1)
 
     def test__messages_from_the_past_exceeding_ttl_rejected(self):
         self.write_secret()
         testdata = factory.make_bytes()
         now = time.time()
         self.patch(time, "time").side_effect = [now - 2, now]
-        token = fernet_encrypt(testdata)
+        token = fernet_encrypt_psk(testdata)
         with ExpectedException(InvalidToken):
-            fernet_decrypt(token, ttl=1)
+            fernet_decrypt_psk(token, ttl=1)
 
     def test__messages_from_future_exceeding_clock_skew_limit_rejected(self):
         self.write_secret()
         testdata = factory.make_bytes()
         now = time.time()
         self.patch(time, "time").side_effect = [now + 61, now]
-        token = fernet_encrypt(testdata)
+        token = fernet_encrypt_psk(testdata)
         with ExpectedException(InvalidToken):
-            fernet_decrypt(token, ttl=1)
+            fernet_decrypt_psk(token, ttl=1)

=== modified file 'src/provisioningserver/utils/beaconing.py'
--- src/provisioningserver/utils/beaconing.py	2017-06-13 05:26:53 +0000
+++ src/provisioningserver/utils/beaconing.py	2017-06-21 15:18:45 +0000
@@ -5,18 +5,35 @@
 
 __all__ = [
     "BeaconingPacket",
+    "BeaconPayload",
+    "InvalidBeaconingPacket",
+    "create_beacon_payload",
+    "read_beacon_payload",
     "add_arguments",
     "run"
 ]
 
+from collections import namedtuple
+from gzip import (
+    compress,
+    decompress,
+)
 import json
 import os
 import stat
+import struct
 import subprocess
 import sys
 from textwrap import dedent
+import uuid
 
-import bson
+from bson import BSON
+from bson.errors import BSONError
+from cryptography.fernet import InvalidToken
+from provisioningserver.security import (
+    fernet_decrypt_psk,
+    fernet_encrypt_psk,
+)
 from provisioningserver.utils import sudo
 from provisioningserver.utils.network import format_eui
 from provisioningserver.utils.pcap import (
@@ -30,8 +47,119 @@
 )
 
 
+BEACON_PORT = 5240
+
+BEACON_TYPES = {
+    "solicitation": 1,
+    "advertisement": 2
+}
+
+BEACON_TYPE_VALUES = {
+    value: name for name, value in BEACON_TYPES.items()
+}
+
+PROTOCOL_VERSION = 1
+BEACON_HEADER_FORMAT_V1 = "!BBH"
+BEACON_HEADER_LENGTH_V1 = 4
+
+
+BeaconPayload = namedtuple('BeaconPayload', (
+    'bytes',
+    'version',
+    'type',
+    'payload',
+))
+
+
+def create_beacon_payload(beacon_type, payload=None, version=PROTOCOL_VERSION):
+    """Creates a beacon payload of the specified type, with the given data.
+
+    :param beacon_type: The beacon packet type. Indicates the purpose of the
+        beacon to the receiver.
+    :param payload: Optional JSON-encodable dictionary. Will be converted to an
+        inner encrypted payload and presented in the "data" field in the
+        resulting dictionary.
+    :param version: Optional protocol version to use (defaults to most recent).
+    :return: BeaconPayload namedtuple representing the packet bytes, the outer
+        payload, and the inner encrypted data (if any).
+    """
+    beacon_type_code = BEACON_TYPES[beacon_type]
+    if payload is not None:
+        payload = payload.copy()
+        payload["uuid"] = str(uuid.uuid1())
+        payload["type"] = beacon_type_code
+        data_bytes = BSON.encode(payload)
+        compressed_bytes = compress(data_bytes, compresslevel=9)
+        payload_bytes = fernet_encrypt_psk(compressed_bytes, raw=True)
+    else:
+        payload_bytes = b''
+    beacon_bytes = struct.pack(
+        BEACON_HEADER_FORMAT_V1 + "%ds" % len(payload_bytes),
+        version, beacon_type_code, len(payload_bytes), payload_bytes)
+    return BeaconPayload(
+        beacon_bytes, version, BEACON_TYPE_VALUES[beacon_type_code], payload)
+
+
+def read_beacon_payload(beacon_bytes):
+    """Returns a BeaconPayload namedtuple representing the given beacon bytes.
+
+    Decrypts the inner beacon data if necessary.
+
+    :param beacon_bytes: beacon payload (bytes).
+    :return: dict
+    """
+    if len(beacon_bytes) < BEACON_HEADER_LENGTH_V1:
+        raise InvalidBeaconingPacket(
+            "Beaconing packet must be at least %d bytes." % (
+                BEACON_HEADER_LENGTH_V1))
+    header = beacon_bytes[:BEACON_HEADER_LENGTH_V1]
+    version, beacon_type_code, expected_payload_length = struct.unpack(
+        BEACON_HEADER_FORMAT_V1, header)
+    actual_payload_length = len(beacon_bytes) - BEACON_HEADER_LENGTH_V1
+    if len(beacon_bytes) - BEACON_HEADER_LENGTH_V1 < expected_payload_length:
+        raise InvalidBeaconingPacket(
+            "Invalid payload length: expected %d bytes, got %d bytes." % (
+                expected_payload_length, actual_payload_length))
+    payload_start = BEACON_HEADER_LENGTH_V1
+    payload_end = BEACON_HEADER_LENGTH_V1 + expected_payload_length
+    payload_bytes = beacon_bytes[payload_start:payload_end]
+    payload = None
+    if version == 1:
+        if len(payload_bytes) == 0:
+            # No encrypted inner payload; nothing to do.
+            pass
+        else:
+            try:
+                decrypted_data = fernet_decrypt_psk(
+                    payload_bytes, raw=True)
+            except InvalidToken:
+                raise InvalidBeaconingPacket(
+                    "Failed to decrypt inner payload: check MAAS secret key.")
+            try:
+                decompressed_data = decompress(decrypted_data)
+            except OSError:
+                raise InvalidBeaconingPacket(
+                    "Failed to decompress inner payload: %r" % decrypted_data)
+            try:
+                # Replace the data in the dictionary with its decrypted form.
+                payload = BSON.decode(decompressed_data)
+            except BSONError:
+                raise InvalidBeaconingPacket(
+                    "Inner beacon payload is not BSON: %r" % decompressed_data)
+    else:
+        raise InvalidBeaconingPacket(
+            "Unknown beacon version: %d" % version)
+    beacon_type_code = payload["type"] if payload else beacon_type_code
+    return BeaconPayload(
+        beacon_bytes, version, BEACON_TYPE_VALUES[beacon_type_code], payload)
+
+
 class InvalidBeaconingPacket(Exception):
-    """Raised internally when a beaconing packet is not valid."""
+    """Raised when a beaconing packet is not valid."""
+
+    def __init__(self, invalid_reason):
+        self.invalid_reason = invalid_reason
+        super().__init__(invalid_reason)
 
 
 class BeaconingPacket:
@@ -57,12 +185,12 @@
         :param out: An object with `write(str)` and `flush()` methods.
         """
         try:
-            payload = bson.decode_all(self.packet)
+            payload = read_beacon_payload(self.packet)
             self.valid = True
             return payload
-        except bson.InvalidBSON:
+        except InvalidBeaconingPacket as ibp:
             self.valid = False
-            self.invalid_reason = "Packet payload is not BSON."
+            self.invalid_reason = ibp.invalid_reason
             return None
 
 
@@ -90,6 +218,8 @@
                     "destination_mac": format_eui(packet.l2.dst_eui),
                     "source_ip": str(packet.l3.src_ip),
                     "destination_ip": str(packet.l3.dst_ip),
+                    "source_port": packet.l4.packet.src_port,
+                    "destination_port": packet.l4.packet.dst_port,
                 }
                 if packet.l2.vid is not None:
                     output_json["vid"] = packet.l2.vid
@@ -99,11 +229,6 @@
                     out.write(json.dumps(output_json))
                     out.write('\n')
                     out.flush()
-                else:
-                    err.write(
-                        "Invalid beacon payload (not BSON): %r.\n" % (
-                            beacon.packet))
-                    err.flush()
             except PacketProcessingError as e:
                 err.write(e.error)
                 err.write("\n")

=== modified file 'src/provisioningserver/utils/tests/test_beaconing.py'
--- src/provisioningserver/utils/tests/test_beaconing.py	2017-06-13 16:09:05 +0000
+++ src/provisioningserver/utils/tests/test_beaconing.py	2017-06-21 15:18:45 +0000
@@ -6,39 +6,152 @@
 __all__ = []
 
 from argparse import ArgumentParser
+from gzip import compress
 import io
+import random
+import struct
 import subprocess
 from tempfile import NamedTemporaryFile
 from unittest.mock import Mock
+from uuid import UUID
 
-from bson import BSON
 from maastesting.factory import factory
 from maastesting.matchers import MockCalledOnceWith
 from maastesting.testcase import MAASTestCase
+from provisioningserver.security import (
+    fernet_encrypt_psk,
+    MissingSharedSecret,
+)
+from provisioningserver.tests.test_security import SharedSecretTestCase
 from provisioningserver.utils import beaconing as beaconing_module
 from provisioningserver.utils.beaconing import (
     add_arguments,
+    BEACON_HEADER_FORMAT_V1,
+    BEACON_TYPES,
     BeaconingPacket,
+    create_beacon_payload,
+    InvalidBeaconingPacket,
+    read_beacon_payload,
     run,
 )
 from provisioningserver.utils.script import ActionScriptError
+from testtools.matchers import (
+    Equals,
+    Is,
+    IsInstance,
+)
 from testtools.testcase import ExpectedException
 
 
-def make_beaconing_packet(payload):
-    # Beaconing packets are BSON-encoded byte strings.
-    beaconing_packet = BSON.encode(payload)
-    return beaconing_packet
+class TestCreateBeaconPayload(SharedSecretTestCase):
+
+    def test__requires_maas_shared_secret_for_inner_data_payload(self):
+        with ExpectedException(
+                MissingSharedSecret, ".*shared secret not found.*"):
+            create_beacon_payload("solicitation", payload={})
+
+    def test__returns_beaconpayload_namedtuple(self):
+        beacon = create_beacon_payload("solicitation")
+        self.assertThat(beacon.bytes, IsInstance(bytes))
+        self.assertThat(beacon.payload, Is(None))
+        self.assertThat(beacon.type, Equals("solicitation"))
+        self.assertThat(beacon.version, Equals(1))
+
+    def test__succeeds_when_shared_secret_present(self):
+        self.write_secret()
+        beacon = create_beacon_payload(
+            "solicitation", payload={})
+        self.assertThat(beacon.type, Equals("solicitation"))
+        self.assertThat(
+            beacon.payload['type'], Equals(BEACON_TYPES["solicitation"]))
+
+    def test__supplements_data_and_returns_complete_data(self):
+        self.write_secret()
+        random_type = random.choice(list(BEACON_TYPES.keys()))
+        random_key = factory.make_string(prefix="_")
+        random_value = factory.make_string()
+        beacon = create_beacon_payload(
+            random_type, payload={random_key: random_value})
+        # Ensure a valid UUID was added.
+        self.assertIsNotNone(UUID(beacon.payload['uuid']))
+        self.assertThat(beacon.type, Equals(random_type))
+        # The type is replicated here for authentication purposes.
+        self.assertThat(
+            beacon.payload['type'], Equals(BEACON_TYPES[random_type]))
+        self.assertThat(beacon.payload[random_key], Equals(random_value))
+
+    def test__creates_packet_that_can_decode(self):
+        self.write_secret()
+        random_type = random.choice(list(BEACON_TYPES.keys()))
+        random_key = factory.make_string(prefix="_")
+        random_value = factory.make_string()
+        packet_bytes, _, _, _ = create_beacon_payload(
+            random_type, payload={random_key: random_value})
+        decrypted = read_beacon_payload(packet_bytes)
+        self.assertThat(decrypted.type, Equals(random_type))
+        self.assertThat(decrypted.payload[random_key], Equals(random_value))
+
+
+def _make_beacon_payload(version=1, type_code=1, length=None, payload=None):
+    if payload is None:
+        payload = b''
+    if length is None:
+        length = len(payload)
+    packet = struct.pack(BEACON_HEADER_FORMAT_V1, version, type_code, length)
+    return packet + payload
+
+
+class TestReadBeaconPayload(SharedSecretTestCase):
+
+    def test__raises_if_packet_too_small(self):
+        with ExpectedException(
+                InvalidBeaconingPacket, ".*packet must be at least 4 bytes.*"):
+            read_beacon_payload(b"")
+
+    def test__raises_if_payload_too_small(self):
+        packet = _make_beacon_payload(payload=b'1234')[:6]
+        with ExpectedException(
+                InvalidBeaconingPacket, ".*expected 4 bytes, got 2 bytes.*"):
+            read_beacon_payload(packet)
+
+    def test__raises_when_version_incorrect(self):
+        packet = _make_beacon_payload(version=0xfe)
+        with ExpectedException(
+                InvalidBeaconingPacket, ".*Unknown beacon version.*"):
+            read_beacon_payload(packet)
+
+    def test__raises_when_inner_payload_does_not_decrypt(self):
+        self.write_secret()
+        packet = _make_beacon_payload(payload=b'\xfe')
+        with ExpectedException(
+                InvalidBeaconingPacket, ".*Failed to decrypt.*"):
+            read_beacon_payload(packet)
+
+    def test__raises_when_inner_encapsulation_does_not_decompress(self):
+        self.write_secret()
+        packet = _make_beacon_payload(
+            payload=fernet_encrypt_psk('\n\n', raw=True))
+        with ExpectedException(
+                InvalidBeaconingPacket, ".*Failed to decompress.*"):
+            read_beacon_payload(packet)
+
+    def test__raises_when_inner_encapsulation_is_not_bson(self):
+        self.write_secret()
+        payload = fernet_encrypt_psk(compress(b"\n\n"), raw=True)
+        packet = _make_beacon_payload(payload=payload)
+        with ExpectedException(
+                InvalidBeaconingPacket, ".*beacon payload is not BSON.*"):
+            read_beacon_payload(packet)
 
 
 class TestBeaconingPacket(MAASTestCase):
 
-    def test__is_valid__succeeds_for_valid_bson(self):
-        packet = make_beaconing_packet({"testing": 123})
-        beacon = BeaconingPacket(packet)
-        self.assertTrue(beacon.valid)
+    def test__is_valid__succeeds_for_valid_payload(self):
+        beacon = create_beacon_payload("solicitation")
+        beacon_packet = BeaconingPacket(beacon.bytes)
+        self.assertTrue(beacon_packet.valid)
 
-    def test__is_valid__fails_for_invalid_bson(self):
+    def test__is_valid__fails_for_invalid_payload(self):
         beacon = BeaconingPacket(b"\n\n\n\n")
         self.assertFalse(beacon.valid)
 
@@ -48,7 +161,7 @@
     b'\x00@\x00\x00\x01\x00\x00\x00v\xe19Y\xadF\x08\x00^\x00\x00\x00^\x00\x00'
     b'\x00\x01\x00^\x00\x00v\x00\x16>\x91zz\x08\x00E\x00\x00P\xe2E@\x00\x01'
     b'\x11\xe0\xce\xac\x10*\x02\xe0\x00\x00v\xda\xc2\x14x\x00<h(4\x00\x00\x00'
-    b'\x02uuid\x00%\x00\x00\x0078d1a4f0-4ca4-11e7-b2bb-00163e917a7a\x00\x00')
+    b'\x02uuid\x00%\x00\x00\x0000000000-0000-0000-0000-000000000000\x00\x00')
 
 
 class TestObserveBeaconsCommand(MAASTestCase):