Skip to content

feat: introducing support for private and public x25519 keys#49

Merged
FranciscoKurpiel merged 2 commits into
masterfrom
x25519
Apr 28, 2026
Merged

feat: introducing support for private and public x25519 keys#49
FranciscoKurpiel merged 2 commits into
masterfrom
x25519

Conversation

@FranciscoKurpiel
Copy link
Copy Markdown
Collaborator

No description provided.

@FranciscoKurpiel FranciscoKurpiel self-assigned this Apr 28, 2026
@FranciscoKurpiel FranciscoKurpiel merged commit 85a313d into master Apr 28, 2026
5 checks passed
@FranciscoKurpiel FranciscoKurpiel deleted the x25519 branch April 28, 2026 07:42
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces X25519PrivateKey and X25519PublicKey types to the xtypes package, enabling the use of X25519 keys within configuration structs. These types support various input formats such as PEM, hex, and raw bytes. The review feedback highlights a typo in an error message, suggests that public keys should not be redacted in usage information, and recommends proper error handling within test helper functions.

Comment thread xtypes/x25519_pub.go
Comment on lines +78 to +81
func (d *X25519PublicKey) GetDefaultValue() (string, error) {
// TODO show the public key
return "<secret>", nil
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Public keys are not secret, so GetDefaultValue should not return "<secret>" as this is misleading. The TODO comment on line 79 also indicates this needs to be addressed.

A better approach would be to return a representation of the public key. Since this is for usage information, a concise format like hex encoding is suitable. If no default value is set, an empty string should be returned.

func (d *X25519PublicKey) GetDefaultValue() (string, error) {
	if d.DefaultValue == nil {
		return "", nil
	}
	return hex.EncodeToString(d.DefaultValue.Bytes()), nil
}

Comment thread xtypes/x25519_pub.go
}
pubK, err := x509.ParsePKIXPublicKey(pemBlock.Bytes)
if err != nil {
return nil, fmt.Errorf("error decoding PEM block as ANS.1 public key: %w", err)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

There is a typo in the error message. ANS.1 should be ASN.1 (Abstract Syntax Notation One).

Suggested change
return nil, fmt.Errorf("error decoding PEM block as ANS.1 public key: %w", err)
return nil, fmt.Errorf("error decoding PEM block as ASN.1 public key: %w", err)

Comment thread xtypes/x25519_test.go
Comment on lines +171 to +172
newPriv, _ := ecdh.X25519().GenerateKey(rand.Reader)
pub = newPriv.PublicKey()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The error returned by ecdh.X25519().GenerateKey(rand.Reader) is ignored. While errors from rand.Reader are rare, it is best practice to handle all errors, even in test helpers, to make the test suite more robust. Please check the error and fail the test if it is not nil.

		newPriv, err := ecdh.X25519().GenerateKey(rand.Reader)
		if err != nil {
			t.Fatalf("failed to generate X25519 private key: %v", err)
		}
		pub = newPriv.PublicKey()

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant