Skip to content

feat: rsa handler#859

Merged
Skarlso merged 10 commits into
open-component-model:mainfrom
jakobmoellerdev:rsa/handler
Sep 17, 2025
Merged

feat: rsa handler#859
Skarlso merged 10 commits into
open-component-model:mainfrom
jakobmoellerdev:rsa/handler

Conversation

@jakobmoellerdev

@jakobmoellerdev jakobmoellerdev commented Sep 9, 2025

Copy link
Copy Markdown
Member

What this PR does / why we need it

  • Added a new Go module in bindings/go/rsa to support RSA signing configurations.
  • Implemented configurations for both RSASSA-PSS (probabilistic) and RSASSA-PKCS1 v1.5 (deterministic) signature schemes.
  • Introduced support for signature encoding policies (Plain and PEM) to manage signature serialization and storage with both supported formats from OCMv1
  • Added logic for media types, algorithms, and integration with runtime schemas.
  • Autogenerated necessary deepcopy and runtime type management code for reusability.
  • Wrote an extensive test suite for all possible signing/verification combinations available

Which issue(s) this PR fixes

part of open-component-model/ocm-project#648

@github-actions github-actions Bot added the size/l Large label Sep 9, 2025
@jakobmoellerdev jakobmoellerdev changed the title Rsa/handler feat: rsa handler Sep 9, 2025
@github-actions github-actions Bot added the kind/feature new feature, enhancement, improvement, extension label Sep 9, 2025
…configurations (open-component-model#864)

#### What this PR does / why we need it
- Added a new Go module in `bindings/go/rsa/signing/v1alpha1` to support RSA signing configurations.
- Implemented configurations for both `RSASSA-PSS` (probabilistic) and `RSASSA-PKCS1 v1.5` (deterministic) signature schemes.
- Introduced support for signature encoding policies (`Plain` and `PEM`) to manage signature serialization and storage.
- Added logic for media types, algorithms, and integration with runtime schemas.
- Autogenerated necessary deepcopy and runtime type management code for reusability.

#### Which issue(s) this PR fixes
- Lays groundwork for integrating modern cryptographic signing methods.
- Ensures backward compatibility with legacy PKCS1 v1.5 while promoting secure PSS usage.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
@jakobmoellerdev jakobmoellerdev force-pushed the rsa/handler branch 2 times, most recently from 7a6afa2 to 99e332c Compare September 9, 2025 13:10
@jakobmoellerdev jakobmoellerdev marked this pull request as ready for review September 9, 2025 13:16
@jakobmoellerdev jakobmoellerdev requested a review from a team as a code owner September 9, 2025 13:16
…for RSA signing handlers (open-component-model#865)

#### What this PR does / why we need it
- Consolidated `PSSConfig` and `PKCS1V15Config` into a single `Config` abstraction to simplify configuration handling for RSA signing.
- Added a new dynamic `Scheme` for consistent runtime type registration.
- Developed an extensive RSA handler test suite:
  - Covers both `RSASSA-PSS` and `RSASSA-PKCS1 v1.5` signature algorithms.
  - Includes tests for various signature encoding policies (`Plain` and `PEM`).
  - Added error path tests for unsupported algorithms, missing credentials, and tampered signatures.
- Enhanced identity handling for signing and verification configurations.

#### Which issue(s) this PR fixes
- Streamlines the configuration structure for RSA signing handlers.
- Ensures better test coverage and resilience for edge cases and error scenarios.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
@matthiasbruns

Copy link
Copy Markdown
Contributor

image

@matthiasbruns matthiasbruns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

More review tomorrow

Comment thread bindings/go/rsa/signing/handler/handler.go Outdated
Comment thread bindings/go/rsa/signing/handler/handler.go
Comment thread bindings/go/rsa/signing/handler/handler.go Outdated

@matthiasbruns matthiasbruns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

beautiful

Comment thread bindings/go/rsa/signing/handler/internal/pem/pem.go Outdated
Comment thread bindings/go/rsa/signing/handler/internal/pem/pem_signature.go Outdated

@Skarlso Skarlso left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Couple of first pass comments.

Comment thread bindings/go/rsa/go.mod Outdated
Comment thread bindings/go/rsa/signing/handler/handler.go
Comment thread bindings/go/rsa/signing/handler/internal/dn/dn.go Outdated
Comment thread bindings/go/rsa/signing/handler/handler.go Outdated
Comment thread bindings/go/rsa/signing/handler/handler.go
Comment thread bindings/go/rsa/signing/handler/internal/dn/dn.go Outdated
…er and matcher

#### What this PR does / why we need it
- Replaced the internal `dn` package with a new `rfc2253` package to handle Distinguished Names (DNs) in compliance with RFC 2253.
  - Improved parsing of escaped characters, quoted values, and encoded forms like `#hex`.
  - Added robust test coverage for conformance, equality, and matching rules.
- Updated RSA signing handler to use the new `rfc2253` package for issuer verification.
- Eliminated old `dn` parsing and matching logic, simplifying the handling code.
- Enhanced error messages for DN matching failures for better clarity.
- Introduced options for lenient or strict DN parsing modes.

#### Which issue(s) this PR fixes

- Aligns DN parsing and matching with RFC 2253 specifications.
- Simplifies codebase by removing redundant and inconsistent logic.
- Improves resilience and correctness in X.509 issuer verification so we are actually using a proper parsing specification instead of a best effort parser like with ocm v1

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
#### What this commit does
- Updated the `New` function's comment in `bindings/go/rsa/signing/handler/handler.go` to clarify the behavior when `useSystemRoots` is `false`.
  - Explicitly states that an empty certificate pool is used in such cases.

#### Why this is needed
- Ensures the behavior of the function is well-documented for developers, improving code readability and reducing potential confusion.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
@jakobmoellerdev

jakobmoellerdev commented Sep 11, 2025

Copy link
Copy Markdown
Member Author

Well this comment here turned out to be a complete disaster.

In OCMv1 we use a badly implemented, kind of weird parser infrastructure for issuer matching that is not really conforming to anything. Its a mix of OpenSSL and LDAP issuer String interpretation.

To standardize here going forward I have decided after discussion with @Skarlso to implement a RFC2253 compliant distinguished name parser.

This parser is important because previously in OCM we couldn't deal with a lot of common encoding formats for certificate attributes such as a simple CN=John, Doe because of special character handling missing.

Before you ask: Yes there are libraries available:

I did test this version here significantly but if we say its too bad to maintain, I suggest we take over https://github.com/tsaarni/x500dn/blob/master/dn.go and adjust it as per MIT license allowed for our requirements. Still a lot of code though...


We decided to mark the entire PEM flow as experimental and proceed with our version of RFC2253 as we believe it to be the most accurate yet least used flow.

…mental

#### What this commit does
- Changed the default signature encoding policy to `Plain` (`SignatureEncodingPolicyDefault`).
- Marked `PEM` signature encoding as experimental with warnings during signing and verification.
- Adjusted `handler.go` to use `context.Context` for logging warnings and improved code maintainability.

#### Why this is needed
- Aligns with simpler and more widely usable default (`Plain`) while leaving `PEM` as an experimental feature for selective use.
- Improves clarity for developers by explicitly marking experimental features and providing logging to reduce confusion.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
Comment thread bindings/go/rsa/signing/handler/handler.go
Comment thread bindings/go/rsa/signing/handler/internal/rfc2253/doc.go Outdated
Skarlso
Skarlso previously approved these changes Sep 11, 2025

@Skarlso Skarlso left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Largely okay. I could re-read this many times and perhaps find something. But we're going to have to iterate on them.

#### What this commit does
- Updated `Equal` function in `rfc2253` package to return errors instead of a boolean.
  - Allows better debugging and tracing of mismatches in Distinguished Names (DNs).
- Modified `handler.go` to handle errors returned by `Equal` and propagate detailed mismatch information.

#### Why this is needed
- Enhances error reporting for DN comparison issues, improving debugging and user feedback.
- Aligns with the goal of providing robust and clear validation mechanisms for X.509 issuer verification logic.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
Skarlso
Skarlso previously approved these changes Sep 12, 2025
Comment thread bindings/go/rsa/signing/v1alpha1/config.go
Comment thread bindings/go/rsa/signing/v1alpha1/group_version.go
Comment thread bindings/go/rsa/signing/handler/internal/pem/pem.go
Comment thread bindings/go/rsa/signing/handler/internal/pem/pem.go Outdated
…arity

#### What this commit does
- Simplifies the `ParseRSAPublicKeyPEM` function to return a single parsed `RSAPublicKeyPEM` when possible.
- Updates documentation to better describe supported PEM containers (`PKCS#1` and `PKIX`) and return behavior.
- Ensures function exits early if no PEM blocks are found, optimizing for clarity and correctness.

#### Why this is needed
- Reduces ambiguity in function usage by clarifying its behavior and supported formats.
- Aligns with clean code principles by improving readability and maintainability.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
- Updated `ParseRSAPublicKeyPEM` documentation to include support for X.509 certificates.
- Improves clarity on accepted PEM container formats (`PKCS#1`, `PKIX`, and `X.509`).
- Enhances developer understanding of the function's capabilities and use cases.

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
@Skarlso Skarlso enabled auto-merge (squash) September 17, 2025 06:23
@Skarlso Skarlso merged commit 9e75eea into open-component-model:main Sep 17, 2025
16 checks passed
jakobmoellerdev added a commit to jakobmoellerdev/open-component-model that referenced this pull request Sep 17, 2025
<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

- Added a new Go module in `bindings/go/rsa` to support RSA signing
configurations.
- Implemented configurations for both `RSASSA-PSS` (probabilistic) and
`RSASSA-PKCS1 v1.5` (deterministic) signature schemes.
- Introduced support for signature encoding policies (`Plain` and `PEM`)
to manage signature serialization and storage with both supported
formats from OCMv1
- Added logic for media types, algorithms, and integration with runtime
schemas.
- Autogenerated necessary deepcopy and runtime type management code for
reusability.
- Wrote an extensive test suite for all possible signing/verification
combinations available

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

part of open-component-model/ocm-project#648

---------

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
Co-authored-by: Gergely Brautigam <gergely.brautigam@sap.com>
matthiasbruns pushed a commit to matthiasbruns/open-component-model that referenced this pull request Sep 18, 2025
<!-- markdownlint-disable MD041 -->
#### What this PR does / why we need it

- Added a new Go module in `bindings/go/rsa` to support RSA signing
configurations.
- Implemented configurations for both `RSASSA-PSS` (probabilistic) and
`RSASSA-PKCS1 v1.5` (deterministic) signature schemes.
- Introduced support for signature encoding policies (`Plain` and `PEM`)
to manage signature serialization and storage with both supported
formats from OCMv1
- Added logic for media types, algorithms, and integration with runtime
schemas.
- Autogenerated necessary deepcopy and runtime type management code for
reusability.
- Wrote an extensive test suite for all possible signing/verification
combinations available

#### Which issue(s) this PR fixes
<!--
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

part of open-component-model/ocm-project#648

---------

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
Co-authored-by: Gergely Brautigam <gergely.brautigam@sap.com>
jakobmoellerdev added a commit that referenced this pull request Sep 19, 2025
#### What this does
- Added a new `SigningRegistry` to manage signing handler plugins.
- Integrated signing handler support into the plugin manager and
registries.
- Updated dependencies in `go.mod` and `go.sum` to include the signing
library.
- Adjusted test cases to align with the new signing plugin
functionality.

_Note: I have made it so that Signing and Verification expect the same
Config Type right now. Technically this is a bit of a conflict with the
SigStore ADR Proposal because it would have two different configs for
signing and verifications but I think having one type that can be used
with different fields would also work, and its significantly easier to
implement here_

#### Why this change is needed

- Expands the plugin framework capabilities to include signing handlers
for enhanced security and verification processes.

part of adopting the RSA handler implemented for
open-component-model/ocm-project#648 in
#859.
It would be registered as an internal handler via
`RegisterInternalComponentSignatureHandler` in the CLI

---------

Signed-off-by: Jakob Möller <jakob.moeller@sap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature new feature, enhancement, improvement, extension size/l Large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants