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

ipa-kdb: use predefined filters for a wild-card searches #5351

Closed
wants to merge 1 commit into from

Conversation

abbra
Copy link
Contributor

@abbra abbra commented Dec 17, 2020

In case we've got a principal name as '*', we don't need to specify
the principal itself, use pre-defined filter for a wild-card search.

Previously, we had to escape the '*' as specifying it with an explicit
matching rule would have violated RFC 4515 section 3. However, since we
don't really need to specify a different matching rule for a wild-card
search, we can remove this part completely.

Fixes: https://pagure.io/freeipa/issue/8624

Signed-off-by: Alexander Bokovoy abokovoy@redhat.com

@abbra abbra added needs review Pull Request is waiting for a review ipa-4-9 Mark for backport to ipa 4.9 ipa-4-8 Mark for backport to ipa 4.8 labels Dec 17, 2020
Comment on lines 59 to 64
#define PRINC_TGS_SEARCH_FILTER_WILD_EXTRA "(&(|(objectclass=krbprincipalaux)" \
"(objectclass=krbprincipal)" \
"(objectclass=ipakrbprincipal))" \
"(|(ipakrbprincipalalias=*)" \
"(krbprincipalname==*))" \
"%s)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aligment of the strings is a bit off. First line is indented one more than the rest, last line is indented one space less.

@abbra
Copy link
Contributor Author

abbra commented Dec 17, 2020

@tiran I updated the patch and also followed your IRC comment about the fact that we can reduce duplication of filter strings, indeed.

@abbra abbra added the re-run Trigger a new run of PR-CI label Dec 17, 2020
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Dec 17, 2020
@frozencemetery
Copy link
Contributor

Maybe I'm missing something but this doesn't seem to fix the issue.

[root@ipa freeipa]# git show | head -n1
commit 5cc511f4d85f5af8aee152e7d764c393e74007a3
[root@ipa freeipa]# rpm -q freeipa-server
freeipa-server-4.10.0.dev202012171559+git5cc511f-0.fc34.x86_64
[root@ipa freeipa]# ipactl restart
Restarting Directory Service
Restarting krb5kdc Service
Restarting kadmin Service
Restarting httpd Service
Restarting ipa-custodia Service
Restarting pki-tomcatd Service
Restarting ipa-otpd Service
ipa: INFO: The ipactl command was successful
[root@ipa freeipa]# kadmin.local getprincs
ldap/ipa.example.test@EXAMPLE.TEST
dogtag/ipa.example.test@EXAMPLE.TEST
HTTP/ipa.example.test@EXAMPLE.TEST
[root@ipa freeipa]# 

@abbra
Copy link
Contributor Author

abbra commented Dec 17, 2020

Can you provide output from /var/lib/dirsrv/slapd-EXAMPLE-TEST/access log that corresponds to this time period? It would be an LDAPI connection. access log is cached so the results appear a bit later than you'd run the tool

@frozencemetery
Copy link
Contributor

frozencemetery commented Dec 17, 2020

