Added vault signer

[ctyano]

Type of changes

Choose one of the below, or leave them empty:

• New feature proposal
• Bug fix
• Minor improvements
• Refactoring (no functional changes)
• Non-code changes (updating documentation, workflows, etc.)

[gemini-code-assist]

Summary of Changes

Hello @ctyano, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the application's certificate signing capabilities by integrating a new signer for HashiCorp Vault. This feature allows the application to leverage Vault's PKI secrets engine for issuing and managing X.509 certificates, providing a robust and secure option for certificate lifecycle management. The changes encompass the core Vault interaction logic, configuration updates, and seamless integration into the command-line tool.

Highlights

• New Vault Signer Implementation: A new signer has been introduced to support HashiCorp Vault, allowing the application to request and obtain X.509 certificates directly from a Vault PKI backend.
• Configuration Updates: The Makefile has been updated to include new default variables for configuring the Vault signer, such as sign URL, CA URL, issuer reference, TTL, and timeout. Existing Crypki and CFSSL signer variables have also been refactored for consistency.
• CLI Integration: The command-line interface now includes a 'vault' option, enabling users to send Certificate Signing Requests (CSRs) to Vault and retrieve the corresponding signed certificates and root CA certificates.
• Enhanced Version Information: The 'version' command output has been extended to display the default configuration parameters for the newly added Vault signer, providing better transparency and ease of debugging.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces support for HashiCorp Vault as a certificate signer. The changes include a new vault.go file in the pkg/signer package, modifications to main.go and test.go to handle the new signer type, updates to the Makefile for Vault configuration, and updates to version.go to display Vault configuration.

My review has identified several issues:

• Critical: There are typos in the Makefile that will cause build failures or incorrect default values for Crypki and CFSSL configurations.
• High: The test command for the Vault signer in cmd/test.go uses incorrect default URLs and calls the wrong function, effectively not testing the Vault integration.
• Medium: In the new pkg/signer/vault.go file, there are opportunities for improvement, such as handling potential errors from string conversions, using idiomatic error wrapping, and fixing a typo in a struct field name.

I have provided detailed comments and code suggestions to address these points.

[gemini-code-assist]

There are typos in the variable names and linker flag paths for the Crypki configuration. The variable names have an extra dot at the beginning (e.g., $(.DEFAULT_...) and the linker flag paths have a double dot (e.g., .../pkg/signer..DEFAULT_...). These will cause incorrect values to be compiled into the binary or result in build failures.

ifneq ($(DEFAULT_SIGNER_CRYPKI_VALIDITY),)
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CRYPKI_VALIDITY=$(DEFAULT_SIGNER_CRYPKI_VALIDITY)'
else
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CRYPKI_VALIDITY=2592000' # 30 * 24 * 60 * 60 seconds
endif
ifneq ($(DEFAULT_SIGNER_CRYPKI_IDENTIFIER),)
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CRYPKI_IDENTIFIER=$(DEFAULT_SIGNER_CRYPKI_IDENTIFIER)'
else
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CRYPKI_IDENTIFIER=athenz'
endif
ifneq ($(DEFAULT_SIGNER_CRYPKI_TIMEOUT),)
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CRYPKI_TIMEOUT=$(DEFAULT_SIGNER_CRYPKI_TIMEOUT)'
else
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CRYPKI_TIMEOUT=10' # seconds
endif

[gemini-code-assist]

There are typos in the variable name and linker flag path for the CFSSL timeout configuration. The variable name has an extra dot at the beginning ($(.DEFAULT_...) and the linker flag path has a double dot (.../pkg/signer..DEFAULT_...). This will cause incorrect values to be compiled into the binary or result in build failures.

ifneq ($(DEFAULT_SIGNER_CFSSL_TIMEOUT),)
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CFSSL_TIMEOUT=$(DEFAULT_SIGNER_CFSSL_TIMEOUT)'
else
LDFLAGS_ARGS += -X '$(APP_REPO_URL)/pkg/signer.DEFAULT_SIGNER_CFSSL_TIMEOUT=10'
endif

[gemini-code-assist]

Similar to the SendVaultCSR function, there are a couple of improvements that can be made here:

1. The error from strconv.Atoi on line 99 is ignored. This could lead to an unintended 0-second timeout. The error should be handled.
2. The error wrapping with fmt.Errorf on line 106 uses %s. It's better to use %w to allow for error unwrapping.

	timeout, err := strconv.Atoi(strings.TrimSpace(DEFAULT_SIGNER_VAULT_TIMEOUT))
	if err != nil {
		return fmt.Errorf("invalid timeout value for vault signer: %w", err), ""
	}
	client := &http.Client{
		Timeout: time.Duration(timeout) * time.Second,
	}

	req, err := http.NewRequest("GET", url, bytes.NewBuffer(nil))
	if err != nil {
		return fmt.Errorf("Failed to create request: %w", err), ""
	}