xaptum / xtt Goto Github PK

The xtt tool package depends on the libxtt package. However, the xtt packaging doesn't require a specific version of the libxtt package, so those two versions can get mis-aligned.

In light of this xtt-spec issue resolving to remove the server ID field from the server certificate, we should reflect this change here in the implementation.

The current xtt appears to me (i.e., this is my opinion!) like an arbitrary collection of commands, not a cohesive xtt utility.

Take the six actions today:

1. genkey (ecdsa)
2. gen509cert (x509 cert)
3. wrapkeys (ASN.1 wrapped keys)
4. genrootcert (root cert)
5. genservercert (server cert)
6. infocert (any cert)

I see the following inconsistencies (not an exhaustive list):

• One of the commands specifies something about the data format (x509); the rest do not.
• The gen cert commands are split by type (x509, server, and root); the info cert command is combined.
• The genkey help string mentions the underlying crypto details (ecdsa). The wrapkeys and various cert commands do not mention the underlying crypto details.
• None of the commands reference the terminology used in our discussions, e.g., provisioning key (DAA) vs identity key (formerly ed25519, now ecdsa).
• 5 of the command names are a combined verb and noun (gen/wrap and key/cert). The 6th is two nouns, info and cert.

I think it's worth discussing an organization for these commands.

Should the "types" of cert and key be based on the usage (e.g., root, identity, server) or should they be based on structure (e.g, ecdsa, x509, ed25519).

Should the commands be organized beyond just a single action? E.g., "xtt cert show", "xtt cert generate", "xtt keys generate", "xtt keys show --public".

What additional commands are planned or desired? Getting those documented (perhaps as strawman usage strings in this issue thread) would be helpful.

EnablingWerror in the default build tends to break downstream projects on compiler upgrades that introduce new warnings. Until XTT is updated to reflect these new warnings, the downstream projects can't build.

So, we shouldn't enable Werror in the default build.

Instead, introduce a new build type with Werror. Run this type in CI, to ensure warning-free builds at the time of merges.

Related to #78

In order to help the eventual separation of xtt using tpm and xtt without a tpm, we could create a group_context that abstracts away the implementation of daa (software or tpm) that we use.

Proposed changes to xtt_client_group_context:

struct xtt_client_group_context {
    xtt_group_id gid;
    unsigned char basename[MAX_BASENAME_LENGTH];
    struct xtt_signing_context_{tpm OR software_daa}; //this is the "private key"
}

struct xtt_signing_context_tpm {
    union {
        xtt_daa_credential_lrsw lrsw;
    } cred;
    TPM_HANDLE key_handle;
    char key_password[MAX_TPM_PASSWORD_LENGTH];
    uint16_t key_password_length;
    TSS2_TCTI_CONTEXT *tcti_context;
}

struct xtt_signing_context_software_daa { //better name to come
    union {
        xtt_daa_credential_lrsw lrsw;
    } cred;
    union {
        xtt_daa_priv_key_lrsw lrsw;
    } priv_key;
}

This should decrease the number of #ifdefs we need.

Originally posted by @drbild over a year ago to the original repository.

Though this is significantly out of date, I think the principles are still relevant, and wanted to retain the discussion.

Crypto interface design
This is a brain-dump for now of my thoughts about the crypto interface design. We can refine it into a better description & discussion over time.

Implications of Design Goals for XTT Implementation
No dynamic memory allocation required.
Support arbitrary implementations of the crypto primitives.
Allow asynchronous crypto primitives for non-blocking use of HSMs.
Be as performant (nearly) as a monolithic crypto lib.
These four goals alone pose some interesting incompatibilities. Consider the following possible implementations.

Shared header w/ arbitrary linked implementation
No dynamic memory allocation implies that all struct definitions must be visible to the calling code. The shared header thus implies that all implementations must share the same struct definitions. Of course, the different possible implementations for linking will use different underlying representations for primitives like keys. Thus, the representations must be translated at the interface boundary for every call. And that is not performant.

The translation could be done (at the cost of some cycles) were the data types always representing the same concept, e.g., different representations of an ed25519 key. The interface would define a common serialization format to translate to and from. But consider an implementation backed by an HSM. The private key would not actually be a key, but instead some opaque identifier for the key inside the HSM. Finding a clean, common representation seems difficult.

