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.

Sign up for GitHub

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

implement ShowExtendedSystemInfo() in version.c #6149

Closed
wants to merge 1 commit into from
Closed

implement ShowExtendedSystemInfo() in version.c #6149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6615,6 +6615,18 @@ AS_IF([test "x$ENABLED_INTEL_QA" = "xyes" || test "x$ENABLED_INTEL_QA_SYNC" = "x
CPPFLAGS="$OLD_CPPFLAGS"
])

# Extended Version Info
AC_ARG_ENABLE([version-extended-info],
[AS_HELP_STRING([--enable-version-extended-info],[Enable extended version info (default: enabled)])],
[ ENABLED_VERSION_EXTENDED_INFO=$enableval ],
[ ENABLED_VERSION_EXTENDED_INFO=yes ]
)

if test "$VERSION_EXTENDED_INFO" = "no"
then
AM_CFLAGS="$AM_CFLAGS -DNO_VERSION_EXTENDED_INFO"
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably don't need this as I don't think you use NO_VERSION_EXTENDED_INFO anywhere. Better to have the affirmative form of options, so that we can avoid things like #ifndef NO_VERSION_ to check that the extended info exists.

fi

################################################################################
# Single Precision option handling #
################################################################################
Expand Down Expand Up @@ -8166,6 +8178,10 @@ AS_IF([test "x$ENABLED_NULL_CIPHER" = "xno" && \
[AM_CFLAGS="$AM_CFLAGS -DHAVE_NULL_CIPHER"
ENABLED_NULL_CIPHER=yes])

# Extended information (see version.c)
AS_IF([test "x$ENABLED_VERSION_EXTENDED_INFO" = "xyes"],
[AM_CFLAGS="$AM_CFLAGS -DHAVE_VERSION_EXTENDED_INFO"])

# wolfSSH and WPA Supplicant both need Public MP, only enable once.
# This will let you know if you enabled wolfSSH but have any of the prereqs
# disabled. Some of these options, disabling them adds things to the FLAGS and
Expand Down Expand Up @@ -8532,6 +8548,7 @@ AM_CONDITIONAL([BUILD_DTLS_CID],[test "x$ENABLED_DTLS_CID" = "xyes"])
AM_CONDITIONAL([BUILD_HPKE],[test "x$ENABLED_HPKE" = "xyes" || test "x$ENABLED_USERSETTINGS" = "xyes"])
AM_CONDITIONAL([BUILD_DTLS],[test "x$ENABLED_DTLS" = "xyes" || test "x$ENABLED_USERSETTINGS" = "xyes"])
AM_CONDITIONAL([BUILD_MAXQ10XX],[test "x$ENABLED_MAXQ10XX" = "xyes"])
AM_CONDITIONAL([BUILD_VERSION_EXTENDED_INFO],[test "x$VERSION_EXTENDED_INFO" != "xno" || test "x$ENABLED_USERSETTINGS" = "xyes"])

if test "$ENABLED_REPRODUCIBLE_BUILD" != "yes" &&
(test "$ax_enable_debug" = "yes" ||
Expand Down Expand Up @@ -8987,6 +9004,7 @@ echo " * PKCS#12: $ENABLED_PKCS12"
echo " * Cavium Nitrox: $ENABLED_CAVIUM"
echo " * Cavium Octeon (Sync): $ENABLED_OCTEON_SYNC"
echo " * Intel Quick Assist: $ENABLED_INTEL_QA"
echo " * Extended Version Info $ENABLED_VERSION_EXTENDED_INFO"
if test "$ENABLED_ARMASM_INLINE" = "yes"
then
ENABLED_ARMASM="inline C"
Expand Down
5 changes: 5 additions & 0 deletions src/include.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ EXTRA_DIST += src/pk.c
EXTRA_DIST += src/ssl_asn1.c
EXTRA_DIST += src/ssl_bn.c
EXTRA_DIST += src/ssl_misc.c
EXTRA_DIST += src/version.c
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a new files requires updating all the other build platforms like CMake, Visual Studio projects, etc. Please don't do this action unless a new file is absolutely required.

Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be a reason not to separate out functionality into smaller files. Our files are too big as it is. Instead, maybe update the other build platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is very ESP centric

Indeed this is very ESP centric at the moment, as I've been doing mostly Espressif development. My plan is to have an infrastructure in place to allow others to add their own platform-specific details as needed & desired. When doing so, and only as needed, the respective build & make platforms could be updated.

Although Espressif-specific now, I do see there being significant value in having a template of common system information for all platforms and all products. That would seem to be a valuable resource for customers to provide when asking for support.

It should be in the ESP specific code or even the ESP port code

I see your point here on the Espressif-specific details. I envisioned all of the "hardware / configuration overview" details being in a single file. As hardware typically has a version as well, that's another reason I was thinking of the version.c as a home. for this information.

If instead we had something way down in the ./port/expressif, what structure would you use? Perhaps a hardware specific ShowHardwareConfiguration() that is implemented for each platform, then called as appropriate from a single function elsewhere?

don't [add version.c] unless a new file is absolutely required

It was quite a learning curve to tease that file into the builds, so I definitely respect that comment. Still, iIntuitively for me, it seems to make sense to have this information in the version.c file. That seems to be a universal file & the most global across all wolfSSL libraries.

Why not extend the existing HAVE_WC_INTROSPECTION?

This is an excellent question. Although similar in concept, some of the information is mutually exclusive from introspection. @douzzer pointed out that: "HAVE_WC_INTROSPECTION requires that the configuration and build time artifacts, particularly the date/time of build and git parameters, be excluded from the build". For example: some of the information such as git hash & date may be updated, but the binary build itself should not actually change.

Given this, if you still feel strongly that I should not add the version.c, I'm happy to implement this functionality elsewhere. Any preference? Perhaps wolfcrypt/logging.c would be a appropriate? I certainly agree with @bandi13 about the big files. I'm in favor of more specific, smaller files when possible.

EXTRA_DIST += src/x509.c
EXTRA_DIST += src/x509_str.c

Expand Down Expand Up @@ -765,6 +766,10 @@ if BUILD_DTLS
src_libwolfssl@LIBSUFFIX@_la_SOURCES += src/dtls.c
endif

if BUILD_VERSION_EXTENDED_INFO
src_libwolfssl@LIBSUFFIX@_la_SOURCES += src/version.c
endif

endif !BUILD_CRYPTONLY


Expand Down
288 changes: 288 additions & 0 deletions src/version.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,288 @@
/* version.c
*
* Copyright (C) 2006-2023 wolfSSL Inc.
*
* This file is part of wolfSSL.
*
* wolfSSL is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* wolfSSL is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335, USA
*/
#include <wolfssl/version.h>


/*
** NOTICE: HAVE_WC_INTROSPECTION requires that the configuration and build time
** artifacts, particularly the date/time of build and git parameters, be
** excluded from the build. See #ifdef HAVE_WC_INTROSPECTION, below.
**
** Fundamentally: the object code needs to be maximally bitwise-invariant.
**
** Edit extended version information with care.
*/

#ifdef HAVE_VERSION_EXTENDED_INFO

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

#ifdef HAVE_CONFIG_H
#include <config.h>
#endif
#include <wolfssl/wolfcrypt/settings.h>

#if defined(WOLFSSL_ESPIDF)
#include <esp_log.h>
#include "sdkconfig.h"
const char* TAG = "Version Info";
#define WOLFSSL_VERSION_PRINTF(...) ESP_LOGI(TAG, __VA_ARGS__)

static int ShowExtendedSystemInfo_platform_espressif(void);
#else
#include <stdio.h>
#define WOLFSSL_VERSION_PRINTF(...) { printf(__VA_ARGS__); printf("\n"); }
#endif

static int ShowExtendedSystemInfo_git(void); /* may be limited during active introspection */

static int ShowExtendedSystemInfo_thread(void);
static int ShowExtendedSystemInfo_platform(void);

/*
*******************************************************************************
** Specific Platforms
*******************************************************************************
*/

/*
** Specific platforms: Espressif
*/
#if defined(WOLFSSL_ESPIDF)
static int ShowExtendedSystemInfo_platform_espressif(void)
{
#if defined(CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ)
WOLFSSL_VERSION_PRINTF("CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ: %u MHz",
CONFIG_ESP32_DEFAULT_CPU_FREQ_MHZ);
#endif

#if CONFIG_IDF_TARGET_ESP32

WOLFSSL_VERSION_PRINTF("Xthal_have_ccount: %u",
Xthal_have_ccount);

/* this is the legacy stack size */
#if defined(CONFIG_MAIN_TASK_STACK_SIZE)
WOLFSSL_VERSION_PRINTF("CONFIG_MAIN_TASK_STACK_SIZE: %d",
CONFIG_MAIN_TASK_STACK_SIZE);
#endif

/* this is the modern stack size */
#if defined(CONFIG_ESP_MAIN_TASK_STACK_SIZE)
WOLFSSL_VERSION_PRINTF("CONFIG_ESP_MAIN_TASK_STACK_SIZE: %d",
CONFIG_ESP_MAIN_TASK_STACK_SIZE);
#endif

#if defined(CONFIG_TIMER_TASK_STACK_SIZE)
WOLFSSL_VERSION_PRINTF("CONFIG_TIMER_TASK_STACK_SIZE: %d",
CONFIG_TIMER_TASK_STACK_SIZE);
#endif

#if defined(CONFIG_TIMER_TASK_STACK_DEPTH)
WOLFSSL_VERSION_PRINTF("CONFIG_TIMER_TASK_STACK_DEPTH: %d",
CONFIG_TIMER_TASK_STACK_DEPTH);
#endif

#if defined(SINGLE_THREADED)
/* see also HAVE_STACK_SIZE_VERBOSE */
char thisHWM = 0;
WOLFSSL_VERSION_PRINTF("Stack HWM: %x", (size_t) &thisHWM);
#else
WOLFSSL_VERSION_PRINTF("Stack HWM: %d",
uxTaskGetStackHighWaterMark(NULL));
#endif

#elif CONFIG_IDF_TARGET_ESP32S2
WOLFSSL_VERSION_PRINTF("Xthal_have_ccount = %u",
Xthal_have_ccount);
#elif CONFIG_IDF_TARGET_ESP32C6
/* not supported at this time */
#elif CONFIG_IDF_TARGET_ESP32C3
/* not supported at this time */
#elif CONFIG_IDF_TARGET_ESP32S3
WOLFSSL_VERSION_PRINTF("Xthal_have_ccount = %u",
Xthal_have_ccount);
#elif CONFIG_IDF_TARGET_ESP32H2
/* not supported at this time */
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't put in unnecessary conditions. When we add support we can add those targets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed. changed logic to known supported targets, everything else is an error

#elif CONFIG_IDF_TARGET_ESP32C2
/* not supported at this time */
#else
/* not supported at this time */
#endif

/* check to see if we are using hardware encryption */
#if defined(NO_ESP32WROOM32_CRYPT)
WOLFSSL_VERSION_PRINTF("NO_ESP32WROOM32_CRYPT defined! "
"HW acceleration DISABLED.");
#else
/* first show what platform hardware acceleration is enabled
** (some new platforms may not be supported yet) */
#if defined(CONFIG_IDF_TARGET_ESP32)
WOLFSSL_VERSION_PRINTF("ESP32WROOM32_CRYPT is enabled for ESP32.");
#elif defined(CONFIG_IDF_TARGET_ESP32S2)
WOLFSSL_VERSION_PRINTF("ESP32WROOM32_CRYPT is enabled for ESP32-S2.");
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
WOLFSSL_VERSION_PRINTF("ESP32WROOM32_CRYPT is enabled for ESP32-S3.");
#else
#error "ESP32WROOM32_CRYPT not yet supported on this IDF TARGET"
#endif

/* Even though enabled, some specifics may be disabled */
#if defined(NO_WOLFSSL_ESP32WROOM32_CRYPT_HASH)
WOLFSSL_VERSION_PRINTF("NO_WOLFSSL_ESP32WROOM32_CRYPT_HASH is defined!"
"(disabled HW SHA).");
#endif

#if defined(NO_WOLFSSL_ESP32WROOM32_CRYPT_AES)
WOLFSSL_VERSION_PRINTF("NO_WOLFSSL_ESP32WROOM32_CRYPT_AES is defined!"
"(disabled HW AES).");
#endif

#if defined(NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI)
WOLFSSL_VERSION_PRINTF("NO_WOLFSSL_ESP32WROOM32_CRYPT_RSA_PRI defined!"
"(disabled HW RSA)");
#endif
#endif

return 0;
}
#endif

/*
*******************************************************************************
** All Platforms
*******************************************************************************
*/

/*
** All platforms: git details
*/
static int ShowExtendedSystemInfo_git(void)
{
#ifdef HAVE_WC_INTROSPECTION
WOLFSSL_VERSION_PRINTF("HAVE_WC_INTROSPECTION enabled. "
"Some extended system details not available.");
#else
/* Display some interesting git values that may change,
** but not desired for introspection which requires object code to be
** maximally bitwise-invariant.
*/
#if defined(LIBWOLFSSL_VERSION_GIT_ORIGIN)
/* git config --get remote.origin.url */
WOLFSSL_VERSION_PRINTF("LIBWOLFSSL_VERSION_GIT_ORIGIN = %s",
LIBWOLFSSL_VERSION_GIT_ORIGIN);
#endif

#if defined(LIBWOLFSSL_VERSION_GIT_BRANCH)
/* git rev-parse --abbrev-ref HEAD */
WOLFSSL_VERSION_PRINTF("LIBWOLFSSL_VERSION_GIT_BRANCH = %s",
LIBWOLFSSL_VERSION_GIT_BRANCH);
#endif

#if defined(LIBWOLFSSL_VERSION_GIT_HASH)
WOLFSSL_VERSION_PRINTF("LIBWOLFSSL_VERSION_GIT_HASH = %s",
LIBWOLFSSL_VERSION_GIT_HASH);
#endif

#if defined(LIBWOLFSSL_VERSION_GIT_SHORT_HASH )
WOLFSSL_VERSION_PRINTF("LIBWOLFSSL_VERSION_GIT_SHORT_HASH = %s",
LIBWOLFSSL_VERSION_GIT_SHORT_HASH);
#endif

#if defined(LIBWOLFSSL_VERSION_GIT_HASH_DATE)
WOLFSSL_VERSION_PRINTF("LIBWOLFSSL_VERSION_GIT_HASH_DATE = %s",
LIBWOLFSSL_VERSION_GIT_HASH_DATE);
#endif

#endif /* else not HAVE_WC_INTROSPECTION */
return 0;
}

/*
** All platforms: thread details
*/
static int ShowExtendedSystemInfo_thread(void)
{
/* all platforms: stack high water mark check */
#if defined(SINGLE_THREADED)
WOLFSSL_VERSION_PRINTF("SINGLE_THREADED");
#else
WOLFSSL_VERSION_PRINTF("NOT SINGLE_THREADED");
#endif
return 0;
}

/*
** All Platforms: platform details
*/
static int ShowExtendedSystemInfo_platform(void)
{
#if defined(WOLFSSL_ESPIDF)
#if defined(CONFIG_IDF_TARGET)
WOLFSSL_VERSION_PRINTF("CONFIG_IDF_TARGET = %s",
CONFIG_IDF_TARGET);
ShowExtendedSystemInfo_platform_espressif(void);
#endif
#endif
return 0;
}

/*
*******************************************************************************
** The public ShowExtendedSystemInfo()
*******************************************************************************
*/

#ifdef __cplusplus
extern "C" {
#endif

int ShowExtendedSystemInfo(void)
{
WOLFSSL_VERSION_PRINTF("Extended Version and Platform Information.");

#if defined(LIBWOLFSSL_VERSION_STRING)
WOLFSSL_VERSION_PRINTF("LIBWOLFSSL_VERSION_STRING = %s",
LIBWOLFSSL_VERSION_STRING);
#endif

#if defined(LIBWOLFSSL_VERSION_HEX)
WOLFSSL_VERSION_PRINTF("LIBWOLFSSL_VERSION_HEX = %x",
LIBWOLFSSL_VERSION_HEX);
#endif

#if defined(WOLFSSL_MULTI_INSTALL_WARNING)
/* CMake may have detected undesired multiple installs, so give warning. */
WOLFSSL_VERSION_PRINTF("");
WOLFSSL_VERSION_PRINTF("WARNING: Multiple wolfSSL installs found.");
WOLFSSL_VERSION_PRINTF("Check ESP-IDF and local project [components] directory.");
WOLFSSL_VERSION_PRINTF("");
#endif

ShowExtendedSystemInfo_git(); /* may be limited during active introspection */
ShowExtendedSystemInfo_platform();
ShowExtendedSystemInfo_thread();
return 0;
}

#endif /* NO_VERSION_EXTENDED_INFO */

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra empty lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

#ifdef __cplusplus
Copy link
Contributor

Choose a reason for hiding this comment

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

The __cplusplus logic only goes in the .h. It is already there, just remove in .c.

}
#endif
Loading