Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check DID Identifier checksum in validation #8

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Check DID Identifier checksum in validation #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion iotics/lib/identity/crypto/identity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ def make_identifier(public_bytes: bytes) -> str:
checksum = bytearray.fromhex(cl2.hexdigest())[:4]

return IDENTIFIER_PREFIX + base58.b58encode(bytes([IDENTIFIER_METHOD, IDENTIFIER_VERSION, IDENTIFIER_PAD])
+ pk_digest + checksum).decode('ascii')
+ pk_digest + checksum,
alphabet=base58.BITCOIN_ALPHABET).decode('ascii')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the use of this alphabet is fundamental - how do other impl of the spec would know about this?
would all of them need to use the same "bitcoin_alphabet" ??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base58 was written for bitcoin so the default is "bitcoin alphabet". I only specify here in case the default in the upstream library changes and to make the code clearer to read. I'm happy to revert this override if you prefer.

16 changes: 15 additions & 1 deletion iotics/lib/identity/validation/identity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
# Copyright (c) IOTIC LABS LIMITED. All rights reserved. Licensed under the Apache License, Version 2.0.

import re
from hashlib import blake2b

from iotics.lib.identity.const import IDENTIFIER_ID_PATTERN, IDENTIFIER_NAME_PATTERN, ISSUER_PATTERN
import base58

from iotics.lib.identity.const import IDENTIFIER_ID_PATTERN, IDENTIFIER_NAME_PATTERN, ISSUER_PATTERN, IDENTIFIER_PREFIX
from iotics.lib.identity.error import IdentityValidationError


Expand All @@ -20,6 +23,17 @@ def validate_identifier(did: str):
if result is None:
raise IdentityValidationError(f'Identifier does not match pattern {did} - {IDENTIFIER_ID_PATTERN}')

did_bytes = base58.b58decode(did[len(IDENTIFIER_PREFIX):], alphabet=base58.BITCOIN_ALPHABET)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