An alternative is for the interface to take pointers to opaque structs defined by the implementation. But this requires dynamic memory allocation, as those opaque structs would have to be allocated by the linked implementation code, where the full definition is known.

Shared header for functions, implementation-specific header for data types
This approach would solve the static allocation issue. The XTT library would have to be compiled against a specific implementation defining the structs. (The shared header defining the methods would just have forward declarations.)

I'm leaning towards this approach currently. My general feeling is that the crypto implementation should be statically linked to the XTT library anyway (not like openSSL with a shared lib for the crypto). So this keep everything clean.

Polymorphism - the Compile/Runtime, Global/Local matrix
The "arbitrary implementation" design goal is still ill-defined. Should the implementation be chosen at compile time or be configurable at run-time? Should it be common to the whole program or changeable for each XTT "session"?

The two approaches in the prior section are in the (compile-time, global) quadrant of the design space.

Supporting runtime selection or local per-session selection of primitives seems to require either dynamic memory allocation or a shared/translatable representation of the data structures, which are non starters.

Proposed Approach
XTT provides a shared header that forward declares the structs and methods used to access crypto primitives. The user is responsible for compiling XTT against an implementation that defines those structs and methods.

We provide two (or more) reference implementations that delegate to different libraries (say mbedTLS and wolfSSL).

These two implementations are in the (compile-time, global) quadrant above. They are applied globally to all XTT sessions within the program.

Any user is free to provide their own implementation instead if they don't like our chosen point in the design space. Such an implementation could

mix and match from various backends
support linking-based selection of the backend by implementing a translation layer (like the first approach listed above)
support runtime polymorphism at the cost of doing dynamic memory allocation from the library.
The point is that our interface doesn't preclude users from doing something more.

The one concession made to support this flexibility is the method for function dispatch. Rather than access the methods forward declared in the shared header directly by name, they are accessed via a vtable in the XTT context objects.

Implementation
I'll do a mock-up of this approach from the existing crypto lib to see how it looks and feels.

The recent changes to the read_nvram design requires the recent v0.6.0 release of xaptum-tpm. However, CMakeLists.txt still allows v0.5.0 or above.

The tool currently has some minor inconsistencies:

• The default for the serverhost parameter to runclient isn't specified, though localhost is the default
• The default for the requestid parameter to runclient is incorrect, I think. The default is to not request a specific ID (i.e. to use the null ID type), right?
• The default assignedid output name should have a .txt extension, not .bin, since it's a textual encoding of the ID
• The help message for the rcert parameter to genrootcert is incorrect. It says this is the root's public key output location, but it's the root's certificate output location
• The help messages for the rpriv and rpub parameters to genrootcert are incorrect. They say these are output locations, but they're input locations
• The help message for the serverpub parameter to genservercert is incorrect. It says this is the private key output location, but it's the public key input location.
• The genservercert parameter sid should probably generate a random ID, like genrootcert does, rather than requiring the server_id.bin file. This is currently the only pre-existing file (other than DAA-related things) that a user would have to create themself, so it would be nice to make a sane default like this so everything could be generated anew.

The xtt tool currently prints error messages to stdout (printf("...")) instead of stderr (fprintf(stderr, "...")). This doesn't play well with piping or redirecting the output.

E.g.,

xtt infocert my-missing-cert.bin > cert_info.txt

would create a file cert_info.txt containing the error message, instead of the cert info. The user won't realize there was problem.

Using stderr, the user will see the error message on the console and the cert_info.txt file will be empty.

A TPM should have all the information a client needs to run a handshake. And, in fact, the tool does read the root ID and root public key from the TPM. But, it also fails if a root_cert.bin file isn't available. This check should only be made when not using a TPM.

This cast in ecdaa_wrapper (lines 73 and 123) is invalid and generates warnings (errors due to -Werror) in GCC 8:

(ecdaa_rand_func)xtt_crypto_get_random;

ecdaa_rand_func returns a void but xtt_crypto_get_random returns an int. These are not compatible. On architectures that place the return value on the stack, a caller of ecdaa_rand_func will expect no return value, and thus not pop off the int placed by xtt_crypto_get_random, clobbering the stack. (x86, ARM, etc, return ints in a register, so this issue doesn't manifest on the platforms we've used.)

