mirror
/
federation
kopia lustrzana https://gitlab.com/jaywink/federation
1
0
Forkuj 0
Kod Zgłoszenia Packages Projekty Wydania Wiki Aktywność

Merge pull request #95 from jaywink/verify-handles 

Verify payload and entity handle are the same
merge-requests/130/head
Jason Robinson 2017-08-06 15:33:58 +03:00 zatwierdzone przez GitHub
rodzic 6ba6f905f3 934bd69bba
commit 54365b3856
6 zmienionych plików z 71 dodań i 28 usunięć
Pokaż statystyki Ściągnij plik aktualizacji Ściągnij plik porównania Expand all files Collapse all files

5
CHANGELOG.md
Wyświetl plik

@ -2,6 +2,11 @@
## [unreleased]
### Security
* Add proper checks to make sure Diaspora protocol payload handle and entity handle are the same. Even though we already verified the signature of the sender, we didn't ensure that the sender isn't trying to fake an entity authored by someone else.
The Diaspora protocol functions `message_to_objects` and `element_to_objects` now require a new parameter, the payload sender handle. These functions should normally not be needed to be used directly.
### Changed
* **Breaking change.** The high level `federation.outbound` functions `handle_send` and `handle_create_payload` signatures have been changed. This has been done to better represent the objects that are actually sent in and to add an optional `parent_user` object.

29
federation/entities/diaspora/mappers.py
Wyświetl plik

@ -52,12 +52,27 @@ def xml_children_as_dict(node):
return dict((e.tag, e.text) for e in node)
def element_to_objects(element, sender_key_fetcher=None, user=None):
def check_sender_and_entity_handle_match(sender_handle, entity_handle):
"""Ensure that sender and entity handles match.
Basically we've already verified the sender is who they say when receiving the payload. However, the sender might
be trying to set another author in the payload itself, since Diaspora has the sender in both the payload headers
AND the object. We must ensure they're the same.
"""
if sender_handle != entity_handle:
logger.warning("sender_handle and entity_handle don't match, aborting! sender_handle: %s, entity_handle: %s",
sender_handle, entity_handle)
return False
return True
def element_to_objects(element, sender, sender_key_fetcher=None, user=None):
"""Transform an Element to a list of entities recursively.
Possible child entities are added to each entity `_children` list.
:param tree: Element
:param sender: Payload sender handle
:param sender_key_fetcher: Function to fetch sender public key. If not given, key will always be fetched
over network. The function should take sender handle as the only parameter.
:param user: Optional receiving user object. If given, should have a `handle`.
@ -73,6 +88,8 @@ def element_to_objects(element, sender_key_fetcher=None, user=None):
if hasattr(cls, "fill_extra_attributes"):
transformed = cls.fill_extra_attributes(transformed)
entity = cls(**transformed)
if not check_sender_and_entity_handle_match(sender, entity.handle):
return []
# Add protocol name
entity._source_protocol = "diaspora"
# Save element object to entity for possible later use
@ -99,7 +116,7 @@ def element_to_objects(element, sender_key_fetcher=None, user=None):
return []
# Do child elements
for child in element:
entity._children.extend(element_to_objects(child))
entity._children.extend(element_to_objects(child, sender))
# Add to entities list
entities.append(entity)
if cls == DiasporaRequest:
@ -110,11 +127,13 @@ def element_to_objects(element, sender_key_fetcher=None, user=None):
return entities
def message_to_objects(message, sender_key_fetcher=None, user=None):
def message_to_objects(message, sender, sender_key_fetcher=None, user=None):
"""Takes in a message extracted by a protocol and maps it to entities.
:param message: XML payload
:type message: str
:param sender: Payload sender handle
:type message: str
:param sender_key_fetcher: Function to fetch sender public key. If not given, key will always be fetched
over network. The function should take sender handle as the only parameter.
:param user: Optional receiving user object. If given, should have a `handle`.
@ -123,12 +142,12 @@ def message_to_objects(message, sender_key_fetcher=None, user=None):
doc = etree.fromstring(message)
# Future Diaspora protocol version contains the element at top level
if doc.tag in TAGS:
return element_to_objects(doc, sender_key_fetcher, user)
return element_to_objects(doc, sender, sender_key_fetcher, user)
# Legacy Diaspora protocol wraps the element in <XML><post></post></XML>, so find the right element
for tag in TAGS:
element = doc.find(".//%s" % tag)
if element is not None:
return element_to_objects(element, sender_key_fetcher, user)
return element_to_objects(element, sender, sender_key_fetcher, user)
return []

