Skip to content

Commit 559fc13

Browse files
committed
feat(sdk/python)!: real ECDSA recovery for verify_env_encrypt_public_key
Replace the stub (always returned None) with a real secp256k1 + Keccak256 implementation backed by eth-keys + eth-utils. Adds the timestamped signature_v1 path the KMS now emits to defeat replay, while keeping a verify_env_encrypt_public_key_legacy fallback (with DeprecationWarning) for older KMS builds. BREAKING CHANGES: - verify_env_encrypt_public_key now requires a `timestamp: int` arg and performs real signature recovery. Callers using the old 3-arg form must switch to verify_env_encrypt_public_key_legacy (deprecated). - verify_signature_simple has been removed. It was an always-True placeholder that did no real verification. eth-keys and eth-utils are added to core dependencies (both pure-Python, already used by vmm-cli for the same flow). Round-trip and known-vector tests cover signature length, timestamp expiry/skew, custom max_age, malformed app_id, and tamper detection. Mirrors the JS SDK behavior in 3539db3 + 24eccb4.
1 parent f7a5d71 commit 559fc13

5 files changed

Lines changed: 282 additions & 245 deletions

File tree

sdk/python/pdm.lock

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sdk/python/pyproject.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ dependencies = [
1515
"asyncio>=3.4.3",
1616
"pydantic>=2.9.2",
1717
"cryptography>=43.0.0",
18+
"eth-keys>=0.5.0",
19+
"eth-utils>=4.0.0",
1820
]
1921
requires-python = ">=3.10"
2022
readme = "README.md"

sdk/python/src/dstack_sdk/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from .get_compose_hash import DockerConfig
2424
from .get_compose_hash import get_compose_hash
2525
from .verify_env_encrypt_public_key import verify_env_encrypt_public_key
26-
from .verify_env_encrypt_public_key import verify_signature_simple
26+
from .verify_env_encrypt_public_key import verify_env_encrypt_public_key_legacy
2727

2828
__all__ = [
2929
# Core clients
@@ -48,5 +48,5 @@
4848
"AppCompose",
4949
"DockerConfig",
5050
"verify_env_encrypt_public_key",
51-
"verify_signature_simple",
51+
"verify_env_encrypt_public_key_legacy",
5252
]

sdk/python/src/dstack_sdk/verify_env_encrypt_public_key.py

Lines changed: 86 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -2,121 +2,113 @@
22
#
33
# SPDX-License-Identifier: Apache-2.0
44

5-
"""Verify ECDSA signatures of environment-encrypt public keys.
5+
"""Verify ECDSA signatures on KMS env-encrypt public keys.
66
7-
This module prepares the message per dstack convention and offers a simplified
8-
API surface. Full public key recovery is not implemented.
7+
The KMS signs the X25519 env-encrypt public key it returns from
8+
``/GetAppEnvEncryptPubKey`` so deployers can prove the key originated from a
9+
specific signer before encrypting secrets against it. There are two message
10+
formats:
11+
12+
- ``signature_v1`` (preferred): includes a Unix timestamp to bound replay.
13+
- legacy: pubkey + app_id only. Kept for backward compatibility with old KMS
14+
builds. Vulnerable to replay; use the v1 variant whenever available.
15+
16+
Both formats sign ``keccak256(prefix + b":" + app_id + [timestamp_be_bytes] +
17+
public_key)`` with secp256k1, producing a 65-byte ``r || s || recovery_id``
18+
signature. This module recovers the signer's compressed public key.
919
"""
1020

11-
import hashlib
21+
import time
1222
from typing import Optional
23+
import warnings
1324

25+
from eth_keys import keys
26+
from eth_utils import keccak
1427

15-
def verify_env_encrypt_public_key(
16-
public_key: bytes, signature: bytes, app_id: str
17-
) -> Optional[str]:
18-
"""Attempt public key recovery from signature; return compressed key or None."""
19-
if len(signature) != 65:
20-
return None
28+
DEFAULT_MAX_AGE_SECONDS = 300
29+
_PREFIX = b"dstack-env-encrypt-pubkey"
30+
_SEPARATOR = b":"
31+
_FUTURE_SKEW_TOLERANCE_SECONDS = 60
2132

22-
try:
23-
# Create the message to verify
24-
prefix = b"dstack-env-encrypt-pubkey"
25-
26-
# Remove 0x prefix if present
27-
clean_app_id = app_id[2:] if app_id.startswith("0x") else app_id
28-
29-
# Validate hex string
30-
try:
31-
app_id_bytes = bytes.fromhex(clean_app_id)
32-
except ValueError:
33-
# Invalid hex string, return None
34-
return None
35-
36-
separator = b":"
37-
38-
# Construct message: prefix + ":" + app_id + public_key
39-
message = prefix + separator + app_id_bytes + public_key
40-
41-
# Hash the message with SHA3-256 (Keccak-256)
42-
# Note: Using hashlib.sha3_256 which is actually Keccak-256 in most implementations
43-
message_hash = hashlib.sha3_256(message).digest()
44-
45-
# Extract r, s, recovery_id from signature
46-
r = signature[:32]
47-
s = signature[32:64]
48-
recovery_id = signature[64]
49-
50-
# Convert r, s to integers
51-
r_int = int.from_bytes(r, byteorder="big")
52-
s_int = int.from_bytes(s, byteorder="big")
53-
54-
# Recover public key from signature
55-
# This is a simplified version - for full ECDSA recovery,
56-
# we need more complex logic to try different recovery possibilities
57-
58-
# For now, let's try a basic approach using known cryptographic libraries
59-
# This is where we would typically use a library like ethereum's ecrecover
60-
# Since we don't have that, let's implement a basic verification
61-
62-
# Try both possible recovery IDs (0 and 1, or adjusted by 27)
63-
for recovery_attempt in [recovery_id, recovery_id ^ 1]:
64-
# This is a stub. A proper ECDSA recovery implementation is required
65-
# to return a real public key. For now, always continue.
66-
recovered_key = _recover_public_key(
67-
message_hash, r_int, s_int, recovery_attempt
68-
)
69-
if recovered_key:
70-
return f"0x{recovered_key.hex()}"
7133

34+
def _normalize_app_id(app_id: str) -> Optional[bytes]:
35+
if app_id.startswith("0x"):
36+
app_id = app_id[2:]
37+
try:
38+
return bytes.fromhex(app_id)
39+
except ValueError:
7240
return None
7341

42+
43+
def _recover_signer(msg_hash: bytes, signature: bytes) -> Optional[str]:
44+
try:
45+
recovered = keys.Signature(
46+
signature_bytes=signature
47+
).recover_public_key_from_msg_hash(msg_hash)
48+
return "0x" + recovered.to_compressed_bytes().hex()
7449
except Exception:
75-
# Keep behavior non-fatal; callers treat None as invalid
76-
# Avoid bare except to satisfy lint rules
7750
return None
7851

7952

80-
def _recover_public_key(
81-
message_hash: bytes, r: int, s: int, recovery_id: int
82-
) -> Optional[bytes]:
83-
"""Recover public key from ECDSA signature components.
53+
def verify_env_encrypt_public_key(
54+
public_key: bytes,
55+
signature: bytes,
56+
app_id: str,
57+
timestamp: int,
58+
*,
59+
max_age_seconds: int = DEFAULT_MAX_AGE_SECONDS,
60+
) -> Optional[str]:
61+
"""Verify a timestamp-protected KMS env-encrypt public key signature.
8462
85-
This is a simplified implementation. In production, you should use
86-
a proper ECDSA recovery library like the one used in Ethereum.
63+
Returns the signer's compressed secp256k1 public key (0x-prefixed hex) on
64+
success, or ``None`` on bad signature length, expired/future timestamp,
65+
invalid hex app_id, or signature recovery failure.
8766
"""
88-
# Public key recovery is not implemented in this simplified version.
89-
return None
67+
if len(signature) != 65:
68+
return None
9069