[17/Dec/2020:16:21:50.915147469 +0000] conn=37 AUTOBIND dn="cn=Directory Manager"
[17/Dec/2020:16:21:50.915150445 +0000] conn=37 op=0 BIND dn="cn=Directory Manager" method=sasl version=3 mech=EXTERNAL
[17/Dec/2020:16:21:50.915180057 +0000] conn=37 op=0 RESULT err=0 tag=97 nentries=0 wtime=0.000215561 optime=0.000058602 etime=0.000273229 dn="cn=Directory Manager"
[17/Dec/2020:16:21:50.915302921 +0000] conn=37 op=1 SRCH base="cn=EXAMPLE.TEST,cn=kerberos,dc=example,dc=test" scope=0 filter="(objectClass=*)" attrs=ALL
[17/Dec/2020:16:21:50.915405218 +0000] conn=37 op=1 RESULT err=0 tag=101 nentries=1 wtime=0.000049049 optime=0.000104275 etime=0.000152370
[17/Dec/2020:16:21:50.915633641 +0000] conn=37 op=2 SRCH base="cn=ipaConfig,cn=etc,dc=example,dc=test" scope=0 filter="(objectClass=*)" attrs="ipaConfigString ipaKrbAuthzData ipaUserAuthType"
[17/Dec/2020:16:21:50.915706451 +0000] conn=37 op=2 RESULT err=0 tag=101 nentries=1 wtime=0.000103210 optime=0.000074242 etime=0.000175974
[17/Dec/2020:16:21:50.915858393 +0000] conn=37 op=3 SRCH base="dc=example,dc=test" scope=2 filter="(objectClass=ipaNTDomainAttrs)" attrs="ipaNTFlatName ipaNTFallbackPrimaryGroup ipaNTSecurityIdentifier"
[17/Dec/2020:16:21:50.916078934 +0000] conn=37 op=3 RESULT err=0 tag=101 nentries=0 wtime=0.000086313 optime=0.000221653 etime=0.000306402
[17/Dec/2020:16:21:50.916274413 +0000] conn=37 op=4 SRCH base="cn=EXAMPLE.TEST,cn=kerberos,dc=example,dc=test" scope=0 filter="(krbMKey=*)" attrs="krbMKey"
[17/Dec/2020:16:21:50.916355721 +0000] conn=37 op=4 RESULT err=0 tag=101 nentries=1 wtime=0.000058257 optime=0.000082609 etime=0.000139432
[17/Dec/2020:16:21:50.916619493 +0000] conn=37 op=5 SRCH base="dc=example,dc=test" scope=2 filter="(&(|(objectClass=krbprincipalaux)(objectClass=krbprincipal)(objectClass=ipakrbprincipal))(|(ipaKrbPrincipalAlias=K/M@EXAMPLE.TEST)(krbPrincipalName:caseIgnoreIA5Match:=K/M@EXAMPLE.TEST)))" attrs="krbPrincipalName krbCanonicalName krbUPEnabled krbPrincipalKey krbTicketPolicyReference krbPrincipalExpiration krbPasswordExpiration krbPwdPolicyReference krbPrincipalType krbPwdHistory krbLastPwdChange krbPrincipalAliases krbLastSuccessfulAuth krbLastFailedAuth krbLoginFailedCount krbPrincipalAuthInd krbExtraData krbLastAdminUnlock krbObjectReferences krbTicketFlags krbMaxTicketLife krbMaxRenewableAge uid nsAccountLock passwordHistory ipaKrbAuthzData ipaUserAuthType ipatokenRadiusConfigLink krbAuthIndMaxT..."
[17/Dec/2020:16:21:50.916900076 +0000] conn=37 op=5 RESULT err=0 tag=101 nentries=1 wtime=0.000125152 optime=0.000282529 etime=0.000406124
[17/Dec/2020:16:21:50.992205590 +0000] conn=37 op=6 SRCH base="dc=example,dc=test" scope=2 filter="(&(|(objectClass=krbprincipalaux)(objectClass=krbprincipal)(objectClass=ipakrbprincipal))(|(ipaKrbPrincipalAlias=*)(krbPrincipalName==*)))" attrs="krbPrincipalName krbCanonicalName krbUPEnabled krbPrincipalKey krbTicketPolicyReference krbPrincipalExpiration krbPasswordExpiration krbPwdPolicyReference krbPrincipalType krbPwdHistory krbLastPwdChange krbPrincipalAliases krbLastSuccessfulAuth krbLastFailedAuth krbLoginFailedCount krbPrincipalAuthInd krbExtraData krbLastAdminUnlock krbObjectReferences krbTicketFlags krbMaxTicketLife krbMaxRenewableAge uid nsAccountLock passwordHistory ipaKrbAuthzData ipaUserAuthType ipatokenRadiusConfigLink krbAuthIndMaxT..."
[17/Dec/2020:16:21:50.992723581 +0000] conn=37 op=6 RESULT err=0 tag=101 nentries=3 wtime=0.000127741 optime=0.000521815 etime=0.000648130 notes=U details="Partially Unindexed Filter"
[17/Dec/2020:16:21:50.992995347 +0000] conn=37 op=7 SRCH base="cn=EXAMPLE.TEST,cn=kerberos,dc=example,dc=test" scope=0 filter="(objectClass=krbticketpolicyaux)" attrs="krbMaxTicketLife krbMaxRenewableAge krbTicketFlags krbAuthIndMaxTicketLife krbAuthIndMaxRenewableAge"
[17/Dec/2020:16:21:50.993141665 +0000] conn=37 op=7 RESULT err=0 tag=101 nentries=1 wtime=0.000133082 optime=0.000167697 etime=0.000299670
[17/Dec/2020:16:21:50.993377235 +0000] conn=37 op=8 SRCH base="cn=EXAMPLE.TEST,cn=kerberos,dc=example,dc=test" scope=0 filter="(objectClass=krbticketpolicyaux)" attrs="krbMaxTicketLife krbMaxRenewableAge krbTicketFlags krbAuthIndMaxTicketLife krbAuthIndMaxRenewableAge"
[17/Dec/2020:16:21:50.993446106 +0000] conn=37 op=8 RESULT err=0 tag=101 nentries=1 wtime=0.000108437 optime=0.000070969 etime=0.000178265
[17/Dec/2020:16:21:50.993583196 +0000] conn=37 op=9 SRCH base="cn=EXAMPLE.TEST,cn=kerberos,dc=example,dc=test" scope=0 filter="(objectClass=krbticketpolicyaux)" attrs="krbMaxTicketLife krbMaxRenewableAge krbTicketFlags krbAuthIndMaxTicketLife krbAuthIndMaxRenewableAge"
[17/Dec/2020:16:21:50.993640846 +0000] conn=37 op=9 RESULT err=0 tag=101 nentries=1 wtime=0.000054905 optime=0.000059937 etime=0.000113690
[17/Dec/2020:16:21:50.996960798 +0000] conn=37 op=10 UNBIND
[17/Dec/2020:16:21:50.996972673 +0000] conn=37 op=10 fd=106 closed error - U1

