Skip to content

Commit 6f79820

Browse files
SparkiDevSparkiDev
authored
Merge pull request #152 from padelsbach/wp_dh_get_params_fix
Add tests for DH get params and match OSSL behavior
2 parents 664b75a + 17232fa commit 6f79820

File tree

4 files changed

+148
-40
lines changed

4 files changed

+148
-40
lines changed

‎scripts/test-wp-cs.sh‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
2222
NUMCPU=${NUMCPU:-8}
2323
WOLFPROV_DEBUG=${WOLFPROV_DEBUG:-0}
2424
source ${SCRIPT_DIR}/utils-wolfprovider.sh
25+
source ${SCRIPT_DIR}/utils-openssl.sh
2526

2627
CERT_DIR=$SCRIPT_DIR/../certs
2728
LOG_FILE=$SCRIPT_DIR/test-wp-cs.log
@@ -225,6 +226,20 @@ CURVES=prime256v1
225226
OPENSSL_ALL_CIPHERS="-cipher ALL -ciphersuites $TLS13_ALL_CIPHERS"
226227
OPENSSL_PORT=$(generate_port)
227228

229+
# ensure we are doing a clean build
230+
printf "Cleaning up previous builds"
231+
rm -rf ${SCRIPT_DIR}/../*-install
232+
if [ -d ${OPENSSL_SOURCE_DIR} ]; then
233+
pushd ${OPENSSL_SOURCE_DIR} > /dev/null
234+
git clean -xdf > /dev/null 2>&1
235+
popd > /dev/null
236+
fi
237+
if [ -d ${WOLFSSL_SOURCE_DIR} ]; then
238+
pushd ${WOLFSSL_SOURCE_DIR} > /dev/null
239+
git clean -xdf > /dev/null 2>&1
240+
popd > /dev/null
241+
fi
242+
228243
init_wolfprov
229244

230245
if [ "${AM_BWRAPPED-}" != "yes" ]; then

‎scripts/utils-openssl.sh‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,13 @@
2525
SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
2626
source ${SCRIPT_DIR}/utils-general.sh
2727

28-
OPENSSL_GIT="https://github.com/openssl/openssl.git"
28+
OPENSSL_GIT_URL="https://github.com/openssl/openssl.git"
2929
OPENSSL_TAG=${OPENSSL_TAG:-"openssl-3.5.0"}
3030
OPENSSL_SOURCE_DIR=${SCRIPT_DIR}/../openssl-source
3131
OPENSSL_INSTALL_DIR=${SCRIPT_DIR}/../openssl-install
32+
OPENSSL_BIN=${OPENSSL_INSTALL_DIR}/bin/openssl
33+
OPENSSL_TEST=${OPENSSL_SOURCE_DIR}/test
34+
OPENSSL_LIB_DIRS="${OPENSSL_INSTALL_DIR}/lib:${OPENSSL_INSTALL_DIR}/lib64"
3235

3336
NUMCPU=${NUMCPU:-8}
3437
WOLFPROV_DEBUG=${WOLFPROV_DEBUG:-0}
@@ -45,15 +48,17 @@ clone_openssl() {
4548
fi
4649

4750
if [ ! -d ${OPENSSL_SOURCE_DIR} ]; then
51+
printf "\tOpenSSL source directory not found: ${OPENSSL_SOURCE_DIR}\n"
52+
printf "\tParent directory:\n"
53+
tree -L 2 $(dirname ${OPENSSL_SOURCE_DIR}/..) || true
4854
CLONE_TAG=${USE_CUR_TAG:+${OPENSSL_TAG_CUR}}
4955
CLONE_TAG=${CLONE_TAG:-${OPENSSL_TAG}}
5056

51-
printf "\tClone OpenSSL ${CLONE_TAG} ... "
52-
5357
DEPTH_ARG=${WOLFPROV_DEBUG:+""}
5458
DEPTH_ARG=${DEPTH_ARG:---depth=1}
5559

56-
git clone ${DEPTH_ARG} -b ${CLONE_TAG} ${OPENSSL_GIT} ${OPENSSL_SOURCE_DIR} >>$LOG_FILE 2>&1
60+
printf "\tClone OpenSSL ${CLONE_TAG} from ${OPENSSL_GIT_URL} ... "
61+
git clone ${DEPTH_ARG} -b ${CLONE_TAG} ${OPENSSL_GIT_URL} ${OPENSSL_SOURCE_DIR}
5762
RET=$?
5863

5964
if [ $RET != 0 ]; then
@@ -62,10 +67,23 @@ clone_openssl() {
6267
exit 1
6368
fi
6469
printf "Done.\n"
70+
71+
printf "\tOpenSSL source cloned to: ${OPENSSL_SOURCE_DIR}\n"
72+
if [ ! -d ${OPENSSL_SOURCE_DIR} ]; then
73+
printf "ERROR: OpenSSL source directory not found after clone: ${OPENSSL_SOURCE_DIR}\n"
74+
fi
75+
else
76+
printf "\tOpenSSL source directory exists: ${OPENSSL_SOURCE_DIR}\n"
77+
if [ ! -d ${OPENSSL_SOURCE_DIR}/.git ]; then
78+
printf "ERROR: OpenSSL source directory is not a git repository: ${OPENSSL_SOURCE_DIR}\n"
79+
do_cleanup
80+
exit 1
81+
fi
6582
fi
6683
}
6784

6885
install_openssl() {
86+
printf "\nInstalling OpenSSL ${OPENSSL_TAG} ..."
6987
clone_openssl
7088
cd ${OPENSSL_SOURCE_DIR}
7189

@@ -114,10 +132,6 @@ init_openssl() {
114132
install_openssl
115133
printf "\tOpenSSL ${OPENSSL_TAG} installed in: ${OPENSSL_INSTALL_DIR}\n"
116134

117-
OPENSSL_BIN=${OPENSSL_INSTALL_DIR}/bin/openssl
118-
OPENSSL_TEST=${OPENSSL_SOURCE_DIR}/test
119-
OPENSSL_LIB_DIRS="${OPENSSL_INSTALL_DIR}/lib:${OPENSSL_INSTALL_DIR}/lib64"
120-
121135
OSSL_VER=`LD_LIBRARY_PATH=${OPENSSL_LIB_DIRS} $OPENSSL_BIN version | tail -n1`
122136
case $OSSL_VER in
123137
OpenSSL\ 3.*) ;;

‎src/wp_dh_kmgmt.c‎

Lines changed: 107 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -685,21 +685,26 @@ static int wp_dh_get_params_encoded_public_key(wp_Dh* dh, OSSL_PARAM params[])
685685

686686
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY);
687687
if (p != NULL) {
688-
size_t outLen = mp_unsigned_bin_size(&dh->key.p);
688+
if (p->data_type != OSSL_PARAM_OCTET_STRING) {
689+
ok = 0;
690+
}
691+
if (ok) {
692+
size_t outLen = mp_unsigned_bin_size(&dh->key.p);
689693

690-
if (p->data != NULL) {
691-
if (p->data_size < outLen) {
692-
ok = 0;
693-
}
694-
if (ok) {
695-
unsigned char* data = p->data;
696-
size_t padSz = outLen - dh->pubSz;
697-
/* Front pad with zeros. */
698-
XMEMSET(data, 0, padSz);
699-
XMEMCPY(data + padSz, dh->pub, dh->pubSz);
694+
if (p->data != NULL) {
695+
if (p->data_size < outLen) {
696+
ok = 0;
697+
}
698+
if (ok) {
699+
unsigned char* data = p->data;
700+
size_t padSz = outLen - dh->pubSz;
701+
/* Front pad with zeros. */
702+
XMEMSET(data, 0, padSz);
703+
XMEMCPY(data + padSz, dh->pub, dh->pubSz);
704+
}
700705
}
706+
p->return_size = outLen;
701707
}
702-
p->return_size = outLen;
703708
}
704709

