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

[READY] v2.0: Support DSA and ECDSA signing keys #705

Merged
merged 6 commits into from
Sep 30, 2024

Conversation

johnnyshields
Copy link
Collaborator

@johnnyshields johnnyshields commented Jul 10, 2024

Fixes #661

Replaces #683

Depends on #711, please merge it first.

Currently RubySaml supports only RSA keys. The SAML standard can also support ECDSA and DSA keys. This PR adds support for both:

  1. Validating IdP EC/DSA sigs AND
  2. Using SP EC/DSA signing keys.

JRuby supports DSA but cannot support ECDSA due to this issue.

You may not use EC/DSA keys for encryption. An ArgumentError is raised if you attempt to do so.

It includes the following changes, which are all done in a backward compatible manner:

  • When generating SP metadata/requests, settings.security[:signature_method] now ignores the "rsa" component of its user-set value and automatically uses whatever type of SP signing public key you actually set (e.g. a DSA key) plus the "sha" component of the value.
    • (Previously, only RSA was supported, so this doesn't break anything.)
  • settings.security[:signature_method] supports shortcut values :sha1, :sha256, etc.
    • Shortcuts :rsa_sha256, :dsa_sha256 etc. also work, but as per above the "rsa"/"dsa" are ignored in favor of the SP public key type.
  • Similar to above settings.security[:digest_method] supports shortcut values sha1, sha256, etc.
  • New module XMLSecurity::Crypto is extracted from XMLSecurity::Document
  • Cleaned-up code, including related to canonicalization

DONE:

  • Ensure existing tests pass
  • request_test.rb -- cover all 3 key types
  • logoutrequest_test.rb -- cover all 3 key types
  • slo_logoutresponse_test.rb -- cover all 3 key types
  • metadata_test.rb -- cover all 3 key types
  • Fix JRuby failures
  • basic tests (settings, etc.) -- cover all 3 key types
  • test EC/DSA keys when used by IdP
  • disable encryption with EC/DSA keys

@johnnyshields johnnyshields changed the base branch from master to v2.x July 10, 2024 16:04
@johnnyshields johnnyshields changed the title Ec dsa redux [WIP - NOT READY] v2.0: Support DSA and ECDSA signing keys Jul 10, 2024
@johnnyshields
Copy link
Collaborator Author

johnnyshields commented Jul 10, 2024

@pitbulk this is ready for your initial review. I'll give it another pass tomorrow but the core changes and tests are basically working 🎉

@johnnyshields johnnyshields changed the title [WIP - NOT READY] v2.0: Support DSA and ECDSA signing keys [PLEASE REVIEW BUT DO NOT MERGE] v2.0: Support DSA and ECDSA signing keys Jul 11, 2024
…y PEM values, including the `RubySaml::Util#format_cert` and `#format_private_key` methods. Introduces new `RubySaml::PemFormatter` module.
@johnnyshields johnnyshields force-pushed the ec-dsa-redux branch 12 times, most recently from 4680d8d to 9430fe9 Compare July 13, 2024 14:52
@johnnyshields johnnyshields changed the title [PLEASE REVIEW BUT DO NOT MERGE] v2.0: Support DSA and ECDSA signing keys [READY] v2.0: Support DSA and ECDSA signing keys Jul 13, 2024
@johnnyshields
Copy link
Collaborator Author

@pitbulk this is ready for your review and merge. (Please review #711 first)

@johnnyshields
Copy link
Collaborator Author

@pitbulk FYI I am now running this branch in prod without issue.

@pitbulk pitbulk merged commit 455d17d into SAML-Toolkits:v2.x Sep 30, 2024
16 of 17 checks passed
@pitbulk
Copy link
Collaborator

pitbulk commented Sep 30, 2024

@johnnyshields, merged this one but rubocop needs to be fixed

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.

2 participants