@abbra
Copy link
Contributor Author

abbra commented Dec 17, 2020

Thanks, there is excessive '=' in the _WILD_EXTRA filter term. I'll fix tonight

In case we've got a principal name as '*', we don't need to specify
the principal itself, use pre-defined filter for a wild-card search.

Previously, we had to escape the '*' as specifying it with an explicit
matching rule would have violated RFC 4515 section 3. However, since we
don't really need to specify a different matching rule for a wild-card
search, we can remove this part completely.

Use this change as an opportunity to simplify the code and reduce
number of duplicated filter constants -- if extra filter is NULL, we can
simply pass "" and use _EXTRA filter constants to format the final
filter.

Fixes: https://pagure.io/freeipa/issue/8624

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Copy link
Contributor

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, working now.

@abbra abbra added re-run Trigger a new run of PR-CI and removed needs review Pull Request is waiting for a review labels Dec 18, 2020
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Dec 18, 2020
@abbra abbra added the re-run Trigger a new run of PR-CI label Dec 18, 2020
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Dec 18, 2020
@abbra abbra added the ack Pull Request approved, can be merged label Dec 18, 2020
@abbra
Copy link
Contributor Author

abbra commented Dec 18, 2020

Added ACK based on @frozencemetery approval.

@abbra abbra added the pushed Pull Request has already been pushed label Dec 18, 2020
@abbra
Copy link
Contributor Author

abbra commented Dec 18, 2020

master:

  • 35362d3 ipa-kdb: use predefined filters for a wild-card searches

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-8 Mark for backport to ipa 4.8 ipa-4-9 Mark for backport to ipa 4.9 pushed Pull Request has already been pushed
Projects
None yet
4 participants