-
Notifications
You must be signed in to change notification settings - Fork 332
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
Allow hosts to read DNS records for IP SAN #4355
Conversation
aa81f32
to
67a8ded
Compare
LGTM |
f0beae3
to
5dbc04a
Compare
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
5dbc04a
to
fa4e6f8
Compare
You add another SAN for the host but it is currently unused. Is that some future change? Should it be removed since it is unused? |
5067b14
to
8547f51
Compare
# assert dnsnames == {self.clients[0].hostname, self.altname} | ||
assert dnsnames == {self.clients[0].hostname} | ||
ipaddrs = set(ext.value.get_values_for_type(x509.IPAddress)) | ||
assert ipaddrs == {self.clients[0].ip} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assert for ipaddrs
fails because you get str
in the set on the right side and IPv4Address
instance on the lest side:
> assert ipaddrs == {self.clients[0].ip}
E AssertionError: assert {IPv4Address(...168.122.193')} == {'192.168.122.193'}
E Extra items in the left set:
E IPv4Address('192.168.122.193')
E Extra items in the right set:
E '192.168.122.193'
E Full diff:
E - {IPv4Address('192.168.122.193')}
E + {'192.168.122.193'}
I think you either need to convert self.clients[0].ip
to an IPAddress instance:
from ipaddress import ip_address
...
assert ipaddrs == {ip_address(self.clients[0].ip)}
8547f51
to
e548475
Compare
LGTM |
For SAN IPAddress extension the cert plugin verifies that the IP address matches the host entry. Certmonger uses the host principal to authenticate and retrieve certificates. But the host principal did not have permission to read DNS entries from LDAP. Allow all hosts to read some entries from active DNS records. Fixes: https://pagure.io/freeipa/issue/8098 Signed-off-by: Christian Heimes <cheimes@redhat.com>
e548475
to
c153328
Compare
master:
|
For SAN IPAddress extension the cert plugin verifies that the IP address
matches the host entry. Certmonger uses the host principal to
authenticate and retrieve certificates. But the host principal did not
have permission to read DNS entries from LDAP.
Allow all hosts to read some entries from active DNS records.
Fixes: https://pagure.io/freeipa/issue/8098
Signed-off-by: Christian Heimes cheimes@redhat.com