Skip to content
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

feat!: adds support for ED25519 Key Pairs and a wide range of public key formats #290

Merged
merged 2 commits into from
Mar 23, 2024

Conversation

udondan
Copy link
Owner

@udondan udondan commented Mar 23, 2024

Resolves #38

Adds support for ED25519 key types.

Adds support for public key formats:

  • PKCS#1
  • PKCS#8
  • RFC4253 (Base64 encoded)
  • PuTTY ppk

Breaking change:

We adopt the key format definition of sshpk. What used to be PublicKeyFormat.OPENSSH now is PublicKeyFormat.SSH. Also the new default format is SSH instead of OPENSSH.

If you explicitly had passed PublicKeyFormat.OPENSSH, change it to PublicKeyFormat.SSH.

Copy link
Contributor

coderabbitai bot commented Mar 23, 2024

Important

Auto Review Skipped

Review was skipped due to path filters

Files ignored due to path filters (2)
  • package-lock.json is excluded by: !**/*.json
  • package.json is excluded by: !**/*.json

Walkthrough

The recent updates bring enhancements to managing RSA and ED25519 key pairs in EC2 via a Lambda function, emphasizing support for various public key formats. Key terminology has been refined across the board for clarity, and the introduction of custom KMS keys for enhanced security is notable. The adoption of the sshpk library for public key generation and encoding signifies a shift towards streamlined operations, while added checks ensure smoother key pair management processes.

Changes

File(s) Summary of Changes
README.md Introduced support for RSA and ED25519 key pairs in EC2 through Lambda, including support for various public key formats. Updated terminology for key pair names and references, demonstrated custom KMS key usage, and outlined public key format requirements and storage settings.
lambda/index.ts - Replaced node-forge import with sshpk.
- Added checks to prevent key pair type changes during updates and for key pair existence before deletion.
- Implemented key type handling, key pair creation with KeyType parameter, and public key generation and encoding enhancements.
- Introduced warning logs for non-existent secrets during deletion.
lambda/types.ts,
lib/index.ts,
test/lib/test-stack.ts
- Updated enums for PublicKeyFormat and KeyType, adding formats and the ED25519 type.
- Included KeyType in ResourceProperties and KeyPairProps, adjusting default values and validations.
- Enhanced KeyPair class to handle KeyType and PublicKeyFormat based on input and OS compatibility.
- Modified imports and added iteration over enums in tests for dynamic KeyPair instance creation.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between efdef59 and 0472c14.
Files ignored due to path filters (4)
  • lambda/package-lock.json is excluded by: !**/*.json
  • lambda/package.json is excluded by: !**/*.json
  • package-lock.json is excluded by: !**/*.json
  • package.json is excluded by: !**/*.json
Files selected for processing (5)
  • README.md (7 hunks)
  • lambda/index.ts (10 hunks)
  • lambda/types.ts (2 hunks)
  • lib/index.ts (8 hunks)
  • test/lib/test-stack.ts (2 hunks)
Additional comments: 23
lambda/types.ts (3)
  • 15-53: The addition of new formats in the PublicKeyFormat enum (SSH, PKCS1, PKCS8, RFC4253, and PUTTY) alongside the existing OPENSSH and PEM formats is a significant enhancement. This broadens the range of public key formats the system can handle, improving flexibility and compatibility.
  • 55-58: The inclusion of ED25519 in the KeyType enum, alongside the existing RSA type, is a crucial update. It expands the cryptographic capabilities of the system by supporting a more diverse set of key types.
  • 72-72: Adding the KeyType property to the ResourceProperties interface is essential for supporting the new key type functionality. This change ensures that the system can handle different types of cryptographic keys, enhancing its versatility.
test/lib/test-stack.ts (2)
  • 11-11: The update to import KeyType alongside LogLevel and PublicKeyFormat is necessary for the test suite to utilize the new key type functionality. This change ensures that the tests can cover the expanded cryptographic capabilities of the system.
  • 107-130: The addition of loops to iterate over PublicKeyFormat and KeyType enums for creating KeyPair instances is a robust approach to testing the new functionality. It ensures comprehensive coverage by validating the system's behavior across a range of key types and public key formats. However, the condition to skip the combination of ED25519 and PKCS1 is crucial, as it reflects the system's limitations and ensures that the tests are aligned with the supported configurations.
README.md (3)
  • 17-28: The documentation now clearly states the support for RSA and ED25519 Key Pairs and the various public key formats. This update is crucial for users to understand the expanded capabilities of the system. It's well-detailed and covers the new features comprehensively.
  • 54-60: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [57-70]

The change in terminology from name to keyPairName for specifying key pair names and from keyName to keyPair for referencing key pairs on EC2 instances is well-documented. This clarification improves the readability and consistency of the documentation.

  • 109-115: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [112-126]

The documentation now includes examples demonstrating the ability to use custom KMS keys for encrypting key pairs. This addition is valuable for users looking to enhance the security of their key management practices. The examples are clear and provide straightforward guidance on implementing this feature.

lib/index.ts (7)
  • 19-25: The addition of KeyType to the imports and exports is necessary for the lib/index.ts file to utilize and expose the new key type functionality. This change ensures that the system can handle different types of cryptographic keys, enhancing its versatility.
  • 104-110: Adding the keyType property to the KeyPairProps interface with a default value of RSA is a crucial update. It allows users to specify the type of key pair they wish to create, providing flexibility in cryptographic key management.
  • 116-116: Changing the default value of publicKeyFormat in the KeyPairProps interface from OPENSSH to SSH aligns with the expanded support for various public key formats. This change should be clearly communicated to users to avoid confusion.
  • 240-247: The validation logic added to the KeyPair constructor to prevent the combination of ED25519 key type and PKCS1 format is essential for maintaining the system's integrity. It ensures that users are guided towards supported configurations, enhancing the system's reliability.
  • 263-270: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [266-281]

Updating the KeyPair constructor to set KeyType and PublicKeyFormat based on input, with sensible defaults, is a significant improvement. It ensures that the system can dynamically handle different cryptographic configurations, improving its flexibility and usability.

  • 410-410: Updating the Lambda runtime to NODEJS_20_X is a necessary change to ensure compatibility with the latest features and security updates. This update should be tested thoroughly to ensure that it does not introduce any regressions in the Lambda function's behavior.
  • 458-464: The modification to the _isOsCompatible method in the KeyPair class to consider keyType for OS compatibility is a thoughtful addition. It ensures that the system can accurately determine compatibility based on the type of cryptographic key, enhancing its reliability in diverse environments.
lambda/index.ts (8)
  • 48-48: The import from 'sshpk' for parsePrivateKey is correctly added to replace 'node-forge', aligning with the PR's objective to support a broader range of cryptographic algorithms and public key formats.
  • 115-120: Adding a check to prevent changing the key pair type during an update is a crucial security measure. This ensures that the cryptographic properties of a key pair remain consistent throughout its lifecycle, preventing potential vulnerabilities.
  • 193-193: The addition of the KeyType parameter in the createKeyPair function is essential for supporting multiple cryptographic algorithms. This change allows users to specify the type of key pair they wish to create, enhancing the system's flexibility.
  • 322-326: Implementing a check for the existence of a key pair before attempting deletion is a good practice. It prevents unnecessary API calls and potential errors when trying to delete non-existent resources.
  • 493-497: Refactoring the public key generation to use the 'sshpk' library is in line with the PR's objectives. This change supports a wider array of public key formats, improving compatibility with different systems and protocols.
  • 513-516: Adding base64 encoding for the RFC4253 public key format is a thoughtful detail. It addresses the issue of CloudFormation's inability to handle binary data directly, ensuring that the public key can be correctly exposed and stored.
  • 577-577: Adding warning logs for non-existing secrets during deletion is a good practice. It provides clear feedback to the user about the state of resources, helping to avoid confusion.
  • 625-644: The introduction of the keyPairExists function is a valuable addition for checking the existence of key pairs. This function enhances the robustness of the system by ensuring operations are performed on valid resources.

@udondan udondan merged commit 35ece30 into main Mar 23, 2024
4 checks passed
@udondan udondan deleted the ed25519-2 branch March 23, 2024 11:21
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.

Add support type of Key: RSA or ed25519
1 participant