Skip to content

Support enclosing the IPv6 address in brackets #627

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

Closed
William-Francis opened this issue Jul 30, 2019 · 4 comments
Closed

Support enclosing the IPv6 address in brackets #627

William-Francis opened this issue Jul 30, 2019 · 4 comments

Comments

@William-Francis
Copy link

Shibboleth provider sends IPv6 address in brackets, like:

 <saml2:SubjectConfirmationData Address="[2001:8003:5555:9999:555a:5555:c77:d5c5]" InResponseTo="xxx" NotOnOrAfter="2019-07-02T12:12:12.966Z" Recipient="xxx"/>

The standard they are using is defined here: https://tools.ietf.org/html/rfc4038#section-5.1
in short:
"The IP address parsers should support enclosing the IPv6 address in brackets, even when the address is not used in conjunction with a port number."

Code Version

Master:
https://github.com/IdentityPython/pysaml2/blob/master/src/saml2/saml.py#L982

Expected Behavior

Check if the address is in brackets, and run the valid_ipv6() function on the item inside the list, then pass.

Current Behavior

raise ShouldValueError("Not an IPv4 or IPv6 address")

Possible Solution

Maybe not the most optimal, but should work.
It would also be possible to extract the item from the list before assigning self.address in the constructor function.

    def verify(self):
        if self.address:
            # dotted-decimal IPv4 or RFC3513 IPv6 address
            if valid_ipv4(self.address) or valid_ipv6(self.address):
                pass
            elif (isinstance(self.address, list) and self.address[0] and valid_ipv6(self.address[0])):
                pass
            else:
                raise ShouldValueError("Not an IPv4 or IPv6 address")
        elif self.dns_name:
            valid_domain_name(self.dns_name)
        return SubjectLocalityType_.verify(self)

Steps to Reproduce

<saml2:SubjectConfirmationData Address="[2001:8003:5555:9999:555a:5555:c77:d5c5]" InResponseTo="xxx" NotOnOrAfter="2019-07-02T12:12:12.966Z" Recipient="xxx"/>
Send through address with IPv6 in brackets.

@William-Francis
Copy link
Author

Just a note, I've managed to get the provider to send the IPv6 without the brackets, but this issue might help someone else.

@ioparaskev
Copy link
Contributor

Another choice to solve this would be to refactor the verify to the following:

from functools import reduce
from operator import add

def verify(self):
    if self.address:
        # dotted-decimal IPv4 or RFC3513 IPv6 address
        if valid_ipv4(self.address) or valid_ipv6(reduce(add, self.address)):
            pass
        else:
            raise ShouldValueError("Not an IPv4 or IPv6 address")
    elif self.dns_name:
        valid_domain_name(self.dns_name)

    return SubjectLocalityType_.verify(self)

an example of how this reduce call works:

In [1]: a = ["lalalalla"]

In [2]: b = "lalalalal"

In [3]: from functools import reduce

In [4]: from operator import add

In [5]: reduce(add, a)
Out[5]: 'lalalalla'

In [6]: reduce(add, b)
Out[6]: 'lalalalal'

@c00kiemon5ter
Copy link
Member

c00kiemon5ter commented Aug 19, 2019

We must normalize the address property to a single type, at the time of creation of the object. We should then call verify_address which would in turn call valid_ipv4(address) valid_ipv6(address) else raise an exception. valid_ipv* functions will work with that single type; there is no need to check for different forms of the IP. The same checks may be needed elsewhere; instead of duplicating them, we just set a single form that will be used internally.

PS: We should also separate the validation of the dns_name from that if construct.

@c00kiemon5ter
Copy link
Member

fixed by #653

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

3 participants