2
federation/inbound.py
Wyświetl plik

@ -48,7 +48,7 @@ def handle_receive(payload, user=None, sender_key_fetcher=None, skip_author_veri
raise NoSuitableProtocolFoundError()
mappers = importlib.import_module("federation.entities.%s.mappers" % found_protocol.PROTOCOL_NAME)
entities = mappers.message_to_objects(message, sender_key_fetcher, user)
entities = mappers.message_to_objects(message, sender, sender_key_fetcher, user)
logger.debug("handle_receive: entities %s", entities)
return sender, found_protocol.PROTOCOL_NAME, entities

3
federation/tests/entities/diaspora/test_entities.py
Wyświetl plik

@ -147,7 +147,8 @@ class TestDiasporaRelayableMixin:
@patch("federation.entities.diaspora.mappers.DiasporaComment._validate_signatures")
def test_sign_with_parent(self, mock_validate):
entities = message_to_objects(DIASPORA_POST_COMMENT, sender_key_fetcher=Mock())
entities = message_to_objects(DIASPORA_POST_COMMENT, "alice@alice.diaspora.example.org",
sender_key_fetcher=Mock())
entity = entities[0]
entity.sign_with_parent(get_dummy_private_key())
assert entity.parent_signature == "UTIDiFZqjxfU6ssVlmjz2RwOD/WPmMTFv57qOm0BZvBhF8Ef49Ynse1c2XTtx3rs8DyRMn54" \

58
federation/tests/entities/diaspora/test_mappers.py
Wyświetl plik

@ -10,7 +10,8 @@ from federation.entities.base import (
from federation.entities.diaspora.entities import (
DiasporaPost, DiasporaComment, DiasporaLike, DiasporaRequest,
DiasporaProfile, DiasporaRetraction, DiasporaContact)
from federation.entities.diaspora.mappers import message_to_objects, get_outbound_entity
from federation.entities.diaspora.mappers import (
message_to_objects, get_outbound_entity, check_sender_and_entity_handle_match)
from federation.tests.fixtures.payloads import (
DIASPORA_POST_SIMPLE, DIASPORA_POST_COMMENT, DIASPORA_POST_LIKE,
DIASPORA_REQUEST, DIASPORA_PROFILE, DIASPORA_POST_INVALID, DIASPORA_RETRACTION,
@ -23,9 +24,9 @@ def mock_fill(attributes):
return attributes
class TestDiasporaEntityMappersReceive():
class TestDiasporaEntityMappersReceive:
def test_message_to_objects_simple_post(self):
entities = message_to_objects(DIASPORA_POST_SIMPLE)
entities = message_to_objects(DIASPORA_POST_SIMPLE, "alice@alice.diaspora.example.org")
assert len(entities) == 1
post = entities[0]
assert isinstance(post, DiasporaPost)
@ -39,7 +40,7 @@ class TestDiasporaEntityMappersReceive():
def test_message_to_objects_post_legacy(self):
# This is the previous XML schema used before renewal of protocol
entities = message_to_objects(DIASPORA_POST_LEGACY)
entities = message_to_objects(DIASPORA_POST_LEGACY, "alice@alice.diaspora.example.org")
assert len(entities) == 1
post = entities[0]
assert isinstance(post, DiasporaPost)
@ -52,12 +53,12 @@ class TestDiasporaEntityMappersReceive():
assert post.provider_display_name == "Socialhome"
def test_message_to_objects_legact_timestamp(self):
entities = message_to_objects(DIASPORA_POST_LEGACY_TIMESTAMP)
entities = message_to_objects(DIASPORA_POST_LEGACY_TIMESTAMP, "alice@alice.diaspora.example.org")
post = entities[0]
assert post.created_at == datetime(2011, 7, 20, 1, 36, 7)
def test_message_to_objects_post_with_photos(self):
entities = message_to_objects(DIASPORA_POST_WITH_PHOTOS)
entities = message_to_objects(DIASPORA_POST_WITH_PHOTOS, "alice@alice.diaspora.example.org")
assert len(entities) == 1
post = entities[0]
assert isinstance(post, DiasporaPost)
@ -75,7 +76,7 @@ class TestDiasporaEntityMappersReceive():
assert photo.public == False
assert photo.created_at == datetime(2011, 7, 20, 1, 36, 7)
entities = message_to_objects(DIASPORA_POST_WITH_PHOTOS_2)
entities = message_to_objects(DIASPORA_POST_WITH_PHOTOS_2, "xxxxxxxxxxxxxxx@diasp.org")
assert len(entities) == 1
post = entities[0]
assert isinstance(post, DiasporaPost)
@ -84,7 +85,8 @@ class TestDiasporaEntityMappersReceive():
@patch("federation.entities.diaspora.mappers.DiasporaComment._validate_signatures")
def test_message_to_objects_comment(self, mock_validate):
entities = message_to_objects(DIASPORA_POST_COMMENT, sender_key_fetcher=Mock())
entities = message_to_objects(DIASPORA_POST_COMMENT, "alice@alice.diaspora.example.org",
sender_key_fetcher=Mock())
assert len(entities) == 1
comment = entities[0]
assert isinstance(comment, DiasporaComment)
@ -102,7 +104,7 @@ class TestDiasporaEntityMappersReceive():
@patch("federation.entities.diaspora.mappers.DiasporaLike._validate_signatures")
def test_message_to_objects_like(self, mock_validate):
entities = message_to_objects(DIASPORA_POST_LIKE, sender_key_fetcher=Mock())
entities = message_to_objects(DIASPORA_POST_LIKE, "alice@alice.diaspora.example.org", sender_key_fetcher=Mock())
assert len(entities) == 1
like = entities[0]
assert isinstance(like, DiasporaLike)
@ -119,7 +121,7 @@ class TestDiasporaEntityMappersReceive():
mock_validate.assert_called_once_with()
def test_message_to_objects_request(self):
entities = message_to_objects(DIASPORA_REQUEST)
entities = message_to_objects(DIASPORA_REQUEST, "bob@example.com")
assert len(entities) == 2
sharing = entities[0]
assert isinstance(sharing, DiasporaRequest)
@ -136,7 +138,7 @@ class TestDiasporaEntityMappersReceive():
@patch("federation.entities.diaspora.entities.DiasporaProfile.fill_extra_attributes", new=mock_fill)
def test_message_to_objects_profile(self):
entities = message_to_objects(DIASPORA_PROFILE)
entities = message_to_objects(DIASPORA_PROFILE, "bob@example.com")
assert len(entities) == 1
profile = entities[0]
assert profile.handle == "bob@example.com"
@ -155,11 +157,11 @@ class TestDiasporaEntityMappersReceive():
@patch("federation.entities.diaspora.entities.DiasporaProfile.fill_extra_attributes", new=mock_fill)
def test_message_to_objects_profile_survives_empty_tag_string(self):
entities = message_to_objects(DIASPORA_PROFILE_EMPTY_TAGS)
entities = message_to_objects(DIASPORA_PROFILE_EMPTY_TAGS, "bob@example.com")
assert len(entities) == 1
def test_message_to_objects_retraction(self):
entities = message_to_objects(DIASPORA_RETRACTION)
entities = message_to_objects(DIASPORA_RETRACTION, "bob@example.com")
assert len(entities) == 1
entity = entities[0]
assert isinstance(entity, Retraction)
@ -168,7 +170,8 @@ class TestDiasporaEntityMappersReceive():
assert entity.entity_type == "Post"
def test_message_to_objects_retraction_legacy_request(self):
entities = message_to_objects(DIASPORA_LEGACY_REQUEST_RETRACTION, user=Mock(guid="swfeuihiwehuifhiwheiuf"))
entities = message_to_objects(DIASPORA_LEGACY_REQUEST_RETRACTION, "jaywink@iliketoast.net",
user=Mock(guid="swfeuihiwehuifhiwheiuf"))
assert len(entities) == 1
entity = entities[0]
assert isinstance(entity, Retraction)
@ -178,7 +181,7 @@ class TestDiasporaEntityMappersReceive():
assert entity._receiving_guid == "swfeuihiwehuifhiwheiuf"
def test_message_to_objects_contact(self):
entities = message_to_objects(DIASPORA_CONTACT)
entities = message_to_objects(DIASPORA_CONTACT, "alice@example.com")
assert len(entities) == 1
entity = entities[0]
assert isinstance(entity, DiasporaContact)
@ -188,32 +191,42 @@ class TestDiasporaEntityMappersReceive():
@patch("federation.entities.diaspora.mappers.logger.error")
def test_invalid_entity_logs_an_error(self, mock_logger):
entities = message_to_objects(DIASPORA_POST_INVALID)
entities = message_to_objects(DIASPORA_POST_INVALID, "alice@alice.diaspora.example.org")
assert len(entities) == 0
assert mock_logger.called
def test_adds_source_protocol_to_entity(self):
entities = message_to_objects(DIASPORA_POST_SIMPLE)
entities = message_to_objects(DIASPORA_POST_SIMPLE, "alice@alice.diaspora.example.org")
assert entities[0]._source_protocol == "diaspora"
@patch("federation.entities.diaspora.mappers.DiasporaComment._validate_signatures")
def test_source_object(self, mock_validate):
entities = message_to_objects(DIASPORA_POST_COMMENT, sender_key_fetcher=Mock())
entities = message_to_objects(DIASPORA_POST_COMMENT, "alice@alice.diaspora.example.org",
sender_key_fetcher=Mock())
entity = entities[0]
assert entity._source_object == etree.tostring(etree.fromstring(DIASPORA_POST_COMMENT))
@patch("federation.entities.diaspora.mappers.DiasporaComment._validate_signatures")
def test_element_to_objects_calls_sender_key_fetcher(self, mock_validate):
mock_fetcher = Mock()
message_to_objects(DIASPORA_POST_COMMENT, mock_fetcher)
message_to_objects(DIASPORA_POST_COMMENT, "alice@alice.diaspora.example.org", mock_fetcher)
mock_fetcher.assert_called_once_with("alice@alice.diaspora.example.org")
@patch("federation.entities.diaspora.mappers.DiasporaComment._validate_signatures")
@patch("federation.entities.diaspora.mappers.retrieve_and_parse_profile")
def test_element_to_objects_calls_retrieve_remote_profile(self, mock_retrieve, mock_validate):
message_to_objects(DIASPORA_POST_COMMENT)
message_to_objects(DIASPORA_POST_COMMENT, "alice@alice.diaspora.example.org")
mock_retrieve.assert_called_once_with("alice@alice.diaspora.example.org")
@patch("federation.entities.diaspora.mappers.check_sender_and_entity_handle_match")
def test_element_to_objects_verifies_handles_are_the_same(self, mock_check):
message_to_objects(DIASPORA_POST_SIMPLE, "bob@example.org")
mock_check.assert_called_once_with("bob@example.org", "alice@alice.diaspora.example.org")
def test_element_to_objects_returns_no_entity_if_handles_are_different(self):
entities = message_to_objects(DIASPORA_POST_SIMPLE, "bob@example.org")
assert not entities
class TestGetOutboundEntity:
def test_already_fine_entities_are_returned_as_is(self, private_key):
@ -279,3 +292,8 @@ class TestGetOutboundEntity:
entity = Comment()
entity.outbound_doc = "foobar"
assert get_outbound_entity(entity, private_key) == entity
def test_check_sender_and_entity_handle_match():
assert not check_sender_and_entity_handle_match("foo", "bar")
assert check_sender_and_entity_handle_match("foo", "foo")

2
federation/tests/fixtures/payloads.py vendored
Wyświetl plik

@ -118,7 +118,7 @@ DIASPORA_POST_WITH_PHOTOS_2 = """
<raw_message>#foo #bar (fewfefewfwfewfwe)</raw_message>
<photo>
<guid>fjwjewiofjoiwjfiowefewew</guid>
<diaspora_handle>xxxxxxxxxxxxxxxxx@diasp.org</diaspora_handle>
<diaspora_handle>xxxxxxxxxxxxxxx@diasp.org</diaspora_handle>
<public>true</public>
<created_at>2017-06-10T14:41:28Z</created_at>
<remote_photo_path>https://diasp.org/uploads/images/</remote_photo_path>