Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Allow to choose algorithms when creating new metadata #645

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

Open
psmiraglia wants to merge 1 commit into IdentityPython:master
base: master
Choose a base branch
Loading
from psmiraglia:feat/allow-to-choose-metadata-algs

Conversation

@psmiraglia
Copy link

@psmiraglia psmiraglia commented Sep 20, 2019

The make_metadata.py uses default (and poor) algorithms for signature and digest computation when creating new metadata. It would be nice to allow the selection of these algorithms. The PR introduces the -S and -D command line arguments that can be used as follows

$ ../../tools/make_metadata.py \
 -s -x /usr/bin/xmlsec1 \
 -k pki/mykey.pem -c pki/mycert.pem \
 -S http://www.w3.org/2001/04/xmldsig-more#rsa-sha384 \
 -D http://www.w3.org/2001/04/xmlenc#sha512 \
 sp_conf

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

c00kiemon5ter reacted with thumbs up emoji
Copy link

codecov bot commented Sep 20, 2019 •
edited
Loading

Codecov Report

Merging #645 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph 

@@ Coverage Diff @@
## master #645 +/- ##
==========================================
+ Coverage 65.44% 65.48% +0.04% 
==========================================
 Files 103 103 
 Lines 25703 25703 
==========================================
+ Hits 16821 16832 +11 
+ Misses 8882 8871 -11
Impacted Files Coverage Δ
src/saml2/__init__.py 87.78% <0%> (-0.19%) ⬇️
src/saml2/time_util.py 87.42% <0%> (+0.59%) ⬆️
src/saml2/validate.py 79.21% <0%> (+4.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b224a02...123427f. Read the comment docs.

Copy link
Member

@psmiraglia Hey Paolo, nice to see you here!
The discussion on this topic is here: #628

I also saw that you have also bringed some commits from another branch of mine (those with allow_create documentation... already merged by c00kieMon5ter here: https://github.com/IdentityPython/pysaml2/pull/632/files).

If you can we could work on this in that PR, close this one.
We should move to a configuration asset that permits users to disable weak alghorithms not only in the metadata but also during signature/encryption verification.

Nice shoot

@psmiraglia psmiraglia force-pushed the feat/allow-to-choose-metadata-algs branch from eace864 to 123427f Compare September 20, 2019 10:31
Copy link
Author

@psmiraglia Hey Paolo, nice to see you here!
The discussion on this topic is here: #628

I don't think so. My PR just covers the script to generate the metadata. Algorithms I'm referring to are just used to compute the signature and the digest of the whole metadata (to check its integrity and authenticity). The mechanism to advertise supported algorithms (for SAML signature and assertions encryption) remains untouched.

I also saw that you have also bringed some commits from another branch of mine (those with allow_create documentation... already merged by c00kieMon5ter here: https://github.com/IdentityPython/pysaml2/pull/632/files).

Don't know why it happened... Anyway, I rebased it!

If you can we could work on this in that PR, close this one.

As I said, topics are similar but different. So, I would keep the PR opened.

We should move to a configuration asset that permits users to disable weak alghorithms not only in the metadata but also during signature/encryption verification.

This makes sense

Nice shoot

😄
peppelinux and c00kiemon5ter reacted with thumbs up emoji

@peppelinux peppelinux force-pushed the feat/allow-to-choose-metadata-algs branch from b6a5322 to 027d8d2 Compare December 15, 2020 16:15
Copy link
Contributor

Hi,
Given that metadata generation can also be invoked within an application (e.g., in SATOSA at /Saml2/proxy_saml2_backend.xml), it might be better to specify the algorithms in the config, instead of extra flags passed to make_metadata.py.

If all input information needed for generating the metadata is captured in the config, then all the different ways of triggering the metadata generation would produce the same output.

How does this sound?

Cheers,
Vlad

Copy link
Member

Good to me!

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

Reviewers

@c00kiemon5ter c00kiemon5ter c00kiemon5ter left review comments

@peppelinux peppelinux peppelinux left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /