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

Regression: login via LDAP not possible: '(LDAP Result Code 2 "Protocol Error": )' #5928

Closed
liquidat opened this Issue Feb 1, 2019 · 27 comments

Comments

7 participants
@liquidat
Copy link

liquidat commented Feb 1, 2019

  • Gitea version (or commit ref): dfad569 built with go1.11.5 : bindata, sqlite, sqlite_unlock_notify
  • Git version: 2.18.1
  • Operating system: offical docker image, docker.io/gitea/gitea, tag 1.7, image ID 044ab4ac3753
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

Description

I am not able to log in via LDAP anymore:

[...gitea/models/user.go:1544 SyncExternalUsers()] [E] LDAP Search failed unexpectedly! (LDAP Result Code 2 "Protocol Error": )

The LDAP problems came as a regression when I updated to 1.6. I thought that the backport of #5816 to 1.7.1 would solve my problems. But unfortunately this is not the case, an update of my container image to the latest 1.7 tag has the above mentioned log.

@lunny lunny added the kind/bug label Feb 3, 2019

@mootboy

This comment has been minimized.

Copy link

mootboy commented Feb 3, 2019

Piling on, running the arm7 binary I get:

gitea[2169]: 2019/02/03 19:09:50 ldap: recovered panic in processMessages: runtime error: invalid memory address or nil pointer dereference

When trying to login over LDAP.

@MarkusAmshove

This comment has been minimized.

Copy link

MarkusAmshove commented Feb 3, 2019

Does this mean updating to 1.7.1 isn't save when using ldap? Currently on 1.7.0

@OndrejSpanel

This comment has been minimized.

Copy link

OndrejSpanel commented Feb 4, 2019

I see the same issue. 1.7.0 working fine, LDAP authentication not working with 1.7.1

I am running on Debian with MySQL, downloading binaries from https://dl.gitea.io/gitea/.

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Feb 4, 2019

What LDAP server are you using?

@OndrejSpanel

This comment has been minimized.

Copy link

OndrejSpanel commented Feb 4, 2019

I am using OpenLDAP - openldap-2.4.31

@liquidat

This comment has been minimized.

Copy link
Author

liquidat commented Feb 4, 2019

My LDAP server: FreeIPA
The protocol error problem came with 1.6 and is now with 1.7.1, never tested with 1.7.0.

@MarkusAmshove

This comment has been minimized.

Copy link

MarkusAmshove commented Feb 4, 2019

I've done the upgrade to 1.7.1 and everything works fine.

SuSe Linux Enterprise 12
Postgres
Windows Active Directory

@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Feb 5, 2019

Hmm I wonder if this is something to do with SSH public key provision in LDAP.

Peeps with the failing LDAP could you check you're definitely on 1.7.1 and that your attributes are definitely correct - in particular if you don't have SSH keys in your LDAP ensure that attribute is empty.

@MarkusAmshove

This comment has been minimized.

Copy link

MarkusAmshove commented Feb 6, 2019

To add on to that and help troubleshooting, we don't have public keys in our AD

@OndrejSpanel

This comment has been minimized.

Copy link

OndrejSpanel commented Feb 6, 2019

you're definitely on 1.7.1

Yes, I was definitely on 1.7.1. I have upgraded from 1.6.x, once I realized LDAP login is not working for me, I downgraded to 1.7.0.

As for SSH keys, I have SSH access disabled on Gitea and I do not have any SSH keys in LDAP.

@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Feb 6, 2019

Thanks @OndrejSpanel, when you checked your configuration for LDAP in 1.7.1 the attribute was definitely blank and empty? I appreciate that it should be - but I wonder if what being set to say something that would represent the empty string rather than the empty string.

@OndrejSpanel

This comment has been minimized.

Copy link

OndrejSpanel commented Feb 6, 2019

I am afraid I do not understand what to check, I supposed you were talking about LDAP attributes and I do not see any SSH related attributes in our LDAP. What attribute is this - some Gitea configuration, or something in LDAP, or someplace else on our server? I may install 1.7.1 again if necessary, but I need to know what to check and what to report. I am not using SSH and I am not familiar with its configuration.

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Feb 6, 2019

@OndrejSpanel he meant this LDAP authorization source configuration attribute in Gitea:
image

@OndrejSpanel

This comment has been minimized.

Copy link

OndrejSpanel commented Feb 6, 2019

I definitely have this empty now in 1.7.0. Unless the upgrade is changing the value, it should be the same in 1.7.1 - I can check this if needed.

Note: I use LDAP (via BindDN)

@liquidat

This comment has been minimized.

Copy link
Author

liquidat commented Feb 6, 2019

@zeripath The container tag says 1.7.1, gitea itself calls the version "dfad569". I assume that is correct?
I verified that the option shown in the screenshot from @lafriks is empty.

@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Feb 7, 2019

