Skip to content

Commit

Permalink
treewide: remove SECVAR_CRYPRO_WRITE_FUNC
Browse files Browse the repository at this point in the history
Originally, SECVAR_CRYPTO_WRITE_FUNC was intended to be a compile-time
flag to remove any generation code, to reduce the resulting binary file
size to be as small as possible for management-only uses (such as
in embedded firmware environment, like Petitboot).

Builds like these never came about, and this option only made builds more
complex and error prone, so remove it.

Signed-off-by: Eric Richter <[email protected]>
  • Loading branch information
erichte-ibm committed Oct 10, 2024
1 parent c863fa4 commit 9b65f4c
Show file tree
Hide file tree
Showing 14 changed files with 10 additions and 68 deletions.
1 change: 0 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ set( SECVARCTL_VERSION "${SECVARCTL_VERSION_MAJOR}.${SECVARCTL_VERSION_MINOR}.${
message( STATUS "Detected version string as ${SECVARCTL_VERSION}" )

target_compile_definitions( secvarctl PRIVATE
SECVAR_CRYPTO_WRITE_FUNC
SECVARCTL_VERSION=\"${SECVARCTL_VERSION}\"
)

Expand Down
9 changes: 1 addition & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,6 @@ INCLUDES = -I. \
-I./external/libstb-secvar/include/secvar \
-I./external/libstb-secvar/external

#use CRYPTO_READ_ONLY for smaller executable but limited functionality
#removes all write functions (secvarctl generate, pem_to_der etc.)
CRYPTO_READ_ONLY = 0
ifeq ($(strip $(CRYPTO_READ_ONLY)), 0)
_CFLAGS += -DSECVAR_CRYPTO_WRITE_FUNC
endif

# Set build version string
include VERSION
_CFLAGS += -DSECVARCTL_VERSION=\"$(SECVARCTL_VERSION)\"
Expand Down Expand Up @@ -141,7 +134,7 @@ $(BIN_DIR)/secvarctl-dbg: $(OBJDBG) $(LIBSTB_SECVAR)
$(CC) $(_CFLAGS) -g $^ $(STATICFLAG) $(SANITIZE_FLAGS) -fprofile-arcs -ftest-coverage -o $@ $(_LDFLAGS)

$(LIBSTB_SECVAR):
$(MAKE) CFLAGS=-DSECVAR_CRYPTO_WRITE_FUNC -C $(LIBSTB_SECVAR_ROOT) lib/$(LIBSTB_SECVAR_ARCHIVE)
$(MAKE) -C $(LIBSTB_SECVAR_ROOT) lib/$(LIBSTB_SECVAR_ARCHIVE)


secvarctl: $(BIN_DIR)/secvarctl
Expand Down
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ Being that the key management process is rather lengthy and difficult, `secvarct
| Static Build | `By Default Static build` | `By Default Static build` |
| Dynamic Build | `DYNAMIC_LIB=1` | `-DDYNAMIC_LIB=1` |
| Reduced Size Build | `default` | `-DSTRIP=1` |
| Build Without Crypto Write Functions | `CRYPTO_READ_ONLY=1` | `-CRYPTO_READ_ONLY=1` |
| Build W Specific Mbedtls Library | `CFLAGS="-I<path>/include" LDFLAGS="-L<path>/library"` | `-DCUSTOM_MBEDTLS=<path>` |
| Build for Coverage Tests | `make [options] coverage` | `-DCoverage=1` |
| Build W Debug Symbols | `make DEBUG=1` | `default` |
Expand Down
10 changes: 1 addition & 9 deletions backends/guest/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ else
_LDFLAGS += -s
endif

#use CRYPTO_READ_ONLY for smaller executable but limited functionality
#removes all write functions (secvarctl generate, pem_to_der etc.)
CRYPTO_READ_ONLY = 0
ifeq ($(strip $(CRYPTO_READ_ONLY)), 0)
_CFLAGS += -DSECVAR_CRYPTO_WRITE_FUNC
endif

DYNAMIC_LIB = 0
STATIC_LIB = 1
ifeq ($(strip $(DYNAMIC_LIB)), 1)
Expand Down Expand Up @@ -77,8 +70,7 @@ endif

libstb-secvar:
@mkdir -p $(LIB_DIR)
@$(MAKE) -C $(GUEST_EXTERNAL_BACKEND_DIR) LIB_DIR=$(LIB_DIRS) STATIC_LIB=$(STATIC_LIB) \
CRYPTO_READ_ONLY=$(CRYPTO_READ_ONLY)
@$(MAKE) -C $(GUEST_EXTERNAL_BACKEND_DIR) LIB_DIR=$(LIB_DIRS) STATIC_LIB=$(STATIC_LIB)

clean:
@$(MAKE) -C $(GUEST_EXTERNAL_BACKEND_DIR) clean
Expand Down
7 changes: 0 additions & 7 deletions backends/guest/common/validate.c
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,8 @@ int validate_cert(const uint8_t *cert_data, size_t cert_data_len, bool is_CA)
{
int rc;
crypto_x509_t *x509;
#ifdef SECVAR_CRYPTO_WRITE_FUNC
uint8_t *cert = NULL;
size_t cert_size = 0;
#endif

x509 = crypto_x509_parse_der(cert_data, cert_data_len);
if (!x509) {
Expand All @@ -317,7 +315,6 @@ int validate_cert(const uint8_t *cert_data, size_t cert_data_len, bool is_CA)
* check if we have compiled with pkcs7_write functions
* if so we can try to convert pem to der and try again
*/
#ifdef SECVAR_CRYPTO_WRITE_FUNC
prlog(PR_INFO, "failed to parse x509 as DER, trying PEM...\n");
rc = crypto_convert_pem_to_der(cert_data, cert_data_len, &cert, &cert_size);
if (rc != CRYPTO_SUCCESS) {
Expand All @@ -330,10 +327,6 @@ int validate_cert(const uint8_t *cert_data, size_t cert_data_len, bool is_CA)
free(cert);
if (!x509)
return SV_X509_PARSE_ERROR;
#else
prlog(PR_INFO, "ERROR: failed to parse x509. Make sure file is in DER not PEM\n");
return SV_X509_PARSE_ERROR;
#endif
}

rc = validate_x509_certificate(x509);
Expand Down
2 changes: 0 additions & 2 deletions backends/guest/guest_svc_generate.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* SPDX-License-Identifier: Apache-2.0
* Copyright 2022-2023 IBM Corp.
*/
#ifdef SECVAR_CRYPTO_WRITE_FUNC
#include <string.h>
#include <argp.h>
#include "err.h"
Expand Down Expand Up @@ -753,4 +752,3 @@ int guest_generate_command(int argc, char *argv[])

return rc;
}
#endif
10 changes: 1 addition & 9 deletions backends/guest/guest_svc_read.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ static int read_cert(const uint8_t *cert_data, const size_t cert_data_len, const
* check if we have compiled with pkcs7_write functions
* if so we can try to convert pem to der and try again
*/
#ifdef SECVAR_CRYPTO_WRITE_FUNC
uint8_t *cert;
size_t cert_size;
prlog(PR_INFO, "failed to parse x509 as DER, trying PEM...\n");
Expand All @@ -117,10 +116,6 @@ static int read_cert(const uint8_t *cert_data, const size_t cert_data_len, const
free(cert);
if (!x509)
return SV_X509_PARSE_ERROR;
#else
prlog(PR_INFO, "ERROR: failed to parse x509. Make sure file is in DER not PEM\n");
return SV_X509_PARSE_ERROR;
#endif
}

rc = validate_x509_certificate(x509);
Expand Down Expand Up @@ -479,7 +474,4 @@ struct command guest_command_table[] = { { .name = "read", .func = guest_read_co
{ .name = "write", .func = guest_write_command },
{ .name = "validate", .func = guest_validation_command },
{ .name = "verify", .func = guest_verify_command },
#ifdef SECVAR_CRYPTO_WRITE_FUNC
{ .name = "generate", .func = guest_generate_command }
#endif
};
{ .name = "generate", .func = guest_generate_command } };
2 changes: 0 additions & 2 deletions backends/host/host_svc_generate.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// SPDX-License-Identifier: Apache-2.0
/* Copyright 2021 IBM Corp.*/
#ifdef SECVAR_CRYPTO_WRITE_FUNC
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
Expand Down Expand Up @@ -1125,4 +1124,3 @@ static int toAuth(const unsigned char *newESL, size_t eslSize, struct Arguments

return rc;
}
#endif
2 changes: 0 additions & 2 deletions backends/host/host_svc_read.c
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,5 @@ struct command edk2_compat_command_table[] = {
{ .name = "write", .func = performWriteCommand },
{ .name = "validate", .func = performValidation },
{ .name = "verify", .func = performVerificationCommand },
#ifdef SECVAR_CRYPTO_WRITE_FUNC
{ .name = "generate", .func = performGenerateCommand }
#endif
};
17 changes: 4 additions & 13 deletions backends/host/host_svc_validate.c
Original file line number Diff line number Diff line change
Expand Up @@ -555,11 +555,9 @@ static int validateCertStruct(crypto_x509_t *x509, const char *varName)
return SUCCESS;
}

#ifdef SECVAR_CRYPTO_WRITE_FUNC
/*
*This function is just an extension of parseX509
*It allows us to declare new variables at the start of the function rather than the middle
*It is dependent on crypto_convert_pem_to_der which is dependent on SECVAR_CRYPTO_WRITE_FUNC
*/
static crypto_x509_t *parseX509_PEM(const unsigned char *data_pem, size_t data_len)
{
Expand All @@ -577,7 +575,6 @@ static crypto_x509_t *parseX509_PEM(const unsigned char *data_pem, size_t data_l

return x509;
}
#endif

/**
*parses x509 certficate buffer (PEM or DER) into certificate struct
Expand All @@ -604,23 +601,17 @@ int parseX509(crypto_x509_t **x509, const unsigned char *certBuf, size_t buflen)
*x509 = crypto_x509_parse_der(certBuf, buflen);
if (*x509)
return SUCCESS;
/*
*if here, parsing cert in der failed
*check if we have compiled with pkcs7_write functions
*if so we can try to convert pem to der and try again
*/
#ifdef SECVAR_CRYPTO_WRITE_FUNC
/*
*if here, parsing cert in der failed
*try to convert pem to der and try again
*/
prlog(PR_INFO, "Failed to parse x509 as DER, trying PEM...\n");
// if failed, maybe input is PEM and so try converting PEM to DER, if conversion fails then we know it was DER and it failed
*x509 = parseX509_PEM(certBuf, buflen);
if (!x509) {
prlog(PR_ERR, "ERROR: Failed to parse x509 (tried DER and PEM formats). \n");
return CERT_FAIL;
}
#else
prlog(PR_INFO, "ERROR: Failed to parse x509. Make sure file is in DER not PEM\n");
return CERT_FAIL;
#endif

return SUCCESS;
}
Expand Down
2 changes: 0 additions & 2 deletions external/skiboot/libstb/secvar/crypto/crypto-gnutls.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ int crypto_pkcs7_signed_hash_verify(crypto_pkcs7_t *pkcs7, crypto_x509_t *x509,
}


#ifdef SECVAR_CRYPTO_WRITE_FUNC
/*
*generates a PKCS7 and create signature with private and public keys
*@param pkcs7, the resulting PKCS7 DER buff, newData not appended, NOTE: REMEMBER TO UNALLOC THIS MEMORY
Expand Down Expand Up @@ -413,7 +412,6 @@ int crypto_convert_pem_to_der(const unsigned char *input, size_t ilen,
return rc;
}

#endif

/**====================X509 Functions ====================**/
int crypto_x509_get_der_len(crypto_x509_t *x509)
Expand Down
6 changes: 1 addition & 5 deletions external/skiboot/libstb/secvar/crypto/crypto-mbedtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,10 @@
#include <mbedtls/oid.h>
// Nick Child edited paths below
// #include "libstb/crypto/pkcs7/pkcs7.h"
// #ifdef SECVAR_CRYPTO_WRITE_FUNC
// #include "libstb/crypto/pkcs7/pkcs7_write.h"
// #endif
#include "external/extraMbedtls/include/pkcs7.h"
#ifdef SECVAR_CRYPTO_WRITE_FUNC
#include "external/extraMbedtls/include/pkcs7_write.h"
#endif
#include <mbedtls/platform.h>

crypto_pkcs7_t *crypto_pkcs7_parse_der(const unsigned char *buf, const int buflen)
Expand Down Expand Up @@ -86,7 +83,6 @@ int crypto_pkcs7_signed_hash_verify(crypto_pkcs7_t *pkcs7, crypto_x509_t *x509,
return mbedtls_pkcs7_signed_hash_verify(pkcs7, x509, hash, hash_len);
}

#ifdef SECVAR_CRYPTO_WRITE_FUNC
int crypto_pkcs7_generate_w_signature(unsigned char **pkcs7, size_t *pkcs7Size,
const unsigned char *newData,
size_t newDataSize, const char **crtFiles,
Expand Down Expand Up @@ -232,7 +228,7 @@ int crypto_convert_pem_to_der(const unsigned char *input, size_t ilen,
{
return mbedtls_convert_pem_to_der(input, ilen, output, olen);
}
#endif

int crypto_x509_get_der_len(crypto_x509_t *x509)
{
return x509->raw.len;
Expand Down
7 changes: 1 addition & 6 deletions secvarctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,9 @@ void usage()
"verify\t\tcompares proposed variable to the current "
"variables,\n\t\t\t"
"use 'secvarctl [MODE] verify --usage/help' for more information\n"
#ifdef SECVAR_CRYPTO_WRITE_FUNC
"\tgenerate\tcreates relevant files for secure variable "
"management,\n\t\t\t"
"use 'secvarctl [MODE] generate --usage/help' for more information\n"
#endif
"\n");
enabled_backends();
}
Expand All @@ -110,11 +108,8 @@ void help()
"type\n\t\t"
"verify - checks that the given files are correctly signed by the "
"current variables\n"
#ifdef SECVAR_CRYPTO_WRITE_FUNC
"\t\tgenerate - create files that are relevant to the secure "
"variable management process\n"
#endif
);
"variable management process\n");

usage();
}
Expand Down

0 comments on commit 9b65f4c

Please sign in to comment.