Added command in oesign to dump oeinfo and signature information #564 #941

berinpaul · 2018-10-17T12:31:09Z

Dump feature added to oesign tool:

Usage: ./oesign <command> [options]

Commands:
sign - Sign the specified enclave.
dump - Print out the Open Enclave metadata for the specified enclave.

For help with a specific command, enter ./oesign <command> -?

CodeMonkeyLeet

We need to design this out a little more before we take this in terms of what groups of data we would like to dump since some of that is changing.

@berinpaul, in the meantime, I would like you to refactor how the command handling is implemented, per the comment for oesign/main.c

CodeMonkeyLeet · 2018-10-24T02:30:21Z

tools/oesign/oedump.c

+    printf("\n");
+
+    /* Dump the entry point */
+    dump_entry_point(&elf);


@mikbras Is this valuable to keep? We use _start now, and we don't expect there to be an explicit flag on the build line to enable this, so I'm not sure if dumping this is useful anymore. This will also end up being something different in PE so I'd like to remove things that have to be supported in different ways on different systems.

CodeMonkeyLeet · 2018-10-24T02:31:50Z

tools/oesign/oedump.c

+    dump_enclave_properties(&props);
+
+    /* Dump the ECALL section */
+    dump_ecall_section(&elf);


@anakrish @mikbras We still have the .ecall section, but it's no longer comprehensive given the EDL changes. Do we want to keep this functionality?

CodeMonkeyLeet · 2018-10-24T02:41:20Z

tools/oesign/main.c

@@ -523,7 +524,17 @@ static const char _usage[] =
    "        -----BEGIN RSA PRIVATE KEY-----\n"
    "\n"
    "    The resulting image is written to <EnclaveImage>.signed.so.\n"
-    "\n";
+    "\n"
+    "	   		---- OR-----\n"


This is an awkward way to express a multifunction CLI tool … we should probably refactor the parameter handling so that it is consolidated across the different sub-functions. This is especially true because as part of supported digest signing for #525, we will also need to add new commands to generate the digest and to inject a signed digest into an enclave sigstruct. To accommodate that I think we will need to standardize the usage format to take an explicit command:

Usage: oesign { sign | hash | info | verify } … <op specific parameters>

CodeMonkeyLeet

See comments inline … also, the original ask was to accommodate non-positional parameters as well, did you consider implementing that as well? With 3 or more parameters, it will be more difficult for the dev to use correctly based on positional parameters.

CodeMonkeyLeet · 2018-11-20T03:15:31Z

tools/oesign/main.c

-    "Usage: %s EnclaveImage ConfigFile KeyFile\n"
+static const char _usage_gen[] =
+    "Usage: %s <command> [options]\n"
+


Remove extra whitespace between lines, they're not preserved on print when concatenated anyway.

CodeMonkeyLeet · 2018-11-20T03:22:46Z

tools/oesign/main.c

+
+    switch (argc)
+    {
+        case 1:


This is not a particularly robust way of enumerating all the cases, consider parsing as a state machine instead of a switch-case predicated on number of arguments, because we may have cases with optional arguments, which this format cannot accommodate (or will be split up across number of arguments as opposed to functional grouping as in sign -? vs. sign … in this case.

CodeMonkeyLeet · 2018-11-20T03:22:50Z

tools/oesign/main.c

-    keyfile = argv[3];
+    /* Collect arguments for signing*/
+    enclave = argv[2];
+    conffile = argv[3];


Please encapsulate the signing-specific codepath into a separate function instead of letting it be a fall-through from arg_handler

CodeMonkeyLeet

Minor correction to the help documentation

CodeMonkeyLeet · 2018-12-01T01:51:15Z

tools/oesign/main.c

+    "For help with a specific command, enter \"%s <command> -?\"\n";
+
+static const char _usage_sign[] =
+    "Usage: %s enclave_image sign config_file key_file\n"


Incorrect ordering, should be:

"Usage: %s sign enclave_image sign enclave_image config_file key_file\n"

CodeMonkeyLeet · 2018-12-03T23:30:05Z

bors try

oe-bors · 2018-12-03T23:42:32Z

try

Build succeeded

continuous-integration/jenkins/branch

CodeMonkeyLeet · 2018-12-03T23:49:45Z

bors r+

941: Added command in oesign to dump oeinfo and signature information #564 r=CodeMonkeyLeet a=berinpaul Dump feature added to oesign tool: Usage: `./oesign <command> [options]` Commands: *sign* - Sign the specified enclave. *dump* - Print out the Open Enclave metadata for the specified enclave. For help with a specific command, enter `./oesign <command> -?` Co-authored-by: berinpaul <berin.paul@vvdntech.in>

oe-bors · 2018-12-04T00:02:28Z

Build succeeded

continuous-integration/jenkins/branch

berinpaul requested a review from CodeMonkeyLeet October 17, 2018 12:31

CodeMonkeyLeet added the functionality Issue describes an enhancement or addition of functionality to Open Enclave SDK label Oct 18, 2018

CodeMonkeyLeet suggested changes Oct 24, 2018

View reviewed changes

berinpaul force-pushed the feature/oedump_into_oesign branch 2 times, most recently from 31b98a7 to e946097 Compare November 15, 2018 13:19

CodeMonkeyLeet suggested changes Nov 20, 2018

View reviewed changes

berinpaul force-pushed the feature/oedump_into_oesign branch 4 times, most recently from 6418c00 to 68f4ee0 Compare November 22, 2018 09:25

CodeMonkeyLeet reviewed Dec 1, 2018

View reviewed changes

berinpaul added 2 commits December 3, 2018 10:23

Added command in oesign to dump oeinfo and signature information #564

1ddb866

Usage correction

327f225

berinpaul force-pushed the feature/oedump_into_oesign branch from 68f4ee0 to 327f225 Compare December 3, 2018 05:03

CodeMonkeyLeet approved these changes Dec 3, 2018

View reviewed changes

oe-bors bot pushed a commit that referenced this pull request Dec 3, 2018

Try #941:

857c572

oe-bors bot merged commit 327f225 into master Dec 4, 2018

oe-bors bot deleted the feature/oedump_into_oesign branch December 4, 2018 00:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added command in oesign to dump oeinfo and signature information #564 #941

Added command in oesign to dump oeinfo and signature information #564 #941

berinpaul commented Oct 17, 2018 •

edited

CodeMonkeyLeet left a comment

CodeMonkeyLeet Oct 24, 2018

CodeMonkeyLeet Oct 24, 2018

CodeMonkeyLeet Oct 24, 2018

CodeMonkeyLeet left a comment

CodeMonkeyLeet Nov 20, 2018

CodeMonkeyLeet Nov 20, 2018

CodeMonkeyLeet Nov 20, 2018

CodeMonkeyLeet left a comment

CodeMonkeyLeet Dec 1, 2018

berinpaul Dec 3, 2018

CodeMonkeyLeet commented Dec 3, 2018

oe-bors bot commented Dec 3, 2018

CodeMonkeyLeet commented Dec 3, 2018

oe-bors bot commented Dec 4, 2018

Added command in oesign to dump oeinfo and signature information #564 #941

Added command in oesign to dump oeinfo and signature information #564 #941

Conversation

berinpaul commented Oct 17, 2018 • edited

CodeMonkeyLeet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodeMonkeyLeet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodeMonkeyLeet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CodeMonkeyLeet commented Dec 3, 2018

oe-bors bot commented Dec 3, 2018

try

Build succeeded

CodeMonkeyLeet commented Dec 3, 2018

oe-bors bot commented Dec 4, 2018

Build succeeded

berinpaul commented Oct 17, 2018 •

edited