Ok, so the error given out is slightly misleading (it's too far up to the callstack.) It's actually coming from here:

sr, err := l.Search(search)

Now the interesting part is on line 257 where there is a log trace that will reveal what it's actually asking your LDAP.

So, if your LDAP logs aren't being helpful at telling you what is going wrong, then we need to turn on trace and look for "Fetching Attributes" to see what attributes we say we're sending. It would be really helpful if you could check what your LDAP is getting though.

I'm thinking about our logging infrastructure at present, and yes, turning on trace is going to spew out a lot of unnecessary rubbish. I think we need to migrate to a much cleverer system.

@liquidat

This comment has been minimized.

Copy link
Author

liquidat commented Feb 9, 2019

Here is what my FreeIPA ldap error log shows:

[22:22:08.170978469] fd=112 slot=112 connection from 172.18.0.6 to 172.18.0.2
[22:22:08.171199824] op=0 BIND dn="uid=system,cn=sysaccounts,cn=etc,dc=bayz,dc=de" method=128 version=3
[22:22:08.223472706] op=0 RESULT err=0 tag=97 nentries=0 etime=0.0052434415 dn="uid=system,cn=sysaccounts,cn=etc,dc=bayz,dc=de"
[22:22:08.223738210] op=1 SRCH base="cn=users,cn=accounts,dc=bayz,dc=de" scope=2 filter="(&(objectClass=person)(uid=rwo))" attrs=ALL
[22:22:08.225467030] op=1 RESULT err=0 tag=101 nentries=1 etime=0.0001797298
[22:22:08.226078299] op=2 BIND dn="uid=rwo,cn=users,cn=accounts,dc=bayz,dc=de" method=128 version=3
[22:22:08.278423889] op=2 RESULT err=0 tag=97 nentries=0 etime=0.0052380180 dn="uid=rwo,cn=users,cn=accounts,dc=bayz,dc=de"
[22:22:08.278705323] op=3 SRCH base="(null)" scope=2 filter="(&(objectClass=person)(uid=rwo))", invalid attribute request
[22:22:08.278722888] op=3 RESULT err=2 tag=101 nentries=0 etime=0.0000084787
[22:22:08.279051788] op=-1 fd=112 closed - B1

The line with the SRCH base="(null)" might be the reason why login fails for me: a recent update of FreeIPA declines requests if there are more than one empty attribute in the request. And iirc I did update the FreeIPA server. This empty attribute situation was a problem for nextcloud as well, see for example here. According to this comment this was tackled in go-ldap but I am not entirely sure if this was fixed properly?

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Feb 10, 2019

we could probably try to upgrade to "gopkg.in/ldap.v3" to see if that resolves your issue

@liquidat

This comment has been minimized.

Copy link
Author

liquidat commented Feb 11, 2019

Sounds like a plan. Is there any way I could test this, given that I am running containers?

zeripath added a commit to zeripath/gitea that referenced this issue Feb 17, 2019

Move to ldap.v3 to fix go-gitea#5928
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Feb 17, 2019

@liquidat is there any way you could test that PR? It simply does what @lafriks suggests. If it works we can get that backported and then work on getting 1.8 ready.

@mootboy

This comment has been minimized.

Copy link

mootboy commented Feb 18, 2019

Currently compiling go1.11.5 on my pi3 to then build and test the @zeripath branch.

I'm running slapd 2.4.44+dfsg-5+deb9u1 on arm7 for the record.

@mootboy

This comment has been minimized.

Copy link

mootboy commented Feb 18, 2019

Fix confirmed as working on arm7 with postgresql and slapd @zeripath

@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Feb 18, 2019

Could you pop that comment on the pr.

@zeripath zeripath closed this in 22770c3 Feb 18, 2019

zeripath added a commit to zeripath/gitea that referenced this issue Feb 18, 2019

Move to ldap.v3 to fix go-gitea#5928 (go-gitea#6105)
Signed-off-by: Andrew Thornton <art27@cantab.net>

zeripath added a commit that referenced this issue Feb 18, 2019

Move to ldap.v3 to fix #5928 (#6105) (#6107)
Signed-off-by: Andrew Thornton <art27@cantab.net>

@lafriks lafriks added this to the 1.7.3 milestone Feb 18, 2019

@mootboy

This comment has been minimized.

Copy link

mootboy commented Feb 18, 2019

Could you pop that comment on the pr.

Sorry, a bit late here, do you want a post-merge comment on the PR?

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Feb 18, 2019

@mootboy all good, no need

@zeripath

This comment has been minimized.

Copy link
Contributor

zeripath commented Feb 18, 2019

Nah it's fine. It was just in case people weren't approving because they weren't sure it would work.

It's a shame we were never able to get a test case to reproduce the problem in our treat suite.

@mootboy

This comment has been minimized.

Copy link

mootboy commented Feb 18, 2019

If I was more familiar with go I would give it a shot, OTOH, LDAPv2 was considered dead in 2003 :-P

(yeah I know it was the library version, poor sense of humour)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment