Skip to content

Conversation

@jcarrete5
Copy link
Contributor

@jcarrete5 jcarrete5 commented Nov 17, 2025

Closes #157

@jcarrete5 jcarrete5 marked this pull request as ready for review November 17, 2025 22:00
Copy link
Contributor

@riwoh riwoh left a comment

Choose a reason for hiding this comment

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

Need to update copyright year if the file was touched.

@jcarrete5 jcarrete5 requested a review from riwoh November 20, 2025 17:37
mhabrat
mhabrat previously approved these changes Nov 21, 2025
riwoh
riwoh previously approved these changes Jan 16, 2026
Copilot AI review requested due to automatic review settings January 16, 2026 23:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR embeds a root keystore directly into the binary to simplify testing and deployment. Previously, the root keystore was read from an external PKCS12 file; now it can fall back to an embedded version when the ROOT_KEYSTORE environment variable is not set.

Changes:

  • Embedded default PKCS12 keystore data as a static array in a new header file
  • Modified the keystore loading logic to use embedded data when no external file is specified
  • Moved shared constants from common.h to the new root_keystore.h header
  • Added tests for the embedded keystore functionality

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
reference/src/util/include/root_keystore.h New header containing embedded PKCS12 keystore data and shared constants
reference/src/util/src/pkcs12.c Updated to load from embedded keystore when ROOT_KEYSTORE env var is not set
reference/src/util/include/common.h Removed DEFAULT_ROOT_KEYSTORE_PASSWORD and COMMON_ROOT_NAME (moved to root_keystore.h)
reference/src/util/include/pkcs12.h Updated documentation to reflect new embedded keystore behavior
reference/src/util/test/pkcs12test.cpp Added tests for embedded keystore loading functionality
reference/src/util/CMakeLists.txt Added root_keystore.h to library sources
reference/src/taimpl/src/porting/otp.c Added include for root_keystore.h
reference/src/taimpl/CMakeLists.txt Added BYPRODUCTS directive for test file copy
reference/src/client/test/sa_key_common.cpp Added include for root_keystore.h
reference/src/client/CMakeLists.txt Added BYPRODUCTS directive for test file copy
reference/README.md Updated documentation describing the new embedded keystore approach

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@riwoh riwoh dismissed stale reviews from mhabrat and themself via d8f88e4 January 16, 2026 23:45
Copilot AI review requested due to automatic review settings January 16, 2026 23:45
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jcarrete5 jcarrete5 force-pushed the embed-root-keystore branch from 827bbd2 to 7a3bd8e Compare January 18, 2026 19:38
Copilot AI review requested due to automatic review settings January 18, 2026 19:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Remove redundant include

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings January 20, 2026 04:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

This avoids copying the data multiple times if the header file is
include in many translation units.
Copilot AI review requested due to automatic review settings January 20, 2026 18:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jcarrete5
Copy link
Contributor Author

Depends on #161 to fix failing tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Embed root_keystore.p12 in the library

3 participants