Make the Makefile more robust #267
Conversation
required directories are present. Signed-off-by: Juan del Cuvillo <[email protected]>
fixed. Signed-off-by: Juan del Cuvillo <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR makes the Makefile more robust by adding conditional checks to prevent errors when OpenSSL tarball files or required directories are missing. The changes ensure that FIPS-related operations only execute when the necessary OpenSSL 3.1 tarball is present and improves the version detection logic.
- Enhanced OpenSSL version detection with error handling and improved sorting
- Added conditional guards around FIPS operations to prevent execution without required files
- Removed outdated OpenSSL 3.1.6 download from CI workflow
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openssl_source/Makefile | Added conditional checks and improved OpenSSL version detection to prevent errors when tarballs or directories are missing |
| .github/workflows/codeql.yml | Removed download of OpenSSL 3.1.6 tarball that is no longer needed |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| ifeq ($(OSSL_FIPS_SOURCE_DIR_SET), 1) | ||
| @rm -rf $(OSSL_FIPS_SOURCE_DIR)/ | ||
| @rm -rf $(OSSL_FIPS_INSTALL_DIR)/ | ||
| @rm -rf $(OSSL_FIPS_BUILD_DIR)/ | ||
| endif |
There was a problem hiding this comment.
The conditional check uses OSSL_FIPS_SOURCE_DIR_SET which is never defined in the code. This variable is not set anywhere in the Makefile, so the condition will always be false. The check should likely use OSSL_FIPS_BUILD_DIR_SET or OSSL_FIPS_INSTALL_DIR_SET instead, or a new variable OSSL_FIPS_SOURCE_DIR_SET should be defined similar to lines 48-49.
There was a problem hiding this comment.
You're right. Initially, I was planning to use one of the defined variables OSSL_FIPS_BUILD_DIR_SET or OSSL_FIPS_INSTALL_DIR_SET. However, since I'm using OSSL_FIPS_SOURCE_DIR_SET, I pushed a commit that defines it.
Signed-off-by: Juan del Cuvillo <[email protected]>
Update Makefile to execute recipes only when OpenSSL tarball and/or required directories are present so it doesn't give an error when executing the target "sgxssl_no_mitigation" of Linux/Makefile.