Skip to content

Commit

Permalink
Remove the optional "kid" parameter
Browse files Browse the repository at this point in the history
This removes the key identifier stored in the JWK's header in the
manifest v2 schema 1 signatures' payload.

The "kid" parameter is optional in JWK. This parameter might be also
considered irrelevant because the key pair for ECDSA is generated each
time the conversion from schema 2 to schema 1 happens. So, clients
cannot verify the origin of the signature with the fingerprint/kid
because the public key is created on the fly and then immediately trashed.

Ref: https://www.rfc-editor.org/rfc/rfc7517#section-4.5
Ref: https://docker-docs.uclv.cu/registry/spec/manifest-v2-1/

closes #1485

(cherry picked from commit 59e06e5)
  • Loading branch information
lubosmj committed Feb 14, 2024
1 parent a527808 commit d214327
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 53 deletions.
3 changes: 3 additions & 0 deletions CHANGES/1485.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Removed the optional "kid" parameter stored inside the signatures' payload generated during
docker manifest v2 schema 1 conversion. This change also removes the ``ecdsa`` dependency,
which is vulnerable to Minevra timing attacks.
52 changes: 0 additions & 52 deletions pulp_container/app/schema_convert.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import base64
import binascii
import datetime
import ecdsa
import hashlib
import itertools
import json
import logging

Expand Down Expand Up @@ -131,7 +127,6 @@ def convert(self):
)

key = jwk.ECKey().load_key(ecc.P256)
key.kid = getKeyId(key)
manifest_data = _jsonDumps(manifest)
signed_manifest_data = sign(manifest_data, key)
return manifest_data, signed_manifest_data
Expand Down Expand Up @@ -252,53 +247,6 @@ def sign(data, key):
return data_with_signature


def getKeyId(key):
"""
DER-encode the key and represent it in the format XXXX:YYYY:...
"""
derRepr = toDer(key)
shaRepr = hashlib.sha256(derRepr).digest()[:30]
b32Repr = base64.b32encode(shaRepr).decode()
return ":".join(byN(b32Repr, 4))


def toDer(key):
"""Return the DER-encoded representation of the key"""
point = (
b"\x00\x04" + number2string(key.x, key.curve.bytes) + number2string(key.y, key.curve.bytes)
)
der = ecdsa.der
curveEncodedOid = der.encode_oid(1, 2, 840, 10045, 3, 1, 7)
return der.encode_sequence(
der.encode_sequence(ecdsa.keys.encoded_oid_ecPublicKey, curveEncodedOid),
der.encode_bitstring(point),
)


def byN(strobj, N):
"""
Yield consecutive substrings of length N from string strobj
"""
it = iter(strobj)
while True:
substr = "".join(itertools.islice(it, N))
if not substr:
return
yield substr


def number2string(num, order):
"""
Hex-encode the number and return a zero-padded (to the left) to a total
length of 2*order
"""
# convert to hex
nhex = "%x" % num
# Zero-pad to the left so the length of the resulting unhexified string is order
nhex = nhex.rjust(2 * order, "0")
return binascii.unhexlify(nhex)


def compute_digest(manifest_data):
"""
Compute the digest from the passed manifest data.
Expand Down
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
# remember to also update unittest_requirements.txt when updating this file
pulpcore>=3.17.0,<3.20
ecdsa~=0.14
pyjwkest~=1.4.0
pyjwt[crypto]~=1.7.1
url-normalize~=1.4.2

0 comments on commit d214327

Please sign in to comment.