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

Add other algorithms for DNS/DHCP (HMAC-SHA512) #7389

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gskouson
Copy link

The PR is to fix issue _#7173

The modification include

Change ddns.pm to allow for other signing algorithms and key names

Without changes to the site table, this should default to working as expected in the past. The default is still HMAC-MD5

Change dhcp.pm to allow for other signing algorithms and key names

Without changes to the site table, this should default to the past functionality.

Summary of changes

In both cases an optional site table parameter (omapi-algorithm) is used to specify the signing algorithm for ddns and dhcp communication with the DNS server. It adds support for HMAC-SHA1, HMAC-SHA256, HMAC-SHA384 and HMAC-SHA512 as possible signing options for DNS communications. While additional options may also be possible the code expects the above options, otherwise HMAC-MD5 is used.

It also allows the site table parameter (omapi-username) to choose a different username for the DNS communication. In some cases the DNS provider may require you to use the key name and secret they provide rather than allowing the use of the key name and secret provided by xCAT.

I have no specific unit tests for this change. I've build/installed RPMs based on this change with no impacts to existing test installation. I've successfully added the omapi-algorithm and omapi-username information to the site table and have updated the passwd table to reflect the username and secret. It seems to build local DNS configurations and also allow for externaldns=1 and allow to point at an external DNS provider when the external provider uses the appropriate key and username.

The UT result

No specific unit tests written for this change
##The UT output##

Co-authored-by: Robert Sand <ros5549@psu.edu>
@samveen
Copy link
Member

samveen commented May 28, 2023

Please review #7173 and #7181 as well (other HMAC related Issues/PRs) for related ideas

@gskouson
Copy link
Author

We reviewed both of these pull requests and this merge request should incorporate the changes from both of these with the addition of allowing a change in the key username.

We also added info to the documentation.

I wasn't sure of the proper way to pull everything together, so I created this pull request. I'm happy to do something differently.

@gskouson
Copy link
Author

gskouson commented Jun 1, 2023

This should also resolve the concern with #6757

@sgroel
Copy link

sgroel commented Jul 28, 2023

I've implemented these changes with success, would appreciate this being merged for future release version.

@samveen
Copy link
Member

samveen commented Aug 1, 2023

@gurevichmark @besawn for your kind attention.

@samveen
Copy link
Member

samveen commented Aug 1, 2023

@gskouson I believe you will need to sign the xCAT CLA for individuals and submit it for this PR to be accepted. (I'd had to do that for my first PR too.) It's a quick process. documented at https://xcat-docs.readthedocs.io/en/latest/developers/license/xcat_individual_contributor_license_agreement.html?highlight=CLA

@gskouson
Copy link
Author

gskouson commented Aug 3, 2023

We've signed an organization agreement from Penn State. I should be on the contributor list.

@besawn
Copy link
Member

besawn commented Aug 4, 2023

@gskouson I have received your contributor license agreement, thank you.

@sgroel
Copy link

sgroel commented Aug 13, 2023

A minor change that may be needed here:
The site table entries containing a dash in the attribute name will cause mypostscript to throw errors. This script is automatically generated and dumps the contents of the site table and exports them as variables. One of the offending entries is omapi-algorithm. The change is simple enough though, convert any dashes to underscores.

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.

None yet

4 participants