Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions api/serializers/feeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from api.serializers.common import SensorSerializer, TagSerializer
from api.serializers.utils import PresenceFlagField, feed_type_as_list, get_valid_feed_types
from greedybear.consts import SHARE_TOKEN_MAX_AGE, SHARE_TOKEN_SALT
from greedybear.consts import SHARE_TOKEN_MAX_AGE
from greedybear.cronjobs.trending import validate_window_minutes
from greedybear.enums import IpReputation
from greedybear.models import ShareToken
Expand Down Expand Up @@ -305,7 +305,11 @@ def validate(self, attrs: dict) -> dict:
except ShareToken.DoesNotExist as exc:
raise serializers.ValidationError("Invalid or expired token") from exc
try:
attrs["feed_params"] = signing.loads(token, salt=SHARE_TOKEN_SALT, max_age=SHARE_TOKEN_MAX_AGE)
attrs["feed_params"] = signing.loads(
token,
salt=attrs["share_token"].signing_salt,
max_age=SHARE_TOKEN_MAX_AGE,
)
except signing.BadSignature as exc:
raise serializers.ValidationError("Invalid or expired token") from exc
return attrs
Expand Down
7 changes: 5 additions & 2 deletions api/views/feeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import hashlib
import json
from datetime import timedelta
from typing import cast

from certego_saas.ext.pagination import CustomPageNumberPagination
from django.contrib.postgres.aggregates import ArrayAgg
Expand Down Expand Up @@ -49,7 +50,7 @@
save_request_source,
stream_ioc_objects,
)
from greedybear.consts import SHARE_TOKEN_SALT, TRENDING_FEEDS_DATA_VERSION_KEY
from greedybear.consts import TRENDING_FEEDS_DATA_VERSION_KEY
from greedybear.cronjobs.repositories import PayloadRepository, TrendingBucketRepository
from greedybear.cronjobs.trending import build_ranked_attackers
from greedybear.models import IOC, ShareToken, ViewType
Expand Down Expand Up @@ -524,7 +525,9 @@ def share(self, request: Request) -> Response:
# so the token format and consume-time replay stay unchanged.
data = request.query_params.dict()
data.pop("reason", None)
token = signing.dumps(data, salt=SHARE_TOKEN_SALT)
# IsAuthenticated guarantees request.user is a persisted user.
user_id = cast("int", request.user.pk)
token = signing.dumps(data, salt=ShareToken.salt_for_user(user_id))
token_hash = hashlib.sha256(token.encode()).hexdigest()
ShareToken.objects.get_or_create(
token_hash=token_hash,
Expand Down
11 changes: 11 additions & 0 deletions greedybear/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.db.models.functions import Lower, Now
from netfields import CidrAddressField, NetManager

from greedybear.consts import SHARE_TOKEN_SALT
from greedybear.enums import IpReputation
from greedybear.settings import AUTH_USER_MODEL
from greedybear.storage import QuarantineStorage
Expand Down Expand Up @@ -393,6 +394,16 @@ def __str__(self):
status = "revoked" if self.revoked else "active"
return f"ShareToken({self.token_hash[:12]}… [{status}])"

@staticmethod
def salt_for_user(user_id: int) -> str:
"""Derive a signing salt scoped to the user who creates a share token."""
return f"{SHARE_TOKEN_SALT}:{user_id}"

@property
def signing_salt(self) -> str:
"""Salt this token was signed with, derived from its owner's ID."""
return self.salt_for_user(self.user_id) # ty: ignore[unresolved-attribute]


class AttackerActivityBucket(models.Model):
attacker_ip = models.GenericIPAddressField()
Expand Down
12 changes: 8 additions & 4 deletions tests/api/views/test_feeds_advanced_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,8 @@ def test_consume_shared_feed_with_end_date_max(self):
response = self.client.get(f"/api/feeds/consume/{token}")
self.assertEqual(response.status_code, 200)

def test_shareable_feed_expired_token(self):
"""Consuming a tampered/expired token returns 400."""
def test_shareable_feed_tampered_token(self):
"""Consuming a tampered token with a matching DB record returns 400."""
data = {
"feed_type": "all",
"attack_type": "all",
Expand All @@ -580,8 +580,12 @@ def test_shareable_feed_expired_token(self):
"start_date": None,
"end_date": None,
}
token = signing.dumps(data, salt="greedybear-feeds")
token = signing.dumps(data, salt=ShareToken.salt_for_user(self.superuser.pk))
tampered = token + "TAMPERED"
ShareToken.objects.create(
user=self.superuser,
token_hash=hashlib.sha256(tampered.encode()).hexdigest(),
)
self.client.logout()
response = self.client.get(f"/api/feeds/consume/{tampered}")
self.assertEqual(response.status_code, 400)
Expand All @@ -608,7 +612,7 @@ def test_consume_valid_token_without_db_record_is_rejected(self):
"start_date": None,
"end_date": None,
}
token = signing.dumps(data, salt="greedybear-feeds")
token = signing.dumps(data, salt=ShareToken.salt_for_user(self.superuser.pk))
token_hash = hashlib.sha256(token.encode()).hexdigest()

