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

SAML Request signing broken due to strip! method #643

Closed
bramleyjl opened this issue Sep 20, 2022 · 7 comments
Closed

SAML Request signing broken due to strip! method #643

bramleyjl opened this issue Sep 20, 2022 · 7 comments

Comments

@bramleyjl
Copy link

We are updating our SAML authentication requests to our service provider by including SAML signing, however the resulting DigestValue is always blank. I traced the problem back to the compute_digest method, which is using strip! to remove preceding and trailing whitespaces.

    def compute_digest(document, digest_algorithm)
      digest = digest_algorithm.digest(document)
      Base64.encode64(digest).strip!
    end

We are using OpenSSL::Digest::SHA256 as our digest_algorithm, but the Base64 encoding of the digest created on the previous line doesn't contain any preceding or trailing whitespaces, causing the strip! method to return nil instead of the supplied value. We have also attempted to alter the SAML metadata provided to the ruby-saml gem in order to introduce a whitespace to pass this method but were unable to find a way to do so.

What is the purpose of using strip! over the base strip method? Is it possible for it to be replaced with strip in this instance?

@StoneCypher
Copy link

OneLogin stopped maintaining this library more than a month ago

#640

@StoneCypher
Copy link

@pitbulk was trying to arrange takeover rights. I don't know whether that ended up happening

@mrmoss
Copy link

mrmoss commented Sep 20, 2022

Still working on getting it swapped over. @eriktalvi - Any updates?

@mrmoss
Copy link

mrmoss commented Sep 22, 2022

@pitbulk - Haven't heard anything...you want to just start forking things?

Let us know how we can help. Could assign @eriktalvi to help with that forking process as long as @Subterrane approves.

@pitbulk
Copy link
Collaborator

pitbulk commented Sep 22, 2022

I had a conversation today with Tat Ng (VP Engineering) and it was cool, I hope to have such access soon.

@mrmoss
Copy link

mrmoss commented Sep 22, 2022

Nice. Maybe don't fork just yet then :)

@pitbulk
Copy link
Collaborator

pitbulk commented Jan 2, 2023

Fixed at #650

@pitbulk pitbulk closed this as completed Jan 2, 2023
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

No branches or pull requests

4 participants