nfcpy / ndeflib

Python package for parsing and generating NFC Data Exchange Format messages.
ISC License
63 stars 12 forks source link

Adding NDEF Signature Record #4

Closed nickaknudson closed 6 years ago

nickaknudson commented 6 years ago

I'm adding NDEF Signature Record to this library.

I'm currently struggling with an elegant way to calculate the signature during message encoding; but because message._message_encoder is a generator, there isn't a way to "peek" at the current state of the message. Therefore one has to summon the encoder multiple times to achieve the desired results (see script below).

Can anyone think of a better way to do this? Should I modify message.py to make integration easier?

Currently this is the script that I am using the generate Signature Records:

import os,sys,inspect
currentdir = os.path.dirname(os.path.abspath(inspect.getfile(inspect.currentframe())))
parentdir = os.path.dirname(currentdir)
sys.path.insert(0,parentdir)
import ndef
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.asymmetric import ec
from cryptography.hazmat.primitives.asymmetric import utils
from asn1crypto.algos import DSASignature
from more_itertools import peekable
private_key = ec.generate_private_key(ec.SECP256K1(), default_backend())

r1=ndef.UriRecord("https://example.com")
r2=ndef.TextRecord("TEST")
encoder = ndef.message_encoder()
results = list()
encoder.send(None)
encoder.send(r1)
results.append(encoder.send(r2))
results.append(encoder.send(ndef.SignatureRecord()))
print(results)
msg_to_sign = b''.join(results)
print(msg_to_sign)
signature = private_key.sign(msg_to_sign,ec.ECDSA(hashes.SHA256()))
signature_p1363 = DSASignature.load(signature, strict=True).to_p1363()
r3=ndef.SignatureRecord("ECDSA-P256", "SHA-256", signature_p1363)
encoder = ndef.message_encoder()
results = list()
encoder.send(None)
encoder.send(r1)
results.append(encoder.send(r2))
results.append(encoder.send(r3))
results.append(encoder.send(None))
m=b''.join(results)
print(m)

d=ndef.message_decoder(m)
d1=next(d)
d2=next(d)
d3=next(d)

encoder = ndef.message_encoder()
results = list()
encoder.send(None)
encoder.send(r1)
results.append(encoder.send(r2))
results.append(encoder.send(ndef.SignatureRecord()))
print(results)
msg_to_verify = b''.join(results)

public_key = private_key.public_key()
verification = public_key.verify(DSASignature.from_p1363(d3.signature).dump(),msg_to_verify,ec.ECDSA(hashes.SHA256()))
TE-StephenTiedemann commented 6 years ago

Thanks a lot for contributing a signature record implementation. It has been asked for since a while but I've never found the time to do it.

Here is an alternative way for encoding. It appends a partly prepared signature record, then encodes the preceding records and adds the signature data before the final encode step.

stream = io.BytesIO()
records = [r1, r2, ndef.SignatureRecord("ECDSA-P256", "SHA-256")]
encoder = ndef.message_encoder(records, stream)
for _ in range(len(records) - 1): next(encoder)
signature = private_key.sign(stream.getvalue(), ec.ECDSA(hashes.SHA256()))
records[-1].signature = DSASignature.load(signature, strict=True).to_p1363()
next(encoder)
octets = stream.getvalue()

For signature verification it is important to use the same payload octets as in the received version. This can be done by setting known_types to just the signature record, everything else is then decoded as a plain record. The last line then decodes the verified records into those known by ndeflib.

# octets = received ndef message octets
records = []
records_to_verify = []
known_types = {'urn:nfc:wkt:Sig': ndef.signature.SignatureRecord}
for record in ndef.message_decoder(octets, known_types=known_types):
    if not record.type == 'urn:nfc:wkt:Sig':
        records_to_verify.append(record)
    else:
        octets_to_verify = b''.join(ndef.message_encoder(records_to_verify))
        if public_key.verify( ... record.signature ... octets_to_verify ...) is True:
            records.extend(records_to_verify)
        records_to_verify = []
records = list(ndef.message_decoder(b''.join(ndef.message_encoder(records))))
nickaknudson commented 6 years ago

Your encoding/decoding looks much cleaner.

I will fix failing tests and add documentation. Look for an update later today.

Anything else you would like in this merge request?

nickaknudson commented 6 years ago

I had to modify your solution in order to make it work because you were reassembling records_to_verify without an additional record at the end, therefore the message_end bit was being set. This test script works for me:

import os,sys,inspect
currentdir = os.path.dirname(os.path.abspath(inspect.getfile(inspect.currentframe())))
parentdir = os.path.dirname(currentdir)
sys.path.insert(0,parentdir)

import ndef
import io
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.asymmetric import ec
from cryptography.hazmat.primitives.asymmetric import utils
from cryptography.exceptions import InvalidSignature
from asn1crypto.algos import DSASignature

private_key = ec.generate_private_key(ec.SECP256K1(), default_backend())
public_key = private_key.public_key()

r1=ndef.UriRecord("https://example.com")
r2=ndef.TextRecord("TEST")

stream = io.BytesIO()
records = [r1, r2, ndef.SignatureRecord("ECDSA-P256", "SHA-256")]
encoder = ndef.message_encoder(records, stream)
for _ in range(len(records) - 1): next(encoder)

signature = private_key.sign(stream.getvalue(), ec.ECDSA(hashes.SHA256()))
records[-1].signature = DSASignature.load(signature, strict=True).to_p1363()
next(encoder)
octets = stream.getvalue()

records_verified = []
records_to_verify = []
known_types = {'urn:nfc:wkt:Sig': ndef.signature.SignatureRecord}
for record in ndef.message_decoder(octets, known_types=known_types):
    if not record.type == 'urn:nfc:wkt:Sig':
        records_to_verify.append(record)
    else:
        stream_to_verify = io.BytesIO()
        encoder_to_verify = ndef.message_encoder(records_to_verify + [record], stream_to_verify)
        for _ in range(len(records_to_verify)): next(encoder_to_verify)
        try:
            public_key.verify(DSASignature.from_p1363(record.signature).dump(), stream_to_verify.getvalue(), ec.ECDSA(hashes.SHA256()))
            records_verified.extend(records_to_verify)
            records_to_verify = []
        except InvalidSignature:
            pass

records_verified = list(ndef.message_decoder(b''.join(ndef.message_encoder(records_verified))))
print(records_verified)
codecov-io commented 6 years ago

Codecov Report

Merging #4 into master will not change coverage. The diff coverage is 100%.

@@          Coverage Diff           @@
##           master     #4    +/-   ##
======================================
  Coverage     100%   100%            
======================================
  Files          10     11     +1     
  Lines        2416   2604   +188     
  Branches      386    425    +39     
======================================
+ Hits         2416   2604   +188
nickaknudson commented 6 years ago

@StephenTiedemann can you help me understand how to pass the TOXENV=docs? I am getting ImportError: No module named 'cryptography' because I use this package in some examples in the documentation. How can I include this package in the documentation?

Also I have about 5% more code coverage to add.

nehpetsde commented 6 years ago

@nickaknudson Add cryptography as a new dependency line in tox.ini (after line 22). That should get tox install it when building the the documentation. Assign me as a reviewer when the checks are passing (or when you think it is time for it).

nickaknudson commented 6 years ago

@nehpetsde I am unable to request a review from you - but can you please review?