Skip to content

Provide PKCS12 keystore support to encrypt secrets#521

Open
arunans23 wants to merge 1 commit into
wso2:masterfrom
arunans23:pkcs12
Open

Provide PKCS12 keystore support to encrypt secrets#521
arunans23 wants to merge 1 commit into
wso2:masterfrom
arunans23:pkcs12

Conversation

@arunans23

@arunans23 arunans23 commented Jun 18, 2026

Copy link
Copy Markdown
Member

Purpose

Describe the problems, issues, or needs driving this feature/fix and include links to related issues in the following format: Resolves issue1, issue2, etc.

$subject

Currently, our CLI supports only JKS type keystore files when encrypting keys. This PR will add an improvement to support PKCS12 type keystore when encrypting keys.

@arunans23
arunans23 requested a review from chanikag as a code owner June 18, 2026 06:14
@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Overview

This pull request adds PKCS12 keystore support for secret encryption in the WSO2 Micro Integrator tooling project, extending the existing JKS-only keystore functionality.

Key Changes

Command Updates

  • Updated the secret init command to support both JKS and PKCS12 keystores
  • Modified keystore type detection to dynamically determine the type from the provided keystore file path
  • Adjusted password prompting logic to be keystore-type aware

Encryption Utilities

  • Enhanced keystore configuration handling with support for both JKS and PKCS12 types
  • Modified validation logic so that key passwords are required only for JKS keystores (PKCS12 uses the keystore password for the key)
  • Added automatic defaulting to JKS type for legacy configuration files without explicit keystore type specification
  • Implemented dedicated PKCS12 decoding path for private key extraction
  • Strengthened RSA key type validation during key extraction to provide clearer error messages

Dependencies

  • Added software.sslmate.com/src/go-pkcs12 v0.7.2 dependency for PKCS12 support

Documentation & Testing

  • Updated user-facing documentation to reflect support for both JKS and PKCS12 keystores
  • Added comprehensive test coverage for keystore type detection and configuration validation

Impact

Users can now encrypt secrets using either JKS or PKCS12 keystores, with backward compatibility maintained for existing JKS-based configurations.

Walkthrough

The changes extend the secret encryption feature to support PKCS12 keystores alongside the existing JKS support. In encryptionUtils.go, keystore type constants are introduced, KeyStoreConfig YAML tags are updated to omitempty, and IsValidKeyStoreConfig is revised so KeyPassword is required only for non-PKCS12 keystores. GetKeyStoreConfigFromFile now defaults KeyStoreType to jks for backward compatibility. getEncryptionKey branches by type, routing PKCS12 through a new getEncryptionKeyFromPKCS12 function and hardening the JKS RSA key assertion. The secret init CLI command replaces extension-based JKS detection with utils.GetKeyStoreType and makes the key password prompt conditional on keystore type. A new go-pkcs12 dependency is added. Unit tests for GetKeyStoreType and IsValidKeyStoreConfig are added, and documentation is updated.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is incomplete. While it addresses Purpose, it omits critical required sections including Goals, Approach, User Stories, Release Notes, Documentation, and all subsequent sections of the template. Complete the pull request description by filling in all remaining template sections: Goals, Approach, User Stories, Release Notes, Documentation, Training, Certification, Marketing, Automation Tests, Security Checks, Samples, Related PRs, Migrations, Test Environment, and Learning.
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main feature being added: PKCS12 keystore support for secret encryption.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
cmd/utils/encryptionUtils.go (1)

184-190: ⚡ Quick win

Consider handling base64 decode errors for clearer diagnostics.

Lines 185 and 198 ignore the error return from base64.StdEncoding.DecodeString. If the stored password is malformed base64, the decode will fail silently, and the resulting empty/partial bytes will cause a downstream authentication failure with a less informative error message.

♻️ Proposed fix
 func getEncryptionKey(keyStoreConfig *KeyStoreConfig) (*rsa.PublicKey, error) {
-	keyStorePassword, _ := base64.StdEncoding.DecodeString(keyStoreConfig.KeyStorePassword)
+	keyStorePassword, err := base64.StdEncoding.DecodeString(keyStoreConfig.KeyStorePassword)
+	if err != nil {
+		return nil, errors.New("Invalid Key Store password encoding: " + err.Error())
+	}
 	if IsPKCS12KeyStore(keyStoreConfig.KeyStoreType) {
 		return getEncryptionKeyFromPKCS12(keyStoreConfig.KeyStorePath, string(keyStorePassword))
 	}
 	return getEncryptionKeyFromJKS(keyStoreConfig, keyStorePassword)
 }

Apply a similar pattern at line 198 for keyPassword decoding.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/utils/encryptionUtils.go` around lines 184 - 190, The getEncryptionKey
function ignores the error return value when calling
base64.StdEncoding.DecodeString on keyStoreConfig.KeyStorePassword at line 185.
Modify the code to capture both the decoded bytes and the error from the
DecodeString call, then check if the error is not nil and return it early with a
descriptive error message. Apply the same pattern to the keyPassword decoding at
line 198 to ensure all base64 decode operations are properly validated before
being used in downstream operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cmd/utils/encryptionUtils.go`:
- Around line 184-190: The getEncryptionKey function ignores the error return
value when calling base64.StdEncoding.DecodeString on
keyStoreConfig.KeyStorePassword at line 185. Modify the code to capture both the
decoded bytes and the error from the DecodeString call, then check if the error
is not nil and return it early with a descriptive error message. Apply the same
pattern to the keyPassword decoding at line 198 to ensure all base64 decode
operations are properly validated before being used in downstream operations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 758404b4-9a08-4b62-80a1-68953cef3aec

📥 Commits

Reviewing files that changed from the base of the PR and between 51c1c5c and 255591f.

⛔ Files ignored due to path filters (1)
  • cmd/go.sum is excluded by !**/*.sum
📒 Files selected for processing (5)
  • cmd/cmd/secret/init.go
  • cmd/docs/mi_secret_init.md
  • cmd/go.mod
  • cmd/utils/encryptionUtils.go
  • cmd/utils/encryptionUtils_test.go

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.

1 participant