Skip to content

Commit 8769fbe

Browse files
authored
Adds hmac signing of the invocation result file (#4366)
See DIAGNijmegen/rse-roadmap#443
1 parent 8d2175e commit 8769fbe

13 files changed

Lines changed: 365 additions & 65 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Generated by Django 4.2.25 on 2025-10-20 14:20
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("algorithms", "0080_alter_algorithmimage_options_and_more"),
10+
]
11+
12+
operations = [
13+
migrations.AddField(
14+
model_name="job",
15+
name="signing_key",
16+
field=models.BinaryField(
17+
help_text="The key used to sign the inference result file",
18+
max_length=32,
19+
null=True,
20+
),
21+
),
22+
]
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Generated by Django 4.2.25 on 2025-10-20 14:21
2+
3+
from django.db import migrations
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("algorithms", "0081_job_signing_key"),
10+
]
11+
12+
operations = [
13+
migrations.RunSQL(
14+
sql="CREATE EXTENSION IF NOT EXISTS pgcrypto;",
15+
reverse_sql="",
16+
elidable=True,
17+
),
18+
migrations.RunSQL(
19+
sql=(
20+
"UPDATE algorithms_job SET signing_key = gen_random_bytes(32) WHERE signing_key IS NULL;"
21+
),
22+
reverse_sql="UPDATE algorithms_job SET signing_key = NULL;",
23+
elidable=True,
24+
),
25+
]
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Generated by Django 4.2.25 on 2025-10-20 14:21
2+
3+
import secrets
4+
5+
from django.db import migrations, models
6+
7+
8+
class Migration(migrations.Migration):
9+
10+
dependencies = [
11+
("algorithms", "0082_auto_20251020_1421"),
12+
]
13+
14+
operations = [
15+
migrations.AlterField(
16+
model_name="job",
17+
name="signing_key",
18+
field=models.BinaryField(
19+
default=secrets.token_bytes,
20+
help_text="The key used to sign the inference result file",
21+
max_length=32,
22+
unique=True,
23+
),
24+
),
25+
]

app/grandchallenge/challenges/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,6 +1292,7 @@ def compute_costs_euro_cents_per_hour(self):
12921292
time_limit=self.inference_time_limit_in_minutes,
12931293
requires_gpu_type=gpu_type,
12941294
use_warm_pool=False,
1295+
signing_key=b"",
12951296
)
12961297
for gpu_type in self.algorithm_selectable_gpu_type_choices
12971298
]

app/grandchallenge/components/backends/base.py

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
11
import asyncio
2+
import binascii
23
import functools
4+
import hashlib
5+
import hmac
36
import io
47
import json
58
import logging
69
import os
10+
import secrets
711
from abc import ABC, abstractmethod
812
from json import JSONDecodeError
913
from math import ceil
@@ -272,6 +276,7 @@ def __init__(
272276
time_limit: int,
273277
requires_gpu_type: GPUTypeChoices,
274278
use_warm_pool: bool,
279+
signing_key: bytes,
275280
algorithm_model=None,
276281
ground_truth=None,
277282
**kwargs,
@@ -285,6 +290,7 @@ def __init__(
285290
self._use_warm_pool = (
286291
use_warm_pool and settings.COMPONENTS_USE_WARM_POOL
287292
)
293+
self._signing_key = signing_key
288294
self._stdout = []
289295
self._stderr = []
290296
self.__s3_client = None
@@ -388,15 +394,21 @@ def invocation_environment(self):
388394
"GRAND_CHALLENGE_COMPONENT_MAX_MEMORY_MB": str(
389395
self._max_memory_mb
390396
),
397+
"GRAND_CHALLENGE_COMPONENT_SIGNING_KEY_HEX": binascii.hexlify(
398+
self._signing_key
399+
).decode("ascii"),
391400
}
401+
392402
if self._algorithm_model:
393403
env["GRAND_CHALLENGE_COMPONENT_MODEL"] = (
394404
f"s3://{settings.COMPONENTS_INPUT_BUCKET_NAME}/{self._algorithm_model_key}"
395405
)
406+
396407
if self._ground_truth:
397408
env["GRAND_CHALLENGE_COMPONENT_GROUND_TRUTH"] = (
398409
f"s3://{settings.COMPONENTS_INPUT_BUCKET_NAME}/{self._ground_truth_key}"
399410
)
411+
400412
return env
401413

402414
@property
@@ -770,43 +782,57 @@ def _get_upload_input_content_task(*, content, key):
770782
)
771783

772784
def _get_task_return_code(self):
773-
with io.BytesIO() as fileobj:
774-
try:
775-
self._s3_client.download_fileobj(
776-
Fileobj=fileobj,
777-
Bucket=settings.COMPONENTS_OUTPUT_BUCKET_NAME,
778-
Key=self._result_key,
779-
)
780-
except botocore.exceptions.ClientError as error:
781-
if error.response["Error"]["Code"] == "404":
782-
raise UncleanExit(
783-
"The invocation request did not return a result"
784-
) from error
785-
else:
786-
raise
785+
try:
786+
response = self._s3_client.get_object(
787+
Bucket=settings.COMPONENTS_OUTPUT_BUCKET_NAME,
788+
Key=self._result_key,
789+
)
790+
except botocore.exceptions.ClientError as error:
791+
if error.response["Error"]["Code"] == "404":
792+
raise UncleanExit(
793+
"The invocation request did not return a result"
794+
) from error
795+
else:
796+
raise
787797

788-
fileobj.seek(0)
798+
body = response["Body"].read()
789799

790-
try:
791-
result = json.loads(
792-
fileobj.read().decode("utf-8"),
793-
)
794-
except JSONDecodeError:
795-
raise ComponentException(
796-
"The invocation request did not return valid json"
797-
)
800+
signature_hmac_sha256 = response["Metadata"].get(
801+
"signature_hmac_sha256"
802+
)
803+
body_signature_hmac_sha256 = hmac.new(
804+
key=self._signing_key, msg=body, digestmod=hashlib.sha256
805+
).hexdigest()
806+
807+
if signature_hmac_sha256 and not secrets.compare_digest(
808+
body_signature_hmac_sha256, signature_hmac_sha256
809+
):
810+
# TODO The signature should always be present when all images use sagemaker shim >= 0.5.0
811+
logger.error(
812+
"The invocation response object has been tampered with"
813+
)
814+
raise ComponentException(
815+
"The invocation response object has been tampered with"
816+
)
798817

799-
logger.info(f"{result=}")
818+
try:
819+
result = json.loads(body.decode("utf-8"))
820+
except JSONDecodeError:
821+
raise ComponentException(
822+
"The invocation request did not return valid json"
823+
)
800824

801-
if result["pk"] != self._job_id:
802-
raise RuntimeError("Wrong result key for this job")
825+
logger.info(f"{result=}")
803826

804-
try:
805-
return int(result["return_code"])
806-
except (KeyError, ValueError):
807-
raise ComponentException(
808-
"The invocation response object is not valid"
809-
)
827+
if result["pk"] != self._job_id:
828+
raise RuntimeError("Wrong result key for this job")
829+
830+
try:
831+
return int(result["return_code"])
832+
except (KeyError, ValueError):
833+
raise ComponentException(
834+
"The invocation response object is not valid"
835+
)
810836

811837
def _handle_completed_job(self):
812838
users_process_exit_code = self._get_task_return_code()

app/grandchallenge/components/models.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import json
22
import logging
33
import re
4+
import secrets
45
from enum import Enum
56
from json import JSONDecodeError
67
from pathlib import Path
@@ -1628,6 +1629,13 @@ class ComponentJob(FieldChangeMixin, UUIDModel):
16281629
"/input/foo/bar/<relative_path>"
16291630
),
16301631
)
1632+
signing_key = models.BinaryField(
1633+
max_length=32,
1634+
default=secrets.token_bytes,
1635+
unique=True,
1636+
editable=False,
1637+
help_text="The key used to sign the inference result file",
1638+
)
16311639
task_on_success = models.JSONField(
16321640
default=None,
16331641
null=True,
@@ -1688,6 +1696,7 @@ def save(self, *args, **kwargs):
16881696
"requires_gpu_type",
16891697
"requires_memory_gb",
16901698
"time_limit",
1699+
"signing_key",
16911700
):
16921701
if self.has_changed(field):
16931702
raise ValueError(f"{field} cannot be changed")
@@ -1757,6 +1766,7 @@ def executor_kwargs(self):
17571766
"requires_gpu_type": self.requires_gpu_type,
17581767
"memory_limit": self.requires_memory_gb,
17591768
"use_warm_pool": self.use_warm_pool,
1769+
"signing_key": self.signing_key,
17601770
}
17611771

17621772
def get_executor(self, *, backend):
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Generated by Django 4.2.25 on 2025-10-20 14:20
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("evaluation", "0097_alter_evaluationgroundtruth_options"),
10+
]
11+
12+
operations = [
13+
migrations.AddField(
14+
model_name="evaluation",
15+
name="signing_key",
16+
field=models.BinaryField(
17+
help_text="The key used to sign the inference result file",
18+
max_length=32,
19+
null=True,
20+
),
21+
),
22+
]
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Generated by Django 4.2.25 on 2025-10-20 14:20
2+
3+
from django.db import migrations
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("evaluation", "0098_evaluation_signing_key"),
10+
]
11+
12+
operations = [
13+
migrations.RunSQL(
14+
sql="CREATE EXTENSION IF NOT EXISTS pgcrypto;",
15+
reverse_sql="",
16+
elidable=True,
17+
),
18+
migrations.RunSQL(
19+
sql=(
20+
"UPDATE evaluation_evaluation SET signing_key = gen_random_bytes(32) WHERE signing_key IS NULL;"
21+
),
22+
reverse_sql="UPDATE evaluation_evaluation SET signing_key = NULL;",
23+
elidable=True,
24+
),
25+
]
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Generated by Django 4.2.25 on 2025-10-20 14:21
2+
3+
import secrets
4+
5+
from django.db import migrations, models
6+
7+
8+
class Migration(migrations.Migration):
9+
10+
dependencies = [
11+
("evaluation", "0099_auto_20251020_1420"),
12+
]
13+
14+
operations = [
15+
migrations.AlterField(
16+
model_name="evaluation",
17+
name="signing_key",
18+
field=models.BinaryField(
19+
default=secrets.token_bytes,
20+
help_text="The key used to sign the inference result file",
21+
max_length=32,
22+
unique=True,
23+
),
24+
),
25+
]

0 commit comments

Comments
 (0)