From 84ffe25dbe314128b80ee93eba818e061d53023f Mon Sep 17 00:00:00 2001 From: Brian Telfer Date: Mon, 16 Jul 2018 17:09:28 -0700 Subject: [PATCH] Tcps cbor update, x509 CSR OID extension, and CyReP bug fixes (#5) * Fixed key usage for device certificates. * Changes to allow TCPS Agent to build. Added application datasheet to BarT * Add support to embed custom OIDs into CSR and generate a cert out of CSR * Fix emulator release * Switch to memcpy as = was crashing due to memory alignment in TAs. * Removed references to internal functionality not exposed from tinycbor, updated to latest reference. --- .gitmodules | 1 + CyReP/RiotDerEnc.c | 2 +- CyReP/RiotEcc.c | 5 +- CyReP/RiotKdf.c | 1 + CyReP/RiotX509Bldr.c | 115 +++++++++++++++++++++---- CyReP/cyrep/RiotX509Bldr.h | 27 +++++- CyReP/tcps/cborhelper.c | 94 ++++++++++++++++---- CyReP/tcps/cborhelper.h | 6 +- Emulator/RIoTEmulator.cpp | 13 ++- Emulator/RIoTEmulator.vcxproj | 1 - Emulator/RIoTEmulator.vcxproj.filters | 3 - External/tinycbor | 2 +- Sample/Barnacle/Tool/.gitignore | 5 +- Sample/Barnacle/Tool/BarT/BarT.vcxproj | 4 +- 14 files changed, 228 insertions(+), 51 deletions(-) diff --git a/.gitmodules b/.gitmodules index 72e9f59..6eb6b86 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,4 @@ [submodule "External/tinycbor"] path = External/tinycbor url = https://github.com/intel/tinycbor.git + branch = dev diff --git a/CyReP/RiotDerEnc.c b/CyReP/RiotDerEnc.c index 59baaf7..4baf2c7 100644 --- a/CyReP/RiotDerEnc.c +++ b/CyReP/RiotDerEnc.c @@ -409,7 +409,6 @@ DERAddOctetString( return -1; } - int DERStartSequenceOrSet( DERBuilderContext *Context, @@ -446,6 +445,7 @@ DERStartExplicit( Error: return -1; } + int DERStartEnvelopingOctetString( DERBuilderContext *Context diff --git a/CyReP/RiotEcc.c b/CyReP/RiotEcc.c index 60d36a7..d7dc552 100644 --- a/CyReP/RiotEcc.c +++ b/CyReP/RiotEcc.c @@ -1023,7 +1023,10 @@ big_divide(bigval_t *tgt, bigval_t const *num, bigval_t const *den, { bigval_t u, v, x1, x2; - u = *den; + // using memcpy instead of dereference with assignment, as the latter + // is crashing OPTEE TAs (issues with alignment) + // u = *den; + memcpy(&u, den, sizeof(u)); v = *modulus; x1 = *num; x2 = big_zero; diff --git a/CyReP/RiotKdf.c b/CyReP/RiotKdf.c index 7d2110d..d0a2e4b 100644 --- a/CyReP/RiotKdf.c +++ b/CyReP/RiotKdf.c @@ -56,6 +56,7 @@ size_t RIOT_KDF_FIXED( { size_t total = (((label) ? labelSize : 0) + ((context) ? contextSize : 0) + 5); + fixedSize; // unreferenced parameter assert(fixedSize >= total); if (label) { diff --git a/CyReP/RiotX509Bldr.c b/CyReP/RiotX509Bldr.c index 764ea8b..6faa141 100644 --- a/CyReP/RiotX509Bldr.c +++ b/CyReP/RiotX509Bldr.c @@ -23,6 +23,7 @@ static int tcpsOID[] = { 2,23,133,5,4,2,-1 }; static int ecdsaWithSHA256OID[] = { 1,2,840,10045,4,3,2,-1 }; static int ecPublicKeyOID[] = { 1,2,840,10045, 2,1,-1 }; static int prime256v1OID[] = { 1,2,840,10045, 3,1,7,-1 }; +static int extensionRequestOID[] = { 1,2,840,113549,1,9,14,-1 }; static int keyUsageOID[] = { 2,5,29,15,-1 }; static int extKeyUsageOID[] = { 2,5,29,37,-1 }; //static int subjectAltNameOID[] = { 2,5,29,17,-1 }; @@ -34,7 +35,7 @@ static int countryNameOID[] = { 2,5,4,6,-1 }; static int orgNameOID[] = { 2,5,4,10,-1 }; static int basicConstraintsOID[] = { 2,5,29,19,-1 }; static int subjectKeyIdentifierOID[] = { 2,5,29,14,-1 }; -static int authorityKeyIdentifierOID[] = { 2,5,29,1,-1 }; +static int authorityKeyIdentifierOID[] = { 2,5,29,35,-1 }; #ifdef __GNUC__ #pragma GCC diagnostic push @@ -56,7 +57,9 @@ X509AddExtensions( uint32_t FwidLen, uint8_t *Tcps, uint32_t TcpsLen, - int32_t PathLen + int32_t PathLen, + const uint8_t* ExtensionBuffer, + uint32_t ExtensionBufferSize ) // Create the RIoT extensions. The RIoT subject altName + extended key usage. { @@ -87,7 +90,7 @@ X509AddExtensions( CHK( DERAddBoolean(Tbs, true)); CHK( DERStartEnvelopingOctetString(Tbs)); CHK( DERStartSequenceOrSet(Tbs, true)); - if(PathLen > 0) + if (PathLen > 0) { CHK( DERAddBoolean(Tbs, true)); CHK( DERAddInteger(Tbs, PathLen)); @@ -122,7 +125,7 @@ X509AddExtensions( CHK( DERPopNesting(Tbs)); CHK( DERPopNesting(Tbs)); } - if (TcpsLen == 0) + if (TcpsLen == 0 && ExtensionBufferSize == 0) // riotOID { CHK( DERStartSequenceOrSet(Tbs, true)); CHK( DERAddOID(Tbs, riotOID)); @@ -144,7 +147,7 @@ X509AddExtensions( CHK( DERPopNesting(Tbs)); CHK( DERPopNesting(Tbs)); } - else + else if (ExtensionBufferSize == 0) // tcpsOID { CHK( DERStartSequenceOrSet(Tbs, true)); CHK( DERAddOID(Tbs, tcpsOID)); @@ -153,6 +156,15 @@ X509AddExtensions( CHK( DERPopNesting(Tbs)); CHK( DERPopNesting(Tbs)); } + else // OID buffer + { + if (Tbs->Length - Tbs->Position < ExtensionBufferSize) + { + goto Error; + } + memcpy(Tbs->Buffer + Tbs->Position, ExtensionBuffer, ExtensionBufferSize); + Tbs->Position += ExtensionBufferSize; + } CHK( DERPopNesting(Tbs)); CHK(DERPopNesting(Tbs)); @@ -177,7 +189,7 @@ X509AddX501Name( CHK( DERAddUTF8String(Context, CommonName)); CHK( DERPopNesting(Context)); CHK( DERPopNesting(Context)); - if(CountryName != NULL) + if (CountryName != NULL) { CHK( DERStartSequenceOrSet(Context, false)); CHK( DERStartSequenceOrSet(Context, true)); @@ -186,7 +198,7 @@ X509AddX501Name( CHK( DERPopNesting(Context)); CHK( DERPopNesting(Context)); } - if(OrgName != NULL) + if (OrgName != NULL) { CHK( DERStartSequenceOrSet(Context, false)); CHK( DERStartSequenceOrSet(Context, true)); @@ -263,7 +275,7 @@ X509GetDeviceCertTBS( CHK( DERPopNesting(Tbs)); CHK( DERPopNesting(Tbs)); CHK( DERPopNesting(Tbs)); - if(PathLength >= 0) + if (PathLength >= 0) { CHK( DERStartSequenceOrSet(Tbs, true)); CHK( DERAddOID(Tbs, basicConstraintsOID)); @@ -283,7 +295,7 @@ X509GetDeviceCertTBS( CHK( DERPopNesting(Tbs)); CHK( DERPopNesting(Tbs)); } - if(PathLength > 0) + if (PathLength > 0) { CHK( DERStartSequenceOrSet(Tbs, true)); CHK( DERAddOID(Tbs, keyUsageOID)); @@ -419,7 +431,7 @@ X509GetAliasCertTBS( CHK( DERAddBitString(Tbs, encBuffer, encBufferLen)); CHK( DERPopNesting(Tbs)); RiotCrypt_ExportEccPub(DevIdKeyPub, encBuffer, &encBufferLen); - CHK( X509AddExtensions(Tbs, encBuffer, encBufferLen, subjectKeyId, sizeof(subjectKeyId), Fwid, FwidLen, Tcps, TcpsLen, PathLen)); + CHK( X509AddExtensions(Tbs, encBuffer, encBufferLen, subjectKeyId, sizeof(subjectKeyId), Fwid, FwidLen, Tcps, TcpsLen, PathLen, NULL, 0)); CHK(DERPopNesting(Tbs)); ASRT(DERGetNestingDepth(Tbs) == 0); @@ -429,6 +441,57 @@ X509GetAliasCertTBS( return -1; } +int +X509GetCSRCertTBS( + DERBuilderContext *Tbs, + RIOT_X509_TBS_DATA *TbsData, + const RIOT_ECC_PUBLIC *CsrKeyPub, + const RIOT_ECC_PUBLIC *AuthorityKeyPub, + int32_t PathLen, + const uint8_t *SubjectKeyDerBuffer, + uint32_t SubjectKeyDerBufferSize, + const uint8_t *ExtensionDerBuffer, + uint32_t ExtensionDerBufferSize +) +{ + uint8_t encBuffer[65]; + uint32_t encBufferLen; + uint8_t subjectKeyId[RIOT_DIGEST_LENGTH]; + + CHK(DERStartSequenceOrSet(Tbs, true)); + CHK( DERAddShortExplicitInteger(Tbs, 2)); + CHK( DERAddIntegerFromArray(Tbs, TbsData->SerialNum, RIOT_X509_SNUM_LEN)); + CHK( DERStartSequenceOrSet(Tbs, true)); + CHK( DERAddOID(Tbs, ecdsaWithSHA256OID)); + CHK( DERPopNesting(Tbs)); + CHK( X509AddX501Name(Tbs, TbsData->IssuerCommon, TbsData->IssuerOrg, TbsData->IssuerCountry)); + CHK( DERStartSequenceOrSet(Tbs, true)); + CHK( DERAddUTCTime(Tbs, TbsData->ValidFrom)); + CHK( DERAddUTCTime(Tbs, TbsData->ValidTo)); + CHK( DERPopNesting(Tbs)); + + if (Tbs->Length - Tbs->Position < SubjectKeyDerBufferSize) + { + goto Error; + } + + memcpy(Tbs->Buffer + Tbs->Position, SubjectKeyDerBuffer, SubjectKeyDerBufferSize); + Tbs->Position += SubjectKeyDerBufferSize; + + RiotCrypt_ExportEccPub(CsrKeyPub, encBuffer, &encBufferLen); + RiotCrypt_Hash(subjectKeyId, sizeof(subjectKeyId), encBuffer, encBufferLen); + + RiotCrypt_ExportEccPub(AuthorityKeyPub, encBuffer, &encBufferLen); + CHK( X509AddExtensions(Tbs, encBuffer, encBufferLen, subjectKeyId, sizeof(subjectKeyId), NULL, 0, NULL, 0, PathLen, ExtensionDerBuffer, ExtensionDerBufferSize)); + CHK(DERPopNesting(Tbs)); + + ASRT(DERGetNestingDepth(Tbs) == 0); + return 0; + +Error: + return -1; +} + int X509MakeAliasCert( DERBuilderContext *AliasCert, @@ -518,10 +581,12 @@ X509GetDEREcc( } int -X509GetDERCsrTbs( +X509GetDERCsrTBS( DERBuilderContext *Context, RIOT_X509_TBS_DATA *TbsData, - const RIOT_ECC_PUBLIC *DeviceIDPub + const RIOT_ECC_PUBLIC *DeviceIDPub, + RIOT_X509_OID *OidExtensions, + const size_t OidExtensionsCount ) { uint8_t encBuffer[65]; @@ -538,8 +603,28 @@ X509GetDERCsrTbs( RiotCrypt_ExportEccPub(DeviceIDPub, encBuffer, &encBufferLen); CHK( DERAddBitString(Context, encBuffer, encBufferLen)); CHK( DERPopNesting(Context)); - CHK(DERStartExplicit(Context,0)); - CHK(DERPopNesting(Context)); + + CHK( DERStartExplicit(Context, 0)); + CHK( DERStartSequenceOrSet(Context, true)); + CHK( DERAddOID(Context, extensionRequestOID)); + CHK( DERStartSequenceOrSet(Context, false)); + CHK( DERStartSequenceOrSet(Context, true)); + + for (size_t i = 0; i < OidExtensionsCount; i++) + { + CHK( DERStartSequenceOrSet(Context, true)); + CHK( DERAddOID(Context, OidExtensions[i].Oid)); + CHK( DERStartEnvelopingOctetString(Context)); + CHK( DERAddOctetString(Context, OidExtensions[i].DerBuffer, OidExtensions[i].DerBufferSize)); + CHK( DERPopNesting(Context)); + CHK( DERPopNesting(Context)); + } + + CHK( DERPopNesting(Context)); + CHK( DERPopNesting(Context)); + CHK( DERPopNesting(Context)); + + CHK( DERPopNesting(Context)); CHK(DERPopNesting(Context)); ASRT(DERGetNestingDepth(Context) == 0); @@ -638,7 +723,7 @@ X509GetRootCertTBS( CHK( DERAddBitString(Tbs, keyUsageCA, sizeof(keyUsageCA))); CHK( DERPopNesting(Tbs)); CHK( DERPopNesting(Tbs)); - if(PathLength >= 0) + if (PathLength >= 0) { CHK( DERStartSequenceOrSet(Tbs, true)); CHK( DERAddOID(Tbs, basicConstraintsOID)); diff --git a/CyReP/cyrep/RiotX509Bldr.h b/CyReP/cyrep/RiotX509Bldr.h index 148040c..bca6b8e 100644 --- a/CyReP/cyrep/RiotX509Bldr.h +++ b/CyReP/cyrep/RiotX509Bldr.h @@ -33,6 +33,14 @@ typedef struct const char *SubjectCountry; } RIOT_X509_TBS_DATA; + +typedef struct +{ + int* Oid; + uint8_t* DerBuffer; + uint32_t DerBufferSize; +} RIOT_X509_OID; + int X509GetDeviceCertTBS( DERBuilderContext *Tbs, @@ -83,10 +91,25 @@ X509GetDEREcc( ); int -X509GetDERCsrTbs( +X509GetDERCsrTBS( DERBuilderContext *Context, RIOT_X509_TBS_DATA *TbsData, - const RIOT_ECC_PUBLIC *DeviceIDPub + const RIOT_ECC_PUBLIC *DeviceIDPub, + RIOT_X509_OID *OidExtensions, + const size_t OidExtensionsCount +); + +int +X509GetCSRCertTBS( + DERBuilderContext *Tbs, + RIOT_X509_TBS_DATA *TbsData, + const RIOT_ECC_PUBLIC *CsrKeyPub, + const RIOT_ECC_PUBLIC *AuthorityKeyPub, + int32_t PathLen, + const uint8_t *SubjectKeyDerBuffer, + uint32_t SubjectKeyDerBufferSize, + const uint8_t *ExtensionDerBuffer, + uint32_t ExtensionDerBufferSize ); int diff --git a/CyReP/tcps/cborhelper.c b/CyReP/tcps/cborhelper.c index 229105d..83cd591 100644 --- a/CyReP/tcps/cborhelper.c +++ b/CyReP/tcps/cborhelper.c @@ -1,12 +1,17 @@ #include "cbor.h" #include "cborhelper.h" -#include "extract_number_p.h" + +// +// NOTE: These wrappers should go away. tinycbor will expose similar APIs +// to cbor_value_get_****_string_chunk, with improvments for single-chunk strings +// + CborError cbor_value_ref_byte_string( CborValue *Cborstring, const uint8_t **Bstr, - uint32_t *BstrSize, + size_t *BstrSize, CborValue *Next ) @@ -23,10 +28,7 @@ Advances the Value to the next cbor object. { CborError err; const uint8_t *ptr; - uint64_t len; - - *Bstr = NULL; - *BstrSize = 0; + size_t len; if (Cborstring == NULL || Bstr == NULL || @@ -36,22 +38,34 @@ Advances the Value to the next cbor object. return CborErrorInternalError; } - if (!cbor_value_is_byte_string(Cborstring) && - !cbor_value_is_text_string(Cborstring)) { + *Bstr = NULL; + *BstrSize = 0; + + if (!cbor_value_is_byte_string( Cborstring )) { return CborErrorIllegalType; } - // Utilize the API to validate the value as well as obtaining the size. - err = cbor_value_get_string_length(Cborstring, BstrSize); + while (true) + { + err = cbor_value_get_byte_string_chunk( Cborstring, &ptr, &len, Next ); + + if (err != CborNoError) { + return err; + } - if (err == CborNoError) { - ptr = Cborstring->ptr; - extract_number(&ptr, Cborstring->parser->end, &len); - if (len > 0) { + if (ptr != NULL) { + // copy out the pointer. As the data is not chunked this should only happen once. + assert( *Bstr == NULL ); *Bstr = ptr; + *BstrSize = len; + continue; } - assert(*BstrSize == len); - err = cbor_value_advance(Next); + + // eof. We should already have a valid str. + if (*Bstr == NULL) { + return CborErrorInternalError; + } + break; } return err; @@ -60,10 +74,52 @@ Advances the Value to the next cbor object. CborError cbor_value_ref_text_string( CborValue *Cborstring, - const uint8_t **Bstr, - uint32_t *BstrSize, + const char **Str, + size_t *StrSize, CborValue *Next ) { - return cbor_value_ref_byte_string(Cborstring, Bstr, BstrSize, Next); + CborError err; + const char *ptr; + size_t len; + + if (Cborstring == NULL || + Str == NULL || + StrSize == NULL || + Next == NULL) + { + return CborErrorInternalError; + } + + *Str = NULL; + *StrSize = 0; + + if (!cbor_value_is_text_string( Cborstring )) { + return CborErrorIllegalType; + } + + while (true) + { + err = cbor_value_get_text_string_chunk( Cborstring, &ptr, &len, Next ); + + if (err != CborNoError) { + return err; + } + + if (ptr != NULL) { + // copy out the pointer. As the data is not chunked this should only happen once. + assert( *Str == NULL ); + *Str = ptr; + *StrSize = len; + continue; + } + + // eof. We should already have a valid str. + if (*Str == NULL) { + return CborErrorInternalError; + } + break; + } + + return err; } \ No newline at end of file diff --git a/CyReP/tcps/cborhelper.h b/CyReP/tcps/cborhelper.h index 6e7e658..542d4bb 100644 --- a/CyReP/tcps/cborhelper.h +++ b/CyReP/tcps/cborhelper.h @@ -26,15 +26,15 @@ CborError cbor_value_ref_byte_string( CborValue *Cborstring, const uint8_t **Bstr, - uint32_t *BstrSize, + size_t *BstrSize, CborValue *Next ); CborError cbor_value_ref_text_string( CborValue *Cborstring, - const uint8_t **Bstr, - uint32_t *BstrSize, + const char **Str, + size_t *BstrSize, CborValue *Next ); diff --git a/Emulator/RIoTEmulator.cpp b/Emulator/RIoTEmulator.cpp index 8528ebc..24bc4ff 100644 --- a/Emulator/RIoTEmulator.cpp +++ b/Emulator/RIoTEmulator.cpp @@ -344,7 +344,7 @@ CreateDeviceAuthBundle( else if (DeviceCertType == RIOT_CSR) { // Generating CSR DERInitContext(&derCtx, derBuffer, DER_MAX_TBS); - X509GetDERCsrTbs(&derCtx, &x509AliasTBSData, &deviceIDPub); + X509GetDERCsrTBS(&derCtx, &x509AliasTBSData, &deviceIDPub, NULL, 0); // Sign the Alias Key Certificate's TBS region RiotCrypt_Sign(&tbsSig, derCtx.Buffer, derCtx.Position, &deviceIDPriv); @@ -429,6 +429,17 @@ CreateDeviceAuthBundle( WriteBinaryFile("R00tCrt.cer", (uint8_t *)PEM, length); #endif + // print root private + length = sizeof(PEM); + DERInitContext(&derCtx, derBuffer, DER_MAX_TBS); + X509GetDEREcc(&derCtx, *(RIOT_ECC_PUBLIC*)eccRootPubBytes, *(RIOT_ECC_PRIVATE*)eccRootPrivBytes); + DERtoPEM(&derCtx, R_ECC_PRIVATEKEY_TYPE, PEM, &length); + +#ifdef DEBUG + PEM[length] = '\0'; // JUST FOR PRINTF + printf("%s", PEM); +#endif + return 0; } diff --git a/Emulator/RIoTEmulator.vcxproj b/Emulator/RIoTEmulator.vcxproj index 17bc562..b4501ea 100644 --- a/Emulator/RIoTEmulator.vcxproj +++ b/Emulator/RIoTEmulator.vcxproj @@ -191,7 +191,6 @@ - $(SolutionDir)..\CyReP;%(AdditionalIncludeDirectories) $(SolutionDir)..\CyReP;%(AdditionalIncludeDirectories) diff --git a/Emulator/RIoTEmulator.vcxproj.filters b/Emulator/RIoTEmulator.vcxproj.filters index 3638fc8..e3e9e73 100644 --- a/Emulator/RIoTEmulator.vcxproj.filters +++ b/Emulator/RIoTEmulator.vcxproj.filters @@ -48,9 +48,6 @@ TinyCBOR - - TinyCBOR - TCPS diff --git a/External/tinycbor b/External/tinycbor index acf202a..aa37315 160000 --- a/External/tinycbor +++ b/External/tinycbor @@ -1 +1 @@ -Subproject commit acf202a38118ed049785d210b5de84effa43c3de +Subproject commit aa3731592c784ce91e7fcafa66665c147b18be25 diff --git a/Sample/Barnacle/Tool/.gitignore b/Sample/Barnacle/Tool/.gitignore index 7b1704c..f01f6e7 100644 --- a/Sample/Barnacle/Tool/.gitignore +++ b/Sample/Barnacle/Tool/.gitignore @@ -1,2 +1,3 @@ -[rR]elease -[dD]ebug \ No newline at end of file +![rR]elease +![dD]ebug +BarT/ diff --git a/Sample/Barnacle/Tool/BarT/BarT.vcxproj b/Sample/Barnacle/Tool/BarT/BarT.vcxproj index bffc4f5..18a6755 100644 --- a/Sample/Barnacle/Tool/BarT/BarT.vcxproj +++ b/Sample/Barnacle/Tool/BarT/BarT.vcxproj @@ -101,7 +101,7 @@ Level3 Disabled _DEBUG;_CONSOLE;%(PreprocessorDefinitions) - $(SolutionDir)..\..\..\CyReP;$(SolutionDir)..\Shared\BarnacleTA;$(SolutionDir)..\Shared\Barnacle;%(AdditionalIncludeDirectories) + $(SolutionDir)..\..\..\CyReP;$(SolutionDir)..\Shared\BarnacleTA;$(SolutionDir)..\Shared\Barnacle;$(SolutionDir)..\..\..\External\tinycbor\src;%(AdditionalIncludeDirectories) Console @@ -133,7 +133,7 @@ true true NDEBUG;_CONSOLE;%(PreprocessorDefinitions) - $(SolutionDir)..\..\..\CyReP;$(SolutionDir)..\Shared\BarnacleTA;$(SolutionDir)..\Shared\Barnacle;%(AdditionalIncludeDirectories) + $(SolutionDir)..\..\..\CyReP;$(SolutionDir)..\Shared\BarnacleTA;$(SolutionDir)..\Shared\Barnacle;$(SolutionDir)..\..\..\External\tinycbor\src;%(AdditionalIncludeDirectories) Console