705710
WOLFPROV_LEAVE(WP_LOG_KE, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
@@ -719,35 +724,107 @@ static int wp_dh_get_params(wp_Dh* dh, OSSL_PARAM params[])
719724
int ok = 1;
720725
OSSL_PARAM* p;
721726

722-
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_MAX_SIZE);
723-
if ((p != NULL) && !OSSL_PARAM_set_int(p,
724-
mp_unsigned_bin_size(&dh->key.p))) {
725-
ok = 0;
727+
if (ok) {
728+
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_MAX_SIZE);
729+
if (p != NULL) {
730+
if (!OSSL_PARAM_set_uint(p, mp_unsigned_bin_size(&dh->key.p))) {
731+
ok = 0;
732+
}
733+
}
726734
}
727735
if (ok) {
728736
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_BITS);
729-
if ((p != NULL) && !OSSL_PARAM_set_int(p, dh->bits)) {
730-
ok = 0;
737+
if (p != NULL) {
738+
if (!OSSL_PARAM_set_int(p, dh->bits)) {
739+
ok = 0;
740+
}
731741
}
732742
}
733743
if (ok) {
734744
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_SECURITY_BITS);
735-
if ((p != NULL) && (!OSSL_PARAM_set_int(p,
736-
wp_dh_get_security_bits(dh)))) {
737-
ok = 0;
745+
if (p != NULL) {
746+
if (!OSSL_PARAM_set_int(p, wp_dh_get_security_bits(dh))) {
747+
ok = 0;
748+
}
738749
}
739750
}
740-
if (ok && (!wp_params_set_mp(params, OSSL_PKEY_PARAM_FFC_P,
741-
&dh->key.p, 1))) {
742-
ok = 0;
751+
if (ok) {
752+
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_FFC_P);
753+
if (p != NULL) {
754+
/* When buffer is NULL, return the size irrespective of type */
755+
if (p->data == NULL) {
756+
ok = wp_params_set_mp(params, OSSL_PKEY_PARAM_FFC_P, &dh->key.g, 1);
757+
}
758+
/* When buffer is non-NULL, type must be int or uint */
759+
else
760+
if (p->data_type == OSSL_PARAM_INTEGER ||
761+
p->data_type == OSSL_PARAM_UNSIGNED_INTEGER) {
762+
ok = wp_params_set_mp(params, OSSL_PKEY_PARAM_FFC_P, &dh->key.p, 1);
763+
}
764+
else {
765+
ok = 0;
766+
}
767+
}
743768
}
744-
if (ok && (!wp_params_set_mp(params, OSSL_PKEY_PARAM_FFC_G,
745-
&dh->key.g, 1))) {
746-
ok = 0;
769+
if (ok) {
770+
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_FFC_G);
771+
if (p != NULL) {
772+
/* When buffer is NULL, return the size irrespective of type */
773+
if (p->data == NULL) {
774+
ok = wp_params_set_mp(params, OSSL_PKEY_PARAM_FFC_G, &dh->key.g, 1);
775+
}
776+
/* When buffer is non-NULL, type must be int or uint */
777+
else if (p->data_type == OSSL_PARAM_INTEGER ||
778+
p->data_type == OSSL_PARAM_UNSIGNED_INTEGER) {
779+
ok = wp_params_set_mp(params, OSSL_PKEY_PARAM_FFC_G, &dh->key.g, 1);
780+
}
781+
else {
782+
ok = 0;
783+
}
784+
}
747785
}
748-
if (ok && (!wp_params_set_octet_string_be(params, OSSL_PKEY_PARAM_PUB_KEY,
749-
dh->pub, dh->pubSz))) {
750-
ok = 0;
786+
if (ok) {
787+
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_FFC_Q);
788+
if (p != NULL) {
789+
/* OSSL does not check the type */
790+
ok = wp_params_set_mp(params, OSSL_PKEY_PARAM_FFC_Q, &dh->key.q, 1);
791+
}
792+
}
793+
if (ok) {
794+
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_PUB_KEY);
795+
if (p != NULL) {
796+
if (p->data == NULL) {
797+
p->return_size = dh->pubSz;
798+
}
799+
else {
800+
/* return_size is set within this function */
801+
ok = wp_params_set_octet_string_be(params, OSSL_PKEY_PARAM_PUB_KEY,
802+
dh->pub, dh->pubSz);
803+
}
804+
}
805+
}
806+
if (ok) {
807+
p = OSSL_PARAM_locate(params, OSSL_PKEY_PARAM_PRIV_KEY);
808+
if (p != NULL) {
809+
if (p->data == NULL) {
810+
p->return_size = dh->pubSz;
811+
}
812+
else if (p->data_type == OSSL_PARAM_UNSIGNED_INTEGER) {
813+
if (p->data_size < dh->privSz) {
814+
ok = 0;
815+
}
816+
else {
817+
/* OSSL returns a BIGNUM, but we copy raw bytes*/
818+
XMEMCPY(p->data, dh->priv, dh->privSz);
819+
p->return_size = dh->privSz;
820+
}
821+
}
822+
else {
823+
/* return_size is set within this function */
824+
ok = wp_params_set_octet_string_be(params, OSSL_PKEY_PARAM_PRIV_KEY,
825+
dh->priv, dh->privSz);
826+
}
827+
}
751828
}
752829
if (ok && (!wp_params_set_octet_string_be(params, OSSL_PKEY_PARAM_PRIV_KEY,
753830
dh->priv, dh->privSz))) {

‎src/wp_params.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,8 +639,10 @@ int wp_params_set_octet_string_be(OSSL_PARAM params[], const char* key,
639639
OSSL_PARAM* p;
640640

641641
p = OSSL_PARAM_locate(params, key);
642-
if ((p != NULL) && (p->data_size < len)) {
643-
ok = 0;
642+
if (p != NULL) {
643+
if ((p->data == NULL) || (p->data_size < len)) {
644+
ok = 0;
645+
}
644646
}
645647
if ((p != NULL) && ok) {
646648
#ifdef LITTLE_ENDIAN_ORDER

0 commit comments

Comments
 (0)