tmeiczin / opendcp

Created digital cinemas packages (DCP)
http://www.opendcp.org
GNU General Public License v3.0
121 stars 52 forks source link

Opendcp enhacement: MXF encryption #245

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Here is a patch set doing the following (sorry if it's a little messy):

* Hooking encryption mechanisms already present to the GUI and the CLI to be 
able to generate encrypted MXFs (Audio and video types).

* Add the option on the GUI dcp tab to add a digest to the CPL as the cli 
already provides since 430-1 requires hashes on the assets in the CPL for a KDM 
to be accepted.

Still to do:

* Generate encrypted subtitles.

Notes:

* I am not entirely satisfied with the key data tests in the cli, but it's the 
best I could pull without importing extra libraries (regexp) and it's at the 
current border of my ability with C.

* I had to modify the test for key id in asdcp_intf.cpp line 502 to 505 since 
the current method of detecting the presence of a key_id ( if 
(opendcp->mxf.key_id ) should in theory work only if the array is undefined (if 
ever) since key_id here represents the address of the first element on the 
array and I'm given to understand this should rarely be null, if ever. I think 
determining if the array is full of zeroes is the best way, and I fill the 
array with zeroes only on the opendcp_mxf cli as the gui requires the presence 
of the key id (this could be changed). I didn't feel this one use merited 
creating yet another function for testing the array so I did it inline with a 
for loop.

Please let me know of any comments.

Lars.

Original issue reported on code.google.com by lars.g...@gmail.com on 6 May 2014 at 2:16

Attachments:

GoogleCodeExporter commented 9 years ago
An extra comment.

I copied the function hex2bin from asdcp's km utils, and modified it a little 
(and made it C not C++) to use in the process of converting the key and keyId 
to byte_t[16] format. I added it with three other little util functions (to 
test keys and uuids, and to shape uuids as a single undashed string to make 
testing and converting easier) in opendcp_common.c in libopendcp

Original comment by lars.g...@gmail.com on 6 May 2014 at 2:18

GoogleCodeExporter commented 9 years ago
HI, your patch is great but I cannot build because I get the following error:

asset_t has no member named 'encrypted'

asset_t has no member named 'key_id'

Thanks

Tom

Original comment by tom.a...@gmail.com on 8 May 2014 at 10:35

GoogleCodeExporter commented 9 years ago
Thanks for catching that, I missed including one patch. Here it is.

Original comment by lars.g...@gmail.com on 8 May 2014 at 6:01

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Lars,

Now the XML working fine, but need some correction in opendcp_xml.c.. I send to 
you the corrected part:

in the filed order: the first is the keyid and after the hash value,
and fixed the key id is now print the "urn:uuid"

best 

Tom

 xmlTextWriterWriteFormatElement(xml, BAD_CAST "Duration", "%d", asset.duration);
    if ( asset.encrypted ) {
        xmlTextWriterWriteFormatElement(xml, BAD_CAST "KeyId", "%s%s", "urn:uuid:", asset.key_id);
    }

    if ( opendcp->dcp.digest_flag ) {
        xmlTextWriterWriteFormatElement(xml, BAD_CAST "Hash", "%s", asset.digest);
    }

    if (asset.essence_class == ACT_PICTURE) {

Original comment by tom.a...@gmail.com on 9 May 2014 at 11:24

GoogleCodeExporter commented 9 years ago
I'll add the urn:uuid as you're right. Ordering between KeyId and Hash should 
really not matter as XML has no concept of ordering between siblings at a same 
nesting level. the only ordering xml handles is nesting depth (father/child 
relationships), and it's the only ordering that a schema/dtd checker will check.

I'll fix the uuid urn today, but as a happy note, an encrypted DCP produced 
with this opendcp code was tested on a Doremi showvault (a DCP2k4) (hooked 
against a christie projector), and worked like a charm.

Original comment by lars.g...@gmail.com on 9 May 2014 at 5:01

GoogleCodeExporter commented 9 years ago
If you test the complete DCP with the dcp_inspect (part of the cinemaslides) 
you get error message if the hash field order does not in right place. So that 
is why I change. If the KeyId is the first and the hash is the second the error 
is 0 :)

Original comment by tom.a...@gmail.com on 9 May 2014 at 6:13

GoogleCodeExporter commented 9 years ago
In the CLI the opendcp_mxf does not appear in the list (opendcp_mxf -h) the key 
and keyid command, but I see in the source...

Can you check this?

thx

Original comment by tom.a...@gmail.com on 10 May 2014 at 10:09

GoogleCodeExporter commented 9 years ago
Ordering generally does not matter in XML, but it does allow for data to be 
read serially. The schema clearly indicates elements are a sequence which means 
they must appear in the proper order.

Original comment by terrenc...@gmail.com on 10 May 2014 at 7:02

GoogleCodeExporter commented 9 years ago
A) In opendcp_mxf -h the key and key id do show in ussage for me, and 
recheckign patch #3 the lines do are added to the dcp_usage fprint set, I've 
included them before help (-h) and version (-v) since I think for readability 
those two opts should be last in the list.

B) I include here a patch with your changes, Terrence is right that in the xsd 
for ST 429-7 2006 KeyId does appear inside a sequence, before hash, so it 
wouldn't hurt (or rather, it should) be first in the order.

