[REVIEW] Implement filestore backend for certificate groups #6479
Conversation
NoelGraf
force-pushed
the
feat_gds_file_store
branch
3 times, most recently
from
871bc03
to
4d5ff96
Compare
|
Hi Noel, |
| if(!certBio) | ||
| return UA_STATUSCODE_BADINTERNALERROR; | ||
|
|
||
| X509 *cert = PEM_read_bio_X509(certBio, NULL, 0, NULL); |
There was a problem hiding this comment.
This looks rather restrictive. Why not allowing DER here too (like MbedTLS nicely does for us).
| UA_String trustedCrlFolder; | ||
| UA_String issuerCertFolder; | ||
| UA_String issuerCrlFolder; | ||
| UA_String rejectedCertFolder; |
There was a problem hiding this comment.
Persistence of rejected certificates is mainly a nice-to-have because it is not required by the spec. Nevertheless I just wanted to ask, if there are any limits applied on how many rejected certificates are kept?
CMakeLists.txt
Outdated
| @@ -1073,6 +1073,10 @@ list(APPEND plugin_sources | |||
| ${PROJECT_SOURCE_DIR}/plugins/crypto/openssl/ua_certificategroup_memorystore_openssl.c) | |||
| endif() | |||
|
|
|||
| if(UA_ENABLE_ENCRYPTION_MBEDTLS OR UA_ENABLE_ENCRYPTION_OPENSSL OR UA_ENABLE_ENCRYPTION_LIBRESSL OR UA_ENABLE_AMALGAMATION) | |||
There was a problem hiding this comment.
UA_ENABLE_ENCRYPTION could be enough to test.
NoelGraf
force-pushed
the
feat_gds_file_store
branch
from
c409016
to
b6b193f
Compare
|
Hi Marcel, |
|
Hi Noel, |
NoelGraf
force-pushed
the
feat_gds_file_store
branch
2 times, most recently
from
6156aaf
to
8dbfa41
Compare
NoelGraf
changed the title
NoelGraf
force-pushed
the
feat_gds_file_store
branch
3 times, most recently
from
148a6ab
to
fb57d66
Compare
| return 0; | ||
| } | ||
|
|
||
| char* tmp_dir = copyStr(dir); |
There was a problem hiding this comment.
copyStr is not needed.
It uses strlen internally - so the input to copyStr is already null-terminated.
The entire code is strange. mkpath calls itself recursively???
| /* Read files from directory */ | ||
| size_t numActCerts = 0; | ||
| dir = opendir(listPath); | ||
| if(dir) { |
There was a problem hiding this comment.
You can reset dir with rewinddir.
That cannot fail.
| if(dir) { | ||
| struct dirent *dirent; | ||
| while((dirent = readdir(dir)) != NULL) { | ||
| if(dirent->d_type == DT_REG) { |
There was a problem hiding this comment.
Use continue to skip instead of nested ifs.
| /* Check parameter */ | ||
| if (certGroup == NULL || trustList == NULL) { | ||
| return UA_STATUSCODE_BADINTERNALERROR; | ||
| } |
There was a problem hiding this comment.
Dont add { } guards for a single instruction-line after an if.
There was a problem hiding this comment.
Why not? It seems to be a bad habbit of not doing it and results in cleaner (here: unambiguous) code?
| static void | ||
| FileCertStore_clear(UA_CertificateGroup *certGroup) { | ||
| /* check parameter */ | ||
| if (certGroup == NULL) { |
There was a problem hiding this comment.
Do it as this:
if(!certGroup || !certGroup->context)
return;Then you also don't need the if-clause below.
…st and rejected list configurable
… for the creation of a thumprint of a certificate
…t returns the key length of a certificate
NoelGraf
force-pushed
the
feat_gds_file_store
branch
from
282e836
to
49fdd99
Compare
No description provided.