UTC time in x509 certificates is formatted as: YYMMDDHHMMSSZ where if YY is >50 the year is interpreted at 19YY. So after 2049, this format will no longer hold. However, if we change our certificates to use Generalized Time (YYYYMMDDHHMMSSZ) we will be able to avoid any issues.

In the event of an error in the tool that causes the operation to fail, we still return 0. This is because the main function in xtt.c always returns 0 (in C99, the default return value for main is 0, and we never explicitly return).

So, the first fix has to be to update the final switch statement in xtt.c to return 0 only on success, and return 1 for errors.

However, it also looks like in some places in the tool, we use return values from util_errors.h, while in other places we use 0 or -1 explicitly (e.g. in run_client if we can't connect to the server). This is a problem because the switch in xtt.c only looks for specific return values, with no default. But, even if we add a default there that returns 1, we still should use appropriate return values in the first place, to be able to give good stderr feedback.

xtt_save_to_file is used by enftun to save the certificate and private key. The private key should not be world-readable, which is the default when writing a file (on many systems).

In util/file_io.h/c:

• Add the file permissions mask as a third argument to xtt_save_to_file
• Add wrapper function xtt_save_key_to_file with mask 0600
• Add wrapper function xtt_save_cert_to_file with mask 0644.

While attempting to run an XTT handshake outside of the tool and XTT's C library, we found that handles have not been standardized for the library.

While implementing support of XTT for enftun, we realized that read_nvram and other functions may be able to move into the xtt library.

read_nvram currently creates a TCTI context each time it reads from the TPM. Is it possible that we could create a function to do that once, and then use that context/credentials to read out the information?

We should make the creation/sending of error messages a clearer part of the implicit state machine. This means, for example, adding context states for sending error messages, and updating handle_io.

Now that we have a tool, having the example/ directory is only useful for the client and server example data it provides. We should keep at least some of the data that the tool cannot yet generate or move the example data into the tool itself.

We should also update the README to reflect these changes a show a user how to run a client and server from the tool.

An in-kernel TPM resource manager was merged in the 4.12 kernel. It exposes a # /dev/tpmrmX device for each TPM that can be concurrently accessed by multiple programs. The kernel takes care of swapping the context in and out of the TPM as needed.

The various utilities and examples (especially xtt_client) should updated to take the device file name as a command line argument.

I'd suggest something like:
xtt_client --tcti device|socket --address,a IP_ADDRESS --port,p PORT --device,d FILE

The --tcti value could be inferred from the provided arguments, defaulting to --device /dev/tpmrm0 (--device /dev/tpm0 if /dev/tpmrm0 doesn't exist) if no arguments are provided.

A plain --tcti socket (--tcti device) with no other args specified can use default values for address and port (file).

While we're at it, use getopt or getopt_long to parse the command line args.

One of the main results of an XTT handshake is the creation of a client's identity ("longterm") keypair (such that the server links the public key with the identity assigned to the client). The private key of this pair is very sensitive (until this client runs another XTT handshake, this private key will be used to prove the client's identity during all subsequent ENF sessions).

Currently, this project saves this identity keypair to disk, in plaintext. To improve the security of this valuable private key, we should generate it on a TPM (secure hardware) such that it never leaves the TPM (and, in fact, is defined to be incapable of leaving the TPM; cf. the TPM flags that can be set during the key's creation).

The xaptum-tpm project, which this project links to transitively via its libecdaa-tpm dependency (when the USE_TPM CMake option is on, of course), already supports ECDSA (using only secp256r1 or p256 or whatever you want to call that curve) key-generation and signing (cf. the test/tss2_sys_sign-secp256-test.c file for example usage).

A rough outline of this work could be (all the new code should be wrapped in USE_TPM ifdefs):

• Add xtt_crypto_create_ecdsap256TPM_key_pair and xtt_crypto_sign_ecdsap256TPM declarations in include/xtt/crypto_wrapper.h
• Create a src/xaptum_tpm_wrapper.c file
  • Implement xtt_crypto_create_ecdsap256TPM_key_pair and xtt_crypto_sign_ecdsap256TPM in this file, calling the xaptum-tpm functions
  • The keypair-generation function will need to make sure it copies the public key out of the TPM structure and into the XTT buffer in the correct format (for later use during the handshake)
• Add a longterm_sign_ecdsap256TPM function in src/internal/crypto_utils.c
  • This simply calls xtt_crypto_sign_ecdsap256TPM (just like sign_lrswTPM is a simple wrapper of xtt_daa_sign_lrswTPM)
• Add a xtt_initialize_client_handshake_context_TPM function to context.[h,c]
  • This will be almost the exact same as xtt_initialize_client_handshake_context, except it will use xtt_crypto_create_ecdsap256TPM_key_pair rather than xtt_crypto_create_ecdsap256_key_pair.
  • There's certainly a cleaner way to support these two functionalities, but this is the way the TPM-vs-no-TPM is handled for xtt_initialize_client_group_context_lrswTPM, so it seems best to just stick to that for now

The above outline is intended to be a minimal effort plan, and as such doesn't help with the existing poorly-conceived API in this project. While we need to re-architect the crypto design, I think this is important-enough functionality that adding support for it before delving into redesign is a good idea.

Questions:

• How to handle key handle?
  • Should this be a Xaptum-wide-constant, defined in xaptum-tpm (like the NVRAM indices, etc.)?
  • Or, should it be user-configurable within this project?

@drbild I'd be interested in your thoughts on this

In the interest of consolidating files that a server or client needs to provide, I propose that we combine the basename and DAA GPK into a single file for input to the tool. This file will conceptually represent the DAA group.

I also propose that this file, by default, be named by the GroupID of the group (e.g. 1234.bin for gid 1234). A new command line flag --gid (for example) could be used to override this default.

This should also be done in our up-coming tool in the ecdaa project.

tools/client.c does not build if USE_TPM=off is specified. Some of the TPM-specific code is not ifdefed out.

When populating the Common Name in the x509 certificate using the longterm identity provisioned during an XTT handshake, convert the 16 bytes of the identity into an IPv6 string representation. The Common Name is typically a DNS hostname or an IP address, so this conforms to expectation, and makes reading easier.

This could be done by updating the xtt_identity_to_string function in crypto_types.c to use the POSIX function inet_ntop. However, that will likely result in a non-constant-length string representation, which would be problematic for our certificate creation function (since we use static lengths). Instead, we could just use sprintf and convert the bytes to the hex form and insert : every 4 characters. This would create a constant-length string, but wouldn't be a canonical IPv6 address.

XTT currently has 3 different types of certificates that XTT produces (root, server, and x509). Right now a xtt_root_certificate is the concatenation of a root_id and root_public_key. We could give the root and server certificate the same underlying structure(that of the current server cert) where the root certificate would be signed by it's private key.

It's also possible that "root_certificate" is a misnomer, and should be changed to something like root_public_info. Since it is really just the public information about a root packaged together.

If we want to change the curve again (or support the use of multiple curves), rather than users to having to supply, a possibly incorrect curve, the root_certificate could give us a curve type.

On a slightly different note, the root certificate could be changed to match the TLS root certificate that is already on the TPM. The existing cert is PEM encoded, and we would need to parse the x509 to get the root's id and public key, but this way we don't have multiple types of root certificates

Cf. this issue in the xdaa-issuer repo for context.

In the x509-wrapping, we should set the notBefore and notAfter as explained in the above linked issue.

For the XTT certificate generation we should simply always set the expiry to "99990000" for now. Soon, though, this field should simply be removed from the XTT certificate (this will need to be updated in the xtt spec).

The latest firmware versions (7.63 and beyond) for the Infineon SLB9670 TPM change the way DAA signatures are done, breaking XTT.

This problem is tracked as xaptum/ecdaa#112

xtt will need to be updated to the new ecdaa, after the issue is fixed.

Our usage of Ed25519 for the asymmetric signature algorithm causes two issues in our ecosystem (one short-term, one more serious):

• Short-term, TLS-library support for Ed25519 is scant, which makes using the "longterm" key communicated during an XTT handshake difficult to use in subsequent TLS sessions (while we're using TLS as the session layer)
• More seriously, since the signature key that's generated is used for all subsequent authentication, we really need to protect it better than simply leaving in the file system. We're already using a TPM in our ecosystem, so we should generate it there. The problem is, TPMs don't support Ed25519

So, we need to use a different, more-widely-available signature scheme. Due to our commitment to low processing- and network-overheads for the devices, ECC is preferable to RSA, which means we choose ECDSA. For 128-bit security level, using a commonly-available curve (including available in TCG's "Mobile Common Profile"), we choose the NIST secp256r aka p256 curve.

The xtt_save_to_file function does not use the O_TRUNC flag when opening a file. Consequently, if the new contents are shorter than the old contents, the tail of the old contents remains at the end of the file.

We should adapt this PR and this PR in xaptum-tpm, to make sure we're testing the building of static libs and installation in travis.

Add the static analysis tools cppcheck and coverity to be run by Travis-ci. The ecdaa project is an example of how to do this

Adding valgrind as well may be useful

Function generate_server_certificate_ed25519 should be named xtt_generate_server_certificate_ed25519.

We should expand the functionality of the build_x509_skeleton function (and the various wrappers of it) to allow non-self-signed certificates to be created.

This should only require:

• Make build_x509_skeleton take two common_name_as_string parameters, one for the subject of the certificate and one for the signer (see the build_tbs_certificate function for where those two uses occur).
• xtt_x509_from_ecdsap256_keypair can remain the same, but renaming it and its parameters might make things clearer (e.g. the public key and the private key aren't necessarily linked). Or perhaps keeping (with just one common name input, to be used for self-signed certs) it but making a clone, that's renamed and has differently-named parameters.
  • A similar renaming/cloning for the tool

The various usage strings in the xtt tool subcommands are not consistent:

• "Usage: xtt a b c"
• "Usage: xtt-tool a b c"
• "Usage: %s a b c"%argv[0]

We should standardize on one.

"Usage: %s a b c"%argv[0]" (at the top-level) or "Usage: %s %s a b c"%(argv[0], argv[1]) (within commands) seems correct to me.

Using argv ensures that usage string mirrors that name that the user typed.

We see this sometimes (mostly when incorrect server credentials are used) in our use of the xtt-cpp wrapper in our server, when using the xtt_client example client.

It's not clear if this is due to the example client sending an error message incorrectly, or due to the server mis-parsing the error message.

This is most likely connected to Issue #33

Explore self-hosting libsodium, the way we do for the TPM simulator

There are projects that require XTT > 0.9.2, but in order for a user to locally install xtt 0.9.2 or later they would need to change the number in the CMakeLists.txt themselves or install xtt-dev using apt-get install which is not possible for Mac users.

In relation to discussion on #78:

In the client code of the handshake, we have initialize_{daa, certs, tcti}. Each of these must read in multiple pieces of information either from files(software DAA) or from the TPM's nvram. Could we create three different "contexts" or "certificates", so that we only need to read in 1 piece of data and then parse those? Those three being a group_context, a TLS root_cert and a xtt root_cert.

Similar to issue 133 in the ecdaa repository xaptum/ecdaa#133 , I had to change includes from
<amcl/...>
to
<amcl/include/...>

Ctest failed 3 tests:

4: Test command: /home/pi/git/xtt/build/testBin/integration_test "3"
4: Test timeout computed to be: 1500
4: Suite Spec: 3
4: Passes all of client step 1
4: Condition 'server->rc == XTT_RETURN_WANT_WRITE' failed
4: 	in file: '/home/pi/git/xtt/test/integration_test.c'
4: 	in function: 'server_step_one'
4: 	at line: 79
4: exiting
4/8 Test #4: integration_test_3 ...............***Failed    0.03 sec
test 5
    Start 5: integration_test_4

5: Test command: /home/pi/git/xtt/build/testBin/integration_test "4"
5: Test timeout computed to be: 1500
5: Suite Spec: 4
5: Passes all of client step 1
5: Condition 'server->rc == XTT_RETURN_WANT_WRITE' failed
5: 	in file: '/home/pi/git/xtt/test/integration_test.c'
5: 	in function: 'server_step_one'
5: 	at line: 79
5: exiting
5/8 Test #5: integration_test_4 ...............***Failed    0.03 sec

and

    Start 8: cpp_test

8: Test command: /home/pi/git/xtt/build/testBin/cpp_test
8: Test timeout computed to be: 1500
8: ==4187==ASan runtime does not come first in initial library list; you should either link runtime to your application or manually preload it with LD_PRELOAD.
8/8 Test #8: cpp_test .........................***Failed    0.03 sec