following up the comment above, should this magic number be put in a constant (since it's repeated in many places or maybe have a id_sdk.did_decode/id_sdk.did_encode functions that wrap base58.b58decode/b58encode(alphabet=base58.BITCOIN_APLHABET) ?
just saying in order to have our custom logic somewhere in one place only

did_digest = did_bytes[3:23]
did_checksum = did_bytes[-4:].hex()

cl2 = blake2b(digest_size=20)
cl2.update(did_digest)
checksum = cl2.hexdigest()[:8]

if did_checksum != checksum:
raise IdentityValidationError(f'Identifier checksum does not match {did_checksum} != {checksum}')

@staticmethod
def validate_issuer_string(issuer: str):
"""
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/iotics/lib/identity/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,14 @@ def other_key_pair(other_key_pair_secrets):

@pytest.fixture
def doc_did():
return 'did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY'
return 'did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this is a magic string that's repeated in many tests, would it be better to have it in a constant with a meaningful name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it would be clearer. I did not want to refactor too much of the tests. I had to override this value because it does not pass the identifier checksum validation.



@pytest.fixture
def deleg_doc_did():
return 'did:iotics:iotHHHHKpPGWWWC4FFo4d6oyzVVk6MXLmEgY'
return 'did:iotics:iotDX4pLcd4y9RA6EvUrj6QaLSjxsZT6EEPJ'


@pytest.fixture
def another_doc_did():
return 'did:iotics:iotCmGJdzEte5dtvjjUspcoAeiWk5D5NTTyE'
2 changes: 1 addition & 1 deletion tests/unit/iotics/lib/identity/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def get_doc_with_keys(public_keys: Iterable[RegisterPublicKey] = None,
_ = [builder.add_control_delegation_obj(k) for k in (deleg_control or ())]
_ = [builder.add_authentication_delegation_obj(k) for k in (deleg_auth or ())]

return builder.build(did=did or 'did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY',
return builder.build(did=did or 'did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7',
purpose=DIDType.TWIN,
proof='a proof',
revoked=True,
Expand Down
8 changes: 4 additions & 4 deletions tests/unit/iotics/lib/identity/register/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ def valid_public_key_base58():

@pytest.fixture
def a_controller():
return Issuer('did:iotics:iotHjrmKpPGWyEC4FFo4d6oyzVVk6MXEEEEE', '#AController')
return Issuer('did:iotics:iotCxqy4BPFVZFJEXGpiTDeDCyDPt8LqLqEf', '#AController')


@pytest.fixture
def b_controller():
return Issuer('did:iotics:iotHjrmKpPGWyEC4FFo4d6oyzVVk6MXXXXXX', '#AController')
return Issuer('did:iotics:iotCGwYfJoBDd791NaXpRDRN6qW8xUyheFLS', '#AController')


@pytest.fixture
Expand Down Expand Up @@ -95,13 +95,13 @@ def minimal_doc(doc_did, doc_proof, min_doc_owner_pub_key):


@pytest.fixture
def full_doc(doc_keys, doc_did, doc_proof, doc_controller):
def full_doc(doc_keys, doc_did, another_doc_did, doc_proof, doc_controller):
return RegisterDocument(did=doc_did,
purpose=DIDType.TWIN,
proof=doc_proof,
revoked=True,
metadata=Metadata.build('a label', 'a comment', 'http://a/url'),
creator='did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MEEEEgY',
creator=another_doc_did,
spec_version=SUPPORTED_VERSIONS[0],
update_time=get_unix_time_ms(),
controller=doc_controller,
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/iotics/lib/identity/register/test_document_builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ def test_can_build_a_register_doc_with_controller_and_not_owner_public_key(doc_d
assert new_doc.controller == controller


def test_can_build_a_register_doc_with_full_data(doc_keys, doc_did, doc_proof, doc_controller):
def test_can_build_a_register_doc_with_full_data(doc_keys, doc_did, another_doc_did, doc_proof, doc_controller):
now_before_create = get_unix_time_ms()
metadata = Metadata.build('a label', 'a comment', 'http://a/url')
spec_version = SUPPORTED_VERSIONS[0]
creator = 'did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MEEEEgY'
creator = another_doc_did
new_doc = RegisterDocumentBuilder() \
.add_public_key(doc_keys['#pub_key1'].name, doc_keys['#pub_key1'].base58, doc_keys['#pub_key1'].revoked) \
.add_public_key_obj(doc_keys['#pub_key2']) \
Expand Down Expand Up @@ -174,14 +174,14 @@ def test_can_build_a_register_doc_from_an_other_doc(minimal_doc, full_doc, is_mi

def test_can_build_a_register_doc_from_an_other_doc_overriding_values(full_doc):
existing_doc = full_doc
new_creator = 'did:iotics:iotEEEEKpPGWyEC4FFo4d6oyzVVk6MEEEEgY'
new_controller = Issuer.build('did:iotics:iotEEEEKpPGWyEC4FFo4d6oyzHHHHMEEEEgY', '#NewController')
new_creator = 'did:iotics:iotA5H2cacnZyRCcuKd6wxPkbNxhAw7WCL2G'
new_controller = Issuer.build('did:iotics:iotB4X2uEA4ZCAHkMjCci8HzD8gU1UoeRH53', '#NewController')
new_metadata = Metadata(label='a label')
new_version = SUPPORTED_VERSIONS[0]
new_pub_key = RegisterPublicKey(name='#new_pub_key1', base58=get_public_base_58_key(), revoked=False)
new_auth_key = RegisterAuthenticationPublicKey(name='#new_auth_key1', base58=get_public_base_58_key(),
revoked=False)
a_controller = Issuer.from_string('did:iotics:iotHjrmKpPGWyEC4FFo4d6oyzVVk6MXEEEEE#AController')
a_controller = Issuer.from_string('did:iotics:iotC3MU9Bhfx6o2CMq8rJb5sbLJTd5HoJmH5#AController')
new_control_deleg_proof = RegisterDelegationProof(name='#new_deleg_control_key1', controller=a_controller,
proof='a_deleg_proof_validated_by_the_resolver',
revoked=False)
Expand Down
16 changes: 8 additions & 8 deletions tests/unit/iotics/lib/identity/register/test_document_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ def test_is_issuer_in_keys_returns_true_if_issuer_in_auth_keys(public_keys, auth

@pytest.fixture
def control_deleg_proof():
issuer1 = Issuer.from_string('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY#ctrl1')
issuer2 = Issuer.from_string('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY#ctrl2')
issuer3 = Issuer.from_string('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY#ctrl3')
issuer1 = Issuer.from_string('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7#ctrl1')
issuer2 = Issuer.from_string('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7#ctrl2')
issuer3 = Issuer.from_string('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7#ctrl3')
return {'#DelegCtrlKey1': RegisterDelegationProof.build('#DelegCtrlKey1',
issuer1,
proof='proof', revoked=False),
Expand All @@ -95,9 +95,9 @@ def control_deleg_proof():

@pytest.fixture
def auth_deleg_proof():
issuer1 = Issuer.from_string('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY#ctrl1')
issuer2 = Issuer.from_string('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY#ctrl2')
issuer3 = Issuer.from_string('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY#ctrl3')
issuer1 = Issuer.from_string('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7#ctrl1')
issuer2 = Issuer.from_string('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7#ctrl2')
issuer3 = Issuer.from_string('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7#ctrl3')
return {'#DelegAuthKey1': RegisterDelegationProof.build('#DelegAuthKey1',
issuer1,
proof='proof', revoked=False),
Expand Down Expand Up @@ -157,7 +157,7 @@ def test_can_get_control_delegation_by_controller(control_deleg_proof, min_doc_o

def test_get_control_delegation_by_controller_returns_none_if_not_found(control_deleg_proof, min_doc_owner_pub_key):
doc = get_doc_with_keys(deleg_control=control_deleg_proof.values(), public_keys=[min_doc_owner_pub_key])
issuer = Issuer.build('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY', '#DoesNotExist')
issuer = Issuer.build('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7', '#DoesNotExist')
deleg_proof = RegisterDocumentHelper.get_register_delegation_proof_by_controller(issuer, doc, include_auth=False)
assert not deleg_proof

Expand All @@ -173,7 +173,7 @@ def test_can_get_auth_delegation_by_controller(auth_deleg_proof, min_doc_owner_p

def test_get_auth_delegation_by_controller_returns_none_if_not_found(auth_deleg_proof, min_doc_owner_pub_key):
doc = get_doc_with_keys(deleg_auth=auth_deleg_proof.values(), public_keys=[min_doc_owner_pub_key])
issuer = Issuer.build('did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY', '#DoesNotExist')
issuer = Issuer.build('did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7', '#DoesNotExist')
deleg_proof = RegisterDocumentHelper.get_register_delegation_proof_by_controller(issuer, doc, include_auth=True)
assert not deleg_proof

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/iotics/lib/identity/validation/test_identifier.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

@pytest.fixture
def an_identifier():
return 'did:iotics:iotHHHHKpPGWyEC4FFo4d6oyzVVk6MXLmEgY'
return 'did:iotics:iotDadb3rSWedk8iqExSbwqLtijG5XQByHC7'


def test_validate_identifier_do_not_raises_if_valid_identifier(an_identifier):
Expand Down