Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sign and validate with public keys (OpenSSL, mbedtls) #68

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ find_package(OpenSSL 1.0 REQUIRED)
option (use_context "Use context pointer for COSE functions" ON)
option (verbose "Produce verbose makefile output" OFF)
option (optimize "Optimize for size" OFF)
option (fatal_warnings "Treat build warnings as error" OFF)
option (fatal_warnings "Treat build warnings as error" ON)
option (coveralls "Generate coveralls data" ON)
option ( coveralls_send "Send data to coveralls site" OFF )
option (build_docs "Create docs using Doxygen" ${DOXYGEN_FOUND} )
Expand Down
2 changes: 1 addition & 1 deletion dumper/dumper.c
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ void DumpTree(const cn_cbor * cbor, FILE * out, const FOO *pFOO, int depth, int
if (pFOO != NULL) {
// Locate the right entry in foo
for (i2 = 0, pFoo2 = pFOO->children; i2 < pFOO->count; pFoo2++, i2 += 1) {
if (pFoo2->type != cbor2->type) continue;
if ((unsigned)pFoo2->type != cbor2->type) continue;
switch (cbor2->type) {
case CN_CBOR_UINT:
if ((group != 0) && (pFoo2->group != 0) && (pFoo2->group != group)) continue;
Expand Down
13 changes: 13 additions & 0 deletions include/cose.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#define USE_MBED_TLS 1

#include <cn-cbor/cn-cbor.h>

#ifdef __cplusplus
Expand Down Expand Up @@ -314,14 +316,25 @@ bool COSE_Signer_SetExternal(HCOSE_SIGNER hcose, const byte * pbExternalData, si
* Sign routines
*/

#if USE_MBED_TLS
typedef struct mbedtls_ecp_keypair eckey_t;
#else
typedef struct eckey_t {
struct ec_key_st *key;
int group;
} eckey_t;
#endif // USE_MBED_TLS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need #includes to get struct mbedtls_ecp_keypair of struct ec_key_st defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are using struct mbedtls_ecp_keypair and struct ec_key_st by just pointers, they are not necessary be defined, but only need to be declared (forward declaration). by writing typedef struct mbedtls_ecp_keypair eckey_t;, the mbedtls_ecp_keypair structure got itself declared.


HCOSE_SIGN0 COSE_Sign0_Init(COSE_INIT_FLAGS flags, CBOR_CONTEXT_COMMA cose_errback * perr);
bool COSE_Sign0_Free(HCOSE_SIGN0 cose);

bool COSE_Sign0_SetContent(HCOSE_SIGN0 cose, const byte * rgbContent, size_t cbContent, cose_errback * errp);
bool COSE_Sign0_SetExternal(HCOSE_SIGN0 hcose, const byte * pbExternalData, size_t cbExternalData, cose_errback * perr);

bool COSE_Sign0_Sign(HCOSE_SIGN0 h, const cn_cbor * pkey, cose_errback * perr);
bool COSE_Sign0_Sign_eckey(HCOSE_SIGN0 h, const eckey_t * pbKey, cose_errback * perr);
bool COSE_Sign0_validate(HCOSE_SIGN0 hSign, const cn_cbor * pkey, cose_errback * perr);
bool COSE_Sign0_validate_eckey(HCOSE_SIGN0 hSign, const eckey_t * pbKey, cose_errback * perr);
cn_cbor * COSE_Sign0_map_get_int(HCOSE_SIGN0 h, int key, int flags, cose_errback * perror);
bool COSE_Sign0_map_put_int(HCOSE_SIGN0 cose, int key, cn_cbor * value, int flags, cose_errback * errp);

Expand Down
3 changes: 2 additions & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ else ()
endif()

set ( cose_sources
Cose.c
Cose.c
key.c
MacMessage.c
MacMessage0.c
Sign.c
Expand Down
2 changes: 0 additions & 2 deletions src/Cose.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ bool IsValidCOSEHandle(HCOSE h)
return true;
}


bool _COSE_Init(COSE_INIT_FLAGS flags, COSE* pobj, int msgType, CBOR_CONTEXT_COMMA cose_errback * perr)
{
cn_cbor_errback errState;;
Expand Down Expand Up @@ -45,7 +44,6 @@ bool _COSE_Init(COSE_INIT_FLAGS flags, COSE* pobj, int msgType, CBOR_CONTEXT_COM
CHECK_CONDITION_CBOR(_COSE_array_replace(pobj, pobj->m_unprotectMap, INDEX_UNPROTECTED, CBOR_CONTEXT_PARAM_COMMA &errState), errState);
pobj->m_ownUnprotectedMap = false;


if (!(flags & COSE_INIT_FLAGS_NO_CBOR_TAG)) {
cn_cbor_errback cbor_error;
cn_cbor * cn = cn_cbor_tag_create(msgType, pobj->m_cborRoot, CBOR_CONTEXT_PARAM_COMMA &cbor_error);
Expand Down
6 changes: 3 additions & 3 deletions src/Encrypt.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ HCOSE_ENVELOPED _COSE_Enveloped_Init_From_Object(cn_cbor * cbor, COSE_Enveloped
bool COSE_Enveloped_Free(HCOSE_ENVELOPED h)
{
#ifdef USE_CBOR_CONTEXT
cn_cbor_context context;
cn_cbor_context *context;
#endif
COSE_Enveloped * p = (COSE_Enveloped *)h;

Expand All @@ -141,14 +141,14 @@ bool COSE_Enveloped_Free(HCOSE_ENVELOPED h)
}

#ifdef USE_CBOR_CONTEXT
context = ((COSE_Enveloped *)h)->m_message.m_allocContext;
context = &((COSE_Enveloped *)h)->m_message.m_allocContext;
#endif

_COSE_RemoveFromList(&EnvelopedRoot, &p->m_message);

_COSE_Enveloped_Release((COSE_Enveloped *)h);

COSE_FREE((COSE_Enveloped *)h, &context);
COSE_FREE((COSE_Enveloped *)h, context);

return true;
}
Expand Down
8 changes: 4 additions & 4 deletions src/Encrypt0.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,21 +128,21 @@ HCOSE_ENCRYPT _COSE_Encrypt_Init_From_Object(cn_cbor * cbor, COSE_Encrypt * pIn,
bool COSE_Encrypt_Free(HCOSE_ENCRYPT h)
{
#ifdef USE_CBOR_CONTEXT
cn_cbor_context context;
cn_cbor_context *context;
#endif
COSE_Encrypt * pEncrypt = (COSE_Encrypt *)h;

if (!IsValidEncryptHandle(h)) return false;

#ifdef USE_CBOR_CONTEXT
context = ((COSE_Encrypt *)h)->m_message.m_allocContext;
context = &((COSE_Encrypt *)h)->m_message.m_allocContext;
#endif

_COSE_Encrypt_Release(pEncrypt);

_COSE_RemoveFromList(&EncryptRoot, &pEncrypt->m_message);
COSE_FREE((COSE_Encrypt *)h, &context);

COSE_FREE((COSE_Encrypt *)h, context);

return true;
}
Expand Down
10 changes: 6 additions & 4 deletions src/MacMessage.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ HCOSE_MAC _COSE_Mac_Init_From_Object(cn_cbor * cbor, COSE_MacMessage * pIn, CBOR
bool COSE_Mac_Free(HCOSE_MAC h)
{
#ifdef USE_CBOR_CONTEXT
cn_cbor_context context;
cn_cbor_context *context;
#endif
COSE_MacMessage * p = (COSE_MacMessage *)h;

Expand All @@ -127,12 +127,12 @@ bool COSE_Mac_Free(HCOSE_MAC h)
_COSE_RemoveFromList(&MacRoot, &p->m_message);

#ifdef USE_CBOR_CONTEXT
context = ((COSE_MacMessage *)h)->m_message.m_allocContext;
context = &((COSE_MacMessage *)h)->m_message.m_allocContext;
#endif

_COSE_Mac_Release((COSE_MacMessage *)h);

COSE_FREE((COSE_MacMessage *)h, &context);
COSE_FREE((COSE_MacMessage *)h, context);

return true;
}
Expand Down Expand Up @@ -234,6 +234,7 @@ bool _COSE_Mac_Build_AAD(COSE * pCose, const char * szContext, byte ** ppbAuthDa
cn_cbor * ptmp = NULL;
cn_cbor * pcn;
size_t cbAuthData;
ssize_t written;
byte * pbAuthData = NULL;

// Build authenticated data
Expand Down Expand Up @@ -286,7 +287,8 @@ bool _COSE_Mac_Build_AAD(COSE * pCose, const char * szContext, byte ** ppbAuthDa
CHECK_CONDITION(cbAuthData > 0, COSE_ERR_CBOR);
pbAuthData = (byte *)COSE_CALLOC(cbAuthData, 1, context);
CHECK_CONDITION(pbAuthData != NULL, COSE_ERR_OUT_OF_MEMORY);
CHECK_CONDITION(cn_cbor_encoder_write(pbAuthData, 0, cbAuthData, pAuthData) == cbAuthData, COSE_ERR_CBOR);
written = cn_cbor_encoder_write(pbAuthData, 0, cbAuthData, pAuthData);
CHECK_CONDITION(written >= 0 && (size_t)written == cbAuthData, COSE_ERR_CBOR);

*ppbAuthData = pbAuthData;
*pcbAuthData = cbAuthData;
Expand Down
6 changes: 3 additions & 3 deletions src/MacMessage0.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ HCOSE_MAC0 _COSE_Mac0_Init_From_Object(cn_cbor * cbor, COSE_Mac0Message * pIn, C
bool COSE_Mac0_Free(HCOSE_MAC0 h)
{
#ifdef USE_CBOR_CONTEXT
cn_cbor_context context;
cn_cbor_context *context;
#endif
COSE_Mac0Message * p = (COSE_Mac0Message *)h;

Expand All @@ -114,12 +114,12 @@ bool COSE_Mac0_Free(HCOSE_MAC0 h)
_COSE_RemoveFromList(&Mac0Root, &p->m_message);

#ifdef USE_CBOR_CONTEXT
context = p->m_message.m_allocContext;
context = &p->m_message.m_allocContext;
#endif

_COSE_Mac0_Release(p);

COSE_FREE(p, &context);
COSE_FREE(p, context);

return true;
}
Expand Down
19 changes: 15 additions & 4 deletions src/Recipient.c
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ static bool HKDF_X(COSE * pCose, bool fHMAC, bool fECDH, bool fStatic, bool fSen

if (fHMAC) {
#ifdef USE_HKDF_SHA2
if (!HKDF_Extract(pCose, pbSecret, cbSecret, cbitHash, rgbDigest, &cbDigest, CBOR_CONTEXT_PARAM_COMMA perr)) goto errorReturn;
if (!HKDF_Extract(pCose, pbSecret, cbSecret, cbitHash, rgbDigest, &cbDigest, perr)) goto errorReturn;

if (!HKDF_Expand(pCose, cbitHash, rgbDigest, cbDigest, pbContext, cbContext, pbKey, cbitKey / 8, perr)) goto errorReturn;
#else
Expand Down Expand Up @@ -251,6 +251,10 @@ bool _COSE_Recipient_decrypt(COSE_RecipientInfo * pRecip, COSE_RecipientInfo * p
int cbitKeyX = 0;
byte rgbKey[256 / 8];

UNUSED(rgbKey);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing. Why would a local variable exist if it is unused. Probably need an #ifdef around the declaration instead.

UNUSED(cbKey2);
UNUSED(pRecipUse);
UNUSED(algIn);
UNUSED(pcose);

#ifdef USE_CBOR_CONTEXT
Expand Down Expand Up @@ -279,7 +283,7 @@ bool _COSE_Recipient_decrypt(COSE_RecipientInfo * pRecip, COSE_RecipientInfo * p
CHECK_CONDITION(pRecip->m_pkey != NULL, COSE_ERR_INVALID_PARAMETER);
cn = cn_cbor_mapget_int(pRecip->m_pkey, -1);
CHECK_CONDITION((cn != NULL) && (cn->type == CN_CBOR_BYTES), COSE_ERR_INVALID_PARAMETER);
CHECK_CONDITION((cn->length == (unsigned int)cbitKeyOut / 8), COSE_ERR_INVALID_PARAMETER);
CHECK_CONDITION(((size_t)cn->length == cbitKeyOut / 8), COSE_ERR_INVALID_PARAMETER);
memcpy(pbKeyOut, cn->v.bytes, cn->length);

return true;
Expand Down Expand Up @@ -574,6 +578,9 @@ bool _COSE_Recipient_encrypt(COSE_RecipientInfo * pRecipient, const byte * pbCon
byte * pbKey = NULL;
size_t cbKey = 0;

UNUSED(pbContent);
UNUSED(cbContent);

#ifdef USE_CBOR_CONTEXT
context = &pRecipient->m_encrypt.m_message.m_allocContext;
#endif // USE_CBOR_CONTEXT
Expand Down Expand Up @@ -867,6 +874,8 @@ byte * _COSE_RecipientInfo_generateKey(COSE_RecipientInfo * pRecipient, int algI
const cn_cbor * pK;
byte *pbSecret = NULL;

UNUSED(algIn);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

algIn is not unused in all cases. I'd like to see a clearer indication of when it is used and when it is not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usages of algIn are all included in #if ... #endif preprocessor directives. algIn will never be used if none of those macros is defined. To make an unused indication, we need to write:

#if !USE_Direct_HKDF_HMAC_SHA_256 && !USE_Direct_HKDF_HMAC_SHA_512 && \
     !USE_Direct_HKDF_AES_128 && !USE_Direct_HKDF_AES_256 && \
     !USE_ECDH_ES_HKDF_256 && !USE_ECDH_ES_HKDF_512 && \
     !USE_ECDH_SS_HKDF_256 && !USE_ECDH_SS_HKDF_512
    UNUSED(algIn);
#endif

Do we truly wants this everywhere?

I think the UNUSED() macro should be understood as MAYBE_UNUSED(). Or I can rename it like that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of a MAYBE_UNUSED(). I think that is more clear. Keep UNUSED() for parameters that are truly never used.


CHECK_CONDITION(cn_Alg != NULL, COSE_ERR_INVALID_PARAMETER);
CHECK_CONDITION((cn_Alg->type == CN_CBOR_UINT) || (cn_Alg->type == CN_CBOR_INT), COSE_ERR_INVALID_PARAMETER);
alg = (int)cn_Alg->v.uint;
Expand All @@ -881,7 +890,7 @@ byte * _COSE_RecipientInfo_generateKey(COSE_RecipientInfo * pRecipient, int algI
CHECK_CONDITION(pRecipient->m_pkey != NULL, COSE_ERR_INVALID_PARAMETER);
pK = cn_cbor_mapget_int(pRecipient->m_pkey, -1);
CHECK_CONDITION((pK != NULL) && (pK->type == CN_CBOR_BYTES), COSE_ERR_INVALID_PARAMETER);
CHECK_CONDITION(pK->length == cbitKeySize / 8, COSE_ERR_INVALID_PARAMETER);
CHECK_CONDITION((size_t)pK->length == cbitKeySize / 8, COSE_ERR_INVALID_PARAMETER);
memcpy(pb, pK->v.bytes, cbitKeySize / 8);
break;

Expand Down Expand Up @@ -1226,6 +1235,7 @@ bool BuildContextBytes(COSE * pcose, int algID, size_t cbitKey, byte ** ppbConte
cn_cbor * cnParam;
byte * pbContext = NULL;
size_t cbContext;
ssize_t written;

pArray = cn_cbor_array_create(CBOR_CONTEXT_PARAM_COMMA &cbor_error);
CHECK_CONDITION_CBOR(pArray != NULL, cbor_error);
Expand Down Expand Up @@ -1350,7 +1360,8 @@ bool BuildContextBytes(COSE * pcose, int algID, size_t cbitKey, byte ** ppbConte
CHECK_CONDITION(cbContext > 0, COSE_ERR_CBOR);
pbContext = (byte *)COSE_CALLOC(cbContext, 1, context);
CHECK_CONDITION(pbContext != NULL, COSE_ERR_OUT_OF_MEMORY);
CHECK_CONDITION(cn_cbor_encoder_write(pbContext, 0, cbContext, pArray) == cbContext, COSE_ERR_CBOR);
written = cn_cbor_encoder_write(pbContext, 0, cbContext, pArray);
CHECK_CONDITION(written >= 0 && (size_t)written == cbContext, COSE_ERR_CBOR);

*ppbContext = pbContext;
*pcbContext = cbContext;
Expand Down
6 changes: 3 additions & 3 deletions src/Sign.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ HCOSE_SIGN _COSE_Sign_Init_From_Object(cn_cbor * cbor, COSE_SignMessage * pIn, C
bool COSE_Sign_Free(HCOSE_SIGN h)
{
#ifdef USE_CBOR_CONTEXT
cn_cbor_context context;
cn_cbor_context *context;
#endif
COSE_SignMessage * pMessage = (COSE_SignMessage *)h;

Expand All @@ -122,12 +122,12 @@ bool COSE_Sign_Free(HCOSE_SIGN h)
_COSE_RemoveFromList(&SignRoot, &pMessage->m_message);

#ifdef USE_CBOR_CONTEXT
context = pMessage->m_message.m_allocContext;
context = &pMessage->m_message.m_allocContext;
#endif

_COSE_Sign_Release(pMessage);

COSE_FREE(pMessage, &context);
COSE_FREE(pMessage, context);

return true;
}
Expand Down
Loading