self.assertFalse(ShareToken.objects.filter(token_hash=token_hash).exists())
Expand Down
53 changes: 52 additions & 1 deletion tests/api/views/test_feeds_share_view.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
from unittest.mock import patch

from certego_saas.apps.user.models import User
from rest_framework import status
from rest_framework.test import APIClient

Expand Down Expand Up @@ -41,7 +44,8 @@ def test_share_reason_truncated_at_256_chars(self):
share_token = ShareToken.objects.get(token_hash=token_hash)
self.assertEqual(len(share_token.reason), 256)

def test_share_reason_only_set_on_create(self):
@patch("django.core.signing.time.time", return_value=1_800_000_000)
def test_share_reason_only_set_on_create(self, _mock_time):
"""get_or_create: when the same token already exists, reason is NOT overwritten."""
r1 = self.client.post("/api/feeds/share?reason=first")
self.assertEqual(r1.status_code, 200)
Expand All @@ -60,6 +64,53 @@ def test_share_reason_only_set_on_create(self):
share_token = ShareToken.objects.get(token_hash=token_hash)
self.assertEqual(share_token.reason, "first")

@patch("django.core.signing.time.time", return_value=1_800_000_000)
def test_same_second_shares_are_scoped_to_their_owners(self, _mock_time):
"""Identical feed params from two regular users must produce independently owned tokens."""
first_user = User.objects.create_user(
username="share-owner-a",
email="share-owner-a@example.com",
password="test-password",
)
second_user = User.objects.create_user(
username="share-owner-b",
email="share-owner-b@example.com",
password="test-password",
)
self.assertFalse(first_user.is_staff)
self.assertFalse(second_user.is_staff)

first_client = APIClient()
first_client.force_authenticate(user=first_user)
first_response = first_client.post("/api/feeds/share?asn=11111&reason=first")
self.assertEqual(first_response.status_code, 200)
first_url = first_response.json()["url"]
first_token = first_url.split("/api/feeds/consume/")[1]
first_hash = self._hash_from_url(first_url)

second_client = APIClient()
second_client.force_authenticate(user=second_user)
second_response = second_client.post("/api/feeds/share?asn=11111&reason=second")
self.assertEqual(second_response.status_code, 200)
second_url = second_response.json()["url"]
second_token = second_url.split("/api/feeds/consume/")[1]
second_hash = self._hash_from_url(second_url)

self.assertNotEqual(first_token, second_token)
self.assertEqual(ShareToken.objects.count(), 2)
self.assertEqual(ShareToken.objects.get(token_hash=first_hash).user, first_user)
self.assertEqual(ShareToken.objects.get(token_hash=first_hash).reason, "first")
self.assertEqual(ShareToken.objects.get(token_hash=second_hash).user, second_user)
self.assertEqual(ShareToken.objects.get(token_hash=second_hash).reason, "second")

self.assertEqual(len(first_client.get("/api/feeds/tokens/").json()), 1)
self.assertEqual(len(second_client.get("/api/feeds/tokens/").json()), 1)

self.assertEqual(second_client.post(f"/api/feeds/revoke/{first_token}").status_code, 403)
self.assertEqual(first_client.post(f"/api/feeds/revoke/{first_token}").status_code, 200)
self.assertEqual(first_client.post(f"/api/feeds/revoke/{second_token}").status_code, 403)
self.assertEqual(second_client.post(f"/api/feeds/revoke/{second_token}").status_code, 200)

def test_get_method_not_allowed(self):
"""GET is no longer accepted on the share endpoint; it must be POSTed."""
response = self.client.get("/api/feeds/share")
Expand Down
Loading