| Message ID | 0d2fc089-d1f6-e48f-dfb4-c8b34945ab9d@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | Support pkcs11 URIs for sslkey and sslcert strings | expand |
Hi Matt, On 03.02.23 19:14, Matt Wood wrote: > Parse sslkey and sslcert strings and set the curl ssl engine, > key type, and certtype if a valid pcks11 URI is found. This can be used > for Secure Elements or HSMs holding keys and certificates. > > Signed-off-by: Matt Wood <matt.wood@microchip.com> > --- > corelib/channel_curl.c | 42 +++++++++++++++++++++++++++++ > examples/configuration/swupdate.cfg | 4 +-- > 2 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c > index 0435f15..4be1cae 100644 > --- a/corelib/channel_curl.c > +++ b/corelib/channel_curl.c > @@ -607,6 +607,48 @@ channel_op_res_t channel_set_options(channel_t *this, channel_data_t *channel_da > goto cleanup; > } > > + /* Check if sslkey string contains a pkcs11 URI > + * and set curl engine and keytype accordinly > + */ > + if (strncasecmp(channel_data->sslkey, "pkcs11:", 7) == 0) { > + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE,"pkcs11"); > + > + if (result != CURLE_OK) { > + ERROR("Error %d setting CURLOPT_SSLENGINE", result); > + result = CHANNEL_EINIT; > + goto cleanup; > + } > + > + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLKEYTYPE, "ENG"); > + > + if (result != CURLE_OK) { > + ERROR("Error %d setting CURLOPT_SSLKEYTYPE", result); > + result = CHANNEL_EINIT; > + goto cleanup; > + } > + } Ok > + > + /* Check if sslcert string contains a pkcs11 object URI > + * and set curl engine and certtype accordingly > + */ > + if (strncasecmp(channel_data->sslcert, "pkcs11:", 7) == 0) { > + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE, "pkcs11"); > + > + if (result != CURLE_OK) { > + ERROR("Error %d setting CURLOPT_SSLENGINE", result); > + result = CHANNEL_EINIT; > + goto cleanup; > + } > + > + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLCERTTYPE, "ENG"); > + > + if (result != CURLE_OK) { > + ERROR("Error %d setting CURLOPT_SSLCERTTYPE", result); > + result = CHANNEL_EINIT; > + goto cleanup; > + } > + } > + Why is this code duplicated ? Copy&paste ? > /* Only use cafile when set, otherwise let curl use > * the default system location for cacert bundle > */ > diff --git a/examples/configuration/swupdate.cfg b/examples/configuration/swupdate.cfg > index db63110..27d378d 100644 > --- a/examples/configuration/swupdate.cfg > +++ b/examples/configuration/swupdate.cfg > @@ -161,9 +161,9 @@ identify : ( > # cafile : string > # File with Public Certificate Authority > # sslkey : string > -# path of the file containing the key for ssl connection > +# path of the file or pkcs11 object URI containing the key for SSL connection > # sslcert : string > -# path of the file containing the certificate for SSL connection > +# path of the file or pkcs11 object URI containing the certificate for SSL connection It will be nice to add here a real example with a pkcs11 string. Best regards, Stefano > # targettoken : string > # hawkBit target security token > # gatewaytoken : string
Hi Stefano, On 2/4/23 07:01, Stefano Babic wrote: > Hi Matt, > > On 03.02.23 19:14, Matt Wood wrote: >> Parse sslkey and sslcert strings and set the curl ssl engine, >> key type, and certtype if a valid pcks11 URI is found. This can be used >> for Secure Elements or HSMs holding keys and certificates. >> >> Signed-off-by: Matt Wood <matt.wood@microchip.com> >> --- >> corelib/channel_curl.c | 42 +++++++++++++++++++++++++++++ >> examples/configuration/swupdate.cfg | 4 +-- >> 2 files changed, 44 insertions(+), 2 deletions(-) >> >> diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c >> index 0435f15..4be1cae 100644 >> --- a/corelib/channel_curl.c >> +++ b/corelib/channel_curl.c >> @@ -607,6 +607,48 @@ channel_op_res_t channel_set_options(channel_t *this, channel_data_t *channel_da >> goto cleanup; >> } >> + /* Check if sslkey string contains a pkcs11 URI >> + * and set curl engine and keytype accordinly >> + */ >> + if (strncasecmp(channel_data->sslkey, "pkcs11:", 7) == 0) { >> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE,"pkcs11"); >> + >> + if (result != CURLE_OK) { >> + ERROR("Error %d setting CURLOPT_SSLENGINE", result); >> + result = CHANNEL_EINIT; >> + goto cleanup; >> + } >> + >> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLKEYTYPE, "ENG"); >> + >> + if (result != CURLE_OK) { >> + ERROR("Error %d setting CURLOPT_SSLKEYTYPE", result); >> + result = CHANNEL_EINIT; >> + goto cleanup; >> + } >> + } > > Ok > >> + >> + /* Check if sslcert string contains a pkcs11 object URI >> + * and set curl engine and certtype accordingly >> + */ >> + if (strncasecmp(channel_data->sslcert, "pkcs11:", 7) == 0) { >> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE, "pkcs11"); >> + >> + if (result != CURLE_OK) { >> + ERROR("Error %d setting CURLOPT_SSLENGINE", result); >> + result = CHANNEL_EINIT; >> + goto cleanup; >> + } >> + >> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLCERTTYPE, "ENG"); >> + >> + if (result != CURLE_OK) { >> + ERROR("Error %d setting CURLOPT_SSLCERTTYPE", result); >> + result = CHANNEL_EINIT; >> + goto cleanup; >> + } >> + } >> + > > Why is this code duplicated ? Copy&paste ? > The two blocks are not actually duplicated, the first handles the sslkey and the second handles the sslcert. There are cases where either the cert or the key would be acted upon, and it is not guaranteed to have both used at the same time which is why I broke the code into two blocks. It could be refactored to a single block; let me know if you'd prefer this instead: + /* Check if sslkey or sslcert strings contains a pkcs11 URI + * and set curl engine and types accordingly + */ + int keyUri, certUri = 0; + + if ((keyUri = strncasecmp(channel_data->sslkey, "pkcs11:", 7) == 0) || + (certUri = strncasecmp(channel_data->sslcert, "pkcs11:", 7) == 0)) { + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE,"pkcs11"); + + if (result != CURLE_OK) { + ERROR("Error %d setting CURLOPT_SSLENGINE", result); + result = CHANNEL_EINIT; + goto cleanup; + } + + if (keyUri == 0) { + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLKEYTYPE, "ENG"); + + if (result != CURLE_OK) { + ERROR("Error %d setting CURLOPT_SSLKEYTYPE", result); + result = CHANNEL_EINIT; + goto cleanup; + } + } + + if (certUri == 0) { + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLCERTTYPE, "ENG"); + + if (result != CURLE_OK) { + ERROR("Error %d setting CURLOPT_SSLCERTTYPE", result); + result = CHANNEL_EINIT; + goto cleanup; + } + } + } > > >> /* Only use cafile when set, otherwise let curl use >> * the default system location for cacert bundle >> */ >> diff --git a/examples/configuration/swupdate.cfg b/examples/configuration/swupdate.cfg >> index db63110..27d378d 100644 >> --- a/examples/configuration/swupdate.cfg >> +++ b/examples/configuration/swupdate.cfg >> @@ -161,9 +161,9 @@ identify : ( >> # cafile : string >> # File with Public Certificate Authority >> # sslkey : string >> -# path of the file containing the key for ssl connection >> +# path of the file or pkcs11 object URI containing the key for SSL connection >> # sslcert : string >> -# path of the file containing the certificate for SSL connection >> +# path of the file or pkcs11 object URI containing the certificate for SSL connection > > It will be nice to add here a real example with a pkcs11 string. Agreed. I had initially put an example string but opted not to as there are many variations. But you are right, there should be some kind of example. How about this? # sslkey : string -# path of the file containing the key for ssl connection +# path of the file containing the key for SSL connection or pkcs11 URI +# (ex. "pkcs11:model=ATECC608B;token=0ABC;serial=0123456789abcdef;object=device;type=private") # sslcert : string -# path of the file containing the certificate for SSL connection +# path of the file containing the certificate for SSL connection or pkcs11 URI + (ex. "pkcs11:model=ATECC608B;token=0ABC;serial=0123456789abcdef;object=device;type=cert") Let me know if these changes are sufficient and I'll resend the patch. Thanks, Matt. > > Best regards, > Stefano > >> # targettoken : string >> # hawkBit target security token >> # gatewaytoken : string >
On 05.02.23 16:20, Matt Wood wrote: > Hi Stefano, > > On 2/4/23 07:01, Stefano Babic wrote: >> Hi Matt, >> >> On 03.02.23 19:14, Matt Wood wrote: >>> Parse sslkey and sslcert strings and set the curl ssl engine, >>> key type, and certtype if a valid pcks11 URI is found. This can be used >>> for Secure Elements or HSMs holding keys and certificates. >>> >>> Signed-off-by: Matt Wood <matt.wood@microchip.com> >>> --- >>> corelib/channel_curl.c | 42 +++++++++++++++++++++++++++++ >>> examples/configuration/swupdate.cfg | 4 +-- >>> 2 files changed, 44 insertions(+), 2 deletions(-) >>> >>> diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c >>> index 0435f15..4be1cae 100644 >>> --- a/corelib/channel_curl.c >>> +++ b/corelib/channel_curl.c >>> @@ -607,6 +607,48 @@ channel_op_res_t channel_set_options(channel_t *this, channel_data_t *channel_da >>> goto cleanup; >>> } >>> + /* Check if sslkey string contains a pkcs11 URI >>> + * and set curl engine and keytype accordinly >>> + */ >>> + if (strncasecmp(channel_data->sslkey, "pkcs11:", 7) == 0) { >>> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE,"pkcs11"); >>> + >>> + if (result != CURLE_OK) { >>> + ERROR("Error %d setting CURLOPT_SSLENGINE", result); >>> + result = CHANNEL_EINIT; >>> + goto cleanup; >>> + } >>> + >>> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLKEYTYPE, "ENG"); >>> + >>> + if (result != CURLE_OK) { >>> + ERROR("Error %d setting CURLOPT_SSLKEYTYPE", result); >>> + result = CHANNEL_EINIT; >>> + goto cleanup; >>> + } >>> + } >> >> Ok >> >>> + >>> + /* Check if sslcert string contains a pkcs11 object URI >>> + * and set curl engine and certtype accordingly >>> + */ >>> + if (strncasecmp(channel_data->sslcert, "pkcs11:", 7) == 0) { >>> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE, "pkcs11"); >>> + >>> + if (result != CURLE_OK) { >>> + ERROR("Error %d setting CURLOPT_SSLENGINE", result); >>> + result = CHANNEL_EINIT; >>> + goto cleanup; >>> + } >>> + >>> + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLCERTTYPE, "ENG"); >>> + >>> + if (result != CURLE_OK) { >>> + ERROR("Error %d setting CURLOPT_SSLCERTTYPE", result); >>> + result = CHANNEL_EINIT; >>> + goto cleanup; >>> + } >>> + } >>> + >> >> Why is this code duplicated ? Copy&paste ? >> > > The two blocks are not actually duplicated, the first handles the sslkey and the second handles the sslcert. There are cases where either the cert or the key would be acted upon, and it is not guaranteed to have both used at the same time which is why I broke the code into two blocks. > > It could be refactored to a single block; let me know if you'd prefer this instead: > > + /* Check if sslkey or sslcert strings contains a pkcs11 URI > + * and set curl engine and types accordingly > + */ > + int keyUri, certUri = 0; > + > + if ((keyUri = strncasecmp(channel_data->sslkey, "pkcs11:", 7) == 0) || > + (certUri = strncasecmp(channel_data->sslcert, "pkcs11:", 7) == 0)) { > + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE,"pkcs11"); > + > + if (result != CURLE_OK) { > + ERROR("Error %d setting CURLOPT_SSLENGINE", result); > + result = CHANNEL_EINIT; > + goto cleanup; > + } > + > + if (keyUri == 0) { > + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLKEYTYPE, "ENG"); > + > + if (result != CURLE_OK) { > + ERROR("Error %d setting CURLOPT_SSLKEYTYPE", result); > + result = CHANNEL_EINIT; > + goto cleanup; > + } > + } > + > + if (certUri == 0) { > + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLCERTTYPE, "ENG"); > + > + if (result != CURLE_OK) { > + ERROR("Error %d setting CURLOPT_SSLCERTTYPE", result); > + result = CHANNEL_EINIT; > + goto cleanup; > + } > + } > + } > > For my personal taste, this is better, thanks. >> >> >>> /* Only use cafile when set, otherwise let curl use >>> * the default system location for cacert bundle >>> */ >>> diff --git a/examples/configuration/swupdate.cfg b/examples/configuration/swupdate.cfg >>> index db63110..27d378d 100644 >>> --- a/examples/configuration/swupdate.cfg >>> +++ b/examples/configuration/swupdate.cfg >>> @@ -161,9 +161,9 @@ identify : ( >>> # cafile : string >>> # File with Public Certificate Authority >>> # sslkey : string >>> -# path of the file containing the key for ssl connection >>> +# path of the file or pkcs11 object URI containing the key for SSL connection >>> # sslcert : string >>> -# path of the file containing the certificate for SSL connection >>> +# path of the file or pkcs11 object URI containing the certificate for SSL connection >> >> It will be nice to add here a real example with a pkcs11 string. > > Agreed. I had initially put an example string but opted not to as there are many variations. But you are right, there should be some kind of example. How about this? > > # sslkey : string > -# path of the file containing the key for ssl connection > +# path of the file containing the key for SSL connection or pkcs11 URI > +# (ex. "pkcs11:model=ATECC608B;token=0ABC;serial=0123456789abcdef;object=device;type=private") > # sslcert : string > -# path of the file containing the certificate for SSL connection > +# path of the file containing the certificate for SSL connection or pkcs11 URI > + (ex. "pkcs11:model=ATECC608B;token=0ABC;serial=0123456789abcdef;object=device;type=cert") > Fine. > > Let me know if these changes are sufficient and I'll resend the patch. > That is fine, please repost, thanks. > Thanks, Matt. >> Best regards, Stefano >> >>> # targettoken : string >>> # hawkBit target security token >>> # gatewaytoken : string >> >
diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c index 0435f15..4be1cae 100644 --- a/corelib/channel_curl.c +++ b/corelib/channel_curl.c @@ -607,6 +607,48 @@ channel_op_res_t channel_set_options(channel_t *this, channel_data_t *channel_da goto cleanup; } + /* Check if sslkey string contains a pkcs11 URI + * and set curl engine and keytype accordinly + */ + if (strncasecmp(channel_data->sslkey, "pkcs11:", 7) == 0) { + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE,"pkcs11"); + + if (result != CURLE_OK) { + ERROR("Error %d setting CURLOPT_SSLENGINE", result); + result = CHANNEL_EINIT; + goto cleanup; + } + + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLKEYTYPE, "ENG"); + + if (result != CURLE_OK) { + ERROR("Error %d setting CURLOPT_SSLKEYTYPE", result); + result = CHANNEL_EINIT; + goto cleanup; + } + } + + /* Check if sslcert string contains a pkcs11 object URI + * and set curl engine and certtype accordingly + */ + if (strncasecmp(channel_data->sslcert, "pkcs11:", 7) == 0) { + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLENGINE, "pkcs11"); + + if (result != CURLE_OK) { + ERROR("Error %d setting CURLOPT_SSLENGINE", result); + result = CHANNEL_EINIT; + goto cleanup; + } + + result = curl_easy_setopt(channel_curl->handle, CURLOPT_SSLCERTTYPE, "ENG"); + + if (result != CURLE_OK) { + ERROR("Error %d setting CURLOPT_SSLCERTTYPE", result); + result = CHANNEL_EINIT; + goto cleanup; + } + } + /* Only use cafile when set, otherwise let curl use * the default system location for cacert bundle */ diff --git a/examples/configuration/swupdate.cfg b/examples/configuration/swupdate.cfg index db63110..27d378d 100644 --- a/examples/configuration/swupdate.cfg +++ b/examples/configuration/swupdate.cfg @@ -161,9 +161,9 @@ identify : ( # cafile : string # File with Public Certificate Authority # sslkey : string -# path of the file containing the key for ssl connection +# path of the file or pkcs11 object URI containing the key for SSL connection # sslcert : string -# path of the file containing the certificate for SSL connection +# path of the file or pkcs11 object URI containing the certificate for SSL connection # targettoken : string # hawkBit target security token # gatewaytoken : string
Parse sslkey and sslcert strings and set the curl ssl engine, key type, and certtype if a valid pcks11 URI is found. This can be used for Secure Elements or HSMs holding keys and certificates. Signed-off-by: Matt Wood <matt.wood@microchip.com> --- corelib/channel_curl.c | 42 +++++++++++++++++++++++++++++ examples/configuration/swupdate.cfg | 4 +-- 2 files changed, 44 insertions(+), 2 deletions(-)