70+
now = int(time.time())
71+
age = now - timestamp
72+
if age < -_FUTURE_SKEW_TOLERANCE_SECONDS:
73+
return None
74+
if age > max_age_seconds:
75+
return None
9176

92-
# Alternative implementation using a more direct approach
93-
def verify_signature_simple(public_key: bytes, signature: bytes, app_id: str) -> bool:
94-
"""Perform simple signature preprocessing without ECDSA recovery."""
95-
if len(signature) != 65:
96-
return False
77+
app_id_bytes = _normalize_app_id(app_id)
78+
if app_id_bytes is None:
79+
return None
9780

98-
try:
99-
# Create the message
100-
prefix = b"dstack-env-encrypt-pubkey"
101-
clean_app_id = app_id[2:] if app_id.startswith("0x") else app_id
81+
timestamp_bytes = timestamp.to_bytes(8, "big")
82+
message = (
83+
_PREFIX + _SEPARATOR + app_id_bytes + timestamp_bytes + public_key
84+
)
85+
return _recover_signer(keccak(message), signature)
10286

103-
# Validate hex string
104-
try:
105-
app_id_bytes = bytes.fromhex(clean_app_id)
106-
except ValueError:
107-
# Invalid hex string
108-
return False
10987

110-
separator = b":"
111-
message = prefix + separator + app_id_bytes + public_key
88+
def verify_env_encrypt_public_key_legacy(
89+
public_key: bytes,
90+
signature: bytes,
91+
app_id: str,
92+
) -> Optional[str]:
93+
"""Verify a legacy (non-timestamped) KMS signature.
11294
113-
# Hash with SHA3-256 (Keccak-256)
114-
# Compute hash to mirror verification flow, but unused in the stub
115-
_ = hashlib.sha3_256(message).digest()
95+
.. deprecated::
96+
Legacy signatures do not protect against replay attacks. Use
97+
:func:`verify_env_encrypt_public_key` with a timestamp from the KMS
98+
response whenever possible.
99+
"""
100+
warnings.warn(
101+
"verify_env_encrypt_public_key_legacy is deprecated; use "
102+
"verify_env_encrypt_public_key with a timestamp from KMS instead.",
103+
DeprecationWarning,
104+
stacklevel=2,
105+
)
106+
if len(signature) != 65:
107+
return None
116108

117-
# For now, return True to indicate the message was processed correctly
118-
# Full signature verification would require ECDSA implementation
119-
return True
109+
app_id_bytes = _normalize_app_id(app_id)
110+
if app_id_bytes is None:
111+
return None
120112

121-
except Exception:
122-
return False
113+
message = _PREFIX + _SEPARATOR + app_id_bytes + public_key
114+
return _recover_signer(keccak(message), signature)

0 commit comments

Comments
 (0)