-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[crypto,devbundle] Package cryptolib as part of the release devbundle (+ doxygen fixes) #30712
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.
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
base: earlgrey_1.0.0
Are you sure you want to change the base?
Changes from all commits
c9ef2a6
9a8f857
93d2d82
bbc44c3
5fa5dcc
d7be32a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| package(default_visibility = ["//visibility:public"]) | ||
|
|
||
| load("@rules_pkg//pkg:mappings.bzl", "pkg_files") | ||
| load("//sw/device/lib/crypto/include:api_config.bzl", "cryptolib_api_config_header") | ||
|
|
||
| config_setting( | ||
| name = "disable_buf_integrity_checks", | ||
|
|
@@ -16,15 +17,28 @@ config_setting( | |
| # Export all headers. | ||
| exports_files(glob(["*.h"])) | ||
|
|
||
| # Header defining build options for the public ABI | ||
| cryptolib_api_config_header( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A question: we also have our FIPS build, for example, --config=crypto_fips_all. Is that something we could place in api_config as well? In fact, we have several build options https://github.com/lowRISC/opentitan/blob/master/doc/security/cryptolib/cryptolib_api.md#build-configurations.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I don't see a reason why we can't place more things in here. The reason that I only put this define in here for now is because it is the only macro I could see that potentially changes the ABI, and could cause issues if somebody used the library incorrectly. For defines that only impact the internals of the implementation, there was no need to expose these. But it might be nice to expose the build options in this way so that users can consume them as well - I think you probably have a better grasp of what would be good to put in here or not, so if you have any suggestions I'm happy to make them (or we can add them in a follow-up PR). It should be quite easy to change the rule to put more stuff here. |
||
| name = "api_config.h", | ||
| buffer_integrity_checks = select({ | ||
| ":disable_buf_integrity_checks": False, | ||
| "//conditions:default": True, | ||
| }), | ||
| ) | ||
|
|
||
| cc_library( | ||
| name = "api_config", | ||
| hdrs = [":api_config.h"], | ||
| defines = ["OTCRYPTO"], | ||
| ) | ||
|
|
||
| cc_library( | ||
| name = "datatypes", | ||
| hdrs = ["datatypes.h"], | ||
| defines = ["OTCRYPTO_IN_REPO=1"] + select({ | ||
| ":disable_buf_integrity_checks": ["OTCRYPTO_DISABLE_BUF_INTEGRITY_CHECKS"], | ||
| "//conditions:default": [], | ||
| }), | ||
| defines = ["OTCRYPTO_IN_REPO=1"], | ||
| includes = ["."], | ||
| deps = [ | ||
| ":api_config", | ||
| "//sw/device/lib/base:hardened", | ||
| "//sw/device/lib/base:status", | ||
| ], | ||
|
|
@@ -57,12 +71,10 @@ cc_library( | |
| "sha2.h", | ||
| "sha3.h", | ||
| ], | ||
| defines = ["OTCRYPTO_IN_REPO=1"] + select({ | ||
| ":disable_buf_integrity_checks": ["OTCRYPTO_DISABLE_BUF_INTEGRITY_CHECKS"], | ||
| "//conditions:default": [], | ||
| }), | ||
| defines = ["OTCRYPTO_IN_REPO=1"], | ||
| includes = ["."], | ||
| deps = [ | ||
| ":api_config", | ||
| "//sw/device/lib/base:hardened", | ||
| "//sw/device/lib/base:status", | ||
| ], | ||
|
|
@@ -96,6 +108,7 @@ cc_library( | |
| "self_integrity.h", | ||
| "sha2.h", | ||
| "sha3.h", | ||
| ":api_config.h", | ||
| "//sw/device/lib/crypto/include/freestanding:absl_status.h", | ||
| "//sw/device/lib/crypto/include/freestanding:defs.h", | ||
| "//sw/device/lib/crypto/include/freestanding:hardened.h", | ||
|
|
@@ -105,13 +118,17 @@ cc_library( | |
|
|
||
| pkg_files( | ||
| name = "package", | ||
| srcs = glob(["*.h"]), | ||
| srcs = glob(["*.h"]) + [ | ||
| ":api_config.h", | ||
| ], | ||
| prefix = "crypto/include", | ||
| ) | ||
|
|
||
| genrule( | ||
| name = "doxygen", | ||
| srcs = glob(["*.h"]), | ||
| srcs = glob(["*.h"]) + [ | ||
| ":api_config.h", | ||
| ], | ||
| outs = [ | ||
| "Doxyfile", | ||
| "latex/refman.pdf", | ||
|
|
@@ -128,6 +145,7 @@ OUTPUT_DIRECTORY="$(RULEDIR)" | |
| HAVE_DOT=NO | ||
| OPTIMIZE_OUTPUT_FOR_C=YES | ||
| LATEX_HIDE_INDICES=YES | ||
| FULL_PATH_NAMES=NO | ||
| EOF | ||
|
|
||
| for f in $(SRCS); do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| # Copyright lowRISC contributors (OpenTitan project). | ||
| # Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| CRYPTOLIB_API_CONFIG_HEADER = """\ | ||
| // Copyright lowRISC contributors (OpenTitan project). | ||
| // Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| #ifndef OPENTITAN_CRYPTOLIB_BUILD_CONFIG_H_ | ||
| #define OPENTITAN_CRYPTOLIB_BUILD_CONFIG_H_ | ||
| /** | ||
| * @file | ||
| * @brief Build configuration options for the OpenTitan cryptography library. | ||
| * | ||
| * Generated as part of an OpenTitan cryptography library build. | ||
| * | ||
| * It describes the configuration of the the library and must match the library | ||
| * archive being linked. Specifically, it defines ABI-affecting configuration | ||
| * selected when building the library. Consumers should not modify or define | ||
| * these values manually. | ||
| */ | ||
| {cryptolib_defines} | ||
| #endif /* OPENTITAN_CRYPTOLIB_BUILD_CONFIG_H_ */ | ||
| """ | ||
|
|
||
| def _cryptolib_api_config_header_impl(ctx): | ||
| output_name = ctx.attr.name | ||
| if not output_name.endswith(".h"): | ||
| output_name = "{}.h".format(output_name) | ||
| header = ctx.actions.declare_file(output_name) | ||
|
|
||
| lines = [] | ||
| if ctx.attr.buffer_integrity_checks: | ||
| lines += [ | ||
| "#ifdef OTCRYPTO_DISABLE_BUF_INTEGRITY_CHECKS", | ||
| "#error \"This release has been built with buffer integrity checks, so these cannot be disabled.\"", | ||
| "#endif /* OTCRYPTO_DISABLE_BUF_INTEGRITY_CHECKS */", | ||
| ] | ||
| else: | ||
| lines.append("#define OTCRYPTO_DISABLE_BUF_INTEGRITY_CHECKS") | ||
| defines = "\n{}\n".format("\n".join(lines)) if lines else "" | ||
|
|
||
| content = CRYPTOLIB_API_CONFIG_HEADER.format( | ||
| cryptolib_defines = defines, | ||
| ) | ||
| ctx.actions.write( | ||
| output = header, | ||
| content = content, | ||
| ) | ||
|
|
||
| return [DefaultInfo(files = depset([header]))] | ||
|
|
||
| cryptolib_api_config_header = rule( | ||
| implementation = _cryptolib_api_config_header_impl, | ||
| attrs = { | ||
| "buffer_integrity_checks": attr.bool(default = True, doc = "Perform additional integrity checks of buffers, at some additional cost"), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On our side, we should also come with the BUILD options for the cryptolib to enter into the release bundle... I believe the vanilla option (no flags) is perfect for now. We will need a FIPS build later, but I need to sit with stakeholders for that. When that shakes out, I can make a PR and put you as reviewer where we try add this FIPS option as well