Original comment by lars.g...@gmail.com on 10 May 2014 at 11:33

Attachments:

GoogleCodeExporter commented 9 years ago
Working fine the key and the keyID command. Great great great great job :)
thx

Lars, do you have any beta or test patch for opendcp for XML based subtitle?

I mean --- please read this: 
https://code.google.com/p/opendcp/issues/detail?id=237

Original comment by tom.a...@gmail.com on 11 May 2014 at 2:34

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
CLI - OK

GUI build problem:

[ 67%] Generating ui_mainwindow.h
uic: Error in line 1201, column 64 : Unexpected element widget
File '/home/TEST/opendcp/gui/forms/mainwindow.ui' is not valid

Original comment by tom.a...@gmail.com on 12 May 2014 at 4:57

Attachments:

GoogleCodeExporter commented 9 years ago
I've done a checkout of origin/HEAD (the head on opendcp's repository), applied 
all 5 patch files, and it all compiles right for me. Are you patching the git 
head or 0.29? this was built on top of the most recent git commit. I've tried 
patching the qt5 branch to see if that is the problem, but too many hunks fail 
on the qt5 branch completely, so I doubt that's it.

P.s. Mr. Meiczinger, I don't see much work on the Qt5 branch right now. Will 
there be a strong push thowards the Qt5 build soon? It seems I'll have to port 
many things by hand, specially the UI for the qt5 build.

Original comment by lars.g...@gmail.com on 14 May 2014 at 12:25

GoogleCodeExporter commented 9 years ago
Hi Lars,

I'll try to build with QT4 only... I can open the unpatched original 
mainwindow.ui opendcp 0.29 version with QT4 designer, after when I patch the 
file the QT4 designer cannot open the file, because of errors.  Can you upload 
here your patched mainwindow.ui file? 

Thanks

Tom

Original comment by tom.a...@gmail.com on 14 May 2014 at 7:00

GoogleCodeExporter commented 9 years ago
I don't meant about using qt5 or qt4 to build per se, but using the qt5 branch 
in the repository. It sounds as though you're patching onto 0.29, I've not 
tested my patches on 0.29 as I did it against head in the repository, that 
means the newest code, yet unreleased as a specific version (what will become 
0.30). I did this because in case my patch is accepted and added to opendcp 
it'll be merged into a future version, so I need to work against the newest 
code.

If you're testing with 0.29, you can get a copy of the newest source with git 
(I do not find a download zip option in google projects), you can see more 
information here: https://code.google.com/p/opendcp/source/checkout

all in all you'd only need to install one program (git, easy on most distros), 
and run one command (listed there) no need to learn all of git to do this.

Original comment by lars.g...@gmail.com on 14 May 2014 at 2:36

GoogleCodeExporter commented 9 years ago
I re-patch the file mainwindow.ui file and now OK the GUI build is 100% 
complete and the encryption is 100% working under GUI. Great job. 

Original comment by tom.a...@gmail.com on 14 May 2014 at 4:59

GoogleCodeExporter commented 9 years ago
The qt5 branch was just an investigation to what needs to be done to switch to 
qt5. the change will happen, but not in the immediate future.

Original comment by terrenc...@gmail.com on 14 May 2014 at 9:42

GoogleCodeExporter commented 9 years ago
Are these changes available in a cloned repo?

Original comment by terrenc...@gmail.com on 18 May 2014 at 10:08

GoogleCodeExporter commented 9 years ago
Hi Terrence,

yes, you can find the all changes in the clone.

Tom

Original comment by tom.a...@gmail.com on 19 May 2014 at 5:04

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hello Terrence.

On my side I have only my git at home, with a few changes for myself 
(certificates), but a branch for sending upstream, if I can later or in the 
week I'll do a google project clone, or a headless in my vps, and push the 
upstream branch into it so you can pull off if you wish.

Lars.

(Pardon me, I had posted this under my Dad's account, as I was logged in from 
his machine.)

Original comment by lars.g...@gmail.com on 19 May 2014 at 9:15

GoogleCodeExporter commented 9 years ago
Lars,

My local git code repo has a few changes, so a patch won't work. If you can 
create a clone with your changes, I can then merge the files.

Original comment by terrenc...@gmail.com on 20 May 2014 at 1:45

GoogleCodeExporter commented 9 years ago
Terrence, I have here a clone, I've used the upstream branch name, it only has 
now three commits for the encryption capabilities (and digest), but in the 
future if I can contribute other patches and changes I'll push them to that 
same branch. 

You can find it here 
https://code.google.com/r/larsgold-opendcp/source/list?name=upstream 

I see you've changed many calls in your code from using strcpy to snprintf to 
fix a bug ( I need to check the bug) and I wonder why. I use strcpy once in my 
new function flatten_uuid in libopendcp/opendcp_common.c in line 767, and I use 
a couple memcpy to populate data into struct members. Should I double check on 
that?

Original comment by lars.g...@gmail.com on 22 May 2014 at 8:55

GoogleCodeExporter commented 9 years ago
The issue is that strcpy can overwrite your destination buffer resulting in 
memory corruption. There is a strncpy, but there are cases where it won't write 
a '\0' which can cause your string to contain trailing garbage. Anyway, I just 
decided to finally go through and change everything to snprintf.

Original comment by terrenc...@gmail.com on 22 May 2014 at 9:17

GoogleCodeExporter commented 9 years ago
I've changed the single added strcpy to snprintf on opendcp_common and pushed 
it onto my upstream contribution clone: 
https://code.google.com/r/larsgold-opendcp/source/list?name=upstream

Searching, I see there's still a few strcpy instances left in opendcp_common as 
it is (instances of old, not new by me). I didn't want to change them yet as 
it'd require a piece by piece debug/test and should probably be done in a 
single patch set/branch as well.

Original comment by lars.g...@gmail.com on 27 May 2014 at 3:45

GoogleCodeExporter commented 9 years ago
p.s. I've not pulled/merged your last 3 master changes in the upstream branch. 
do you want me to pull them and push them into my upstream? I could cherry-pick 
them from my own master locally which does have those merged.

Original comment by lars.g...@gmail.com on 27 May 2014 at 3:46

GoogleCodeExporter commented 9 years ago
I have a problem with the encryption under GUI...

the file encryption is looks like good the DCP xml was looks good.

the problem with the decryption... the encyrption key does not decrypt the 
encrypted file with asdcp-test... if I encrypt the reel with asdcp the 
encryption and the decryption was good. but under opendcp does not. can you 
confirm?

Original comment by tom.a...@gmail.com on 17 Oct 2014 at 5:10

GoogleCodeExporter commented 9 years ago

Original comment by terrenc...@gmail.com on 18 Dec 2014 at 6:45

GoogleCodeExporter commented 9 years ago
Thanks... where can I find the source code?

Original comment by tom.a...@gmail.com on 18 Dec 2014 at 7:10