Ldap authentication

[jdoucerain]

This PR is to allow LDAP authentication of chisel client. The chisel server ldap configuration is maintained in a file pointed by the --ldap-config option The ldap-config file structure is as follow: BindDN for non anonymous search BindPassword for non anonymous search Url ldap server url BaseDN for search Filter ldap objects filter IDMapTo ldap attribute used as id CA file containing the PEM formatted certificates to trust when connecting to ldap server Insecure true if we skip the certificates validation

The users are authentication against the ldap server. The authorization pattern is unchanged (in authfile) In the auth file we maintain the userid and the remotes. The password is left to an empty string

[kblanckaert]

Hi,

Interesting feature! Would really like to see this added as well, but I wouldn't completely disable the password authentication functionality once LDAP configuration is in place.

The documentation about how to use this could also be made more clear:

• An example of the ldap.json configuration file
• The fact that the LDAP users have to be added to the users.json file
• The fact that LDAP over port 389 doesn't seem to work (TLS issues when trying)

I've added some comments about the above mentioned improvements in your code.

[kblanckaert]

@jpillora Can you merge this please? Thanks!

[kblanckaert]

binary size difference: 12214.113 KB (master) -> 12397.106 KB (ldap-authentication)

[jdoucerain]

@jpillora could you please have a look at the bit improved version of the password validation ? Thanks

[jdoucerain]

@jpillora thanks for the review you did and I tried to follow what you said. Could you please review this change again ? Thanks again

[jpillora]

Sorry @jdoucerain I was going to merge a bunch of PRs and release though tests are broken for latest versions of Go and I haven't had time to fix them https://github.com/jpillora/chisel/issues/390

[jdoucerain]

Hi @jpillora would you have some time to check if you could merge this branch ?

[jdoucerain]

Hi @jpillora could you please check if you could merge this branch ? Thanks

[jpillora]

Looks pretty good, will try take a look toknight

I think the server config should only have the ldap config, the path string should be in main (cli code) - settings can export a LoadLDAP(path) LDAPConfig which is used my main, and by server package consumers

I’m happy to do this change if you’d like

[jdoucerain]

@jpillora please go ahead if you have some bandwidth. Thanks

[jpillora]

looking at the is PR now. it appears you still have to define users in the --auth or --authfile, and the "local" password still works. ldap just adds an additional password that will now work. can you confirm this is correct and intended?

[jpillora]

IMO, i think ldap should work in either one of two ways:

1. using ldap should be used if the local user is not found. so by default all user attempts use ldap. config.AllowedUsers []string could be added to whitelist users. maybe you could place remote configuration as a user attribute? otherwise all remotes are allowed
2. using ldap should force all "local" passwords to be blank. must be --auth foo if --auth foo:bar then fails to start. similarly with --authfile

[jdoucerain]

looking at the is PR now. it appears you still have to define users in the --auth or --authfile, and the "local" password still works. ldap just adds an additional password that will now work. can you confirm this is correct and intended?

[jdoucerain]

Yes this is intentional. We have to cover case where the users could or could not be in the enterprise directory.

[jdoucerain]

And the presence in the directory is not enough for us to allow anyone to use chisel. This is why we are maintaining the authfile for each individual. This is our use case.

[jdoucerain]

I think using ldap as authentication only and applying a default parameterized remote is another use case to be developed. And limiting the authentication to ldap only and blocking the ability to authenticate with local password when ldap is active is also another use case.

[jpillora]

gotcha, would you prefer option 2?

I’m finding it hard to see a good use case for two passwords

[jdoucerain]

Yes I agree with option 2 I will update the PR consequently

[jdoucerain]

@jpillora option 2 (using ldap should force all "local" passwords to be blank. must be --auth foo if --auth foo:bar then fails to start. similarly with --authfile) is implemented.

[jpillora]

Thanks! Hopefully will get some more time soon

On Wed, 12 Jul 2023 at 12:24 am jdoucerain @.***> wrote:

@jpillora https://github.com/jpillora option 2 (using ldap should force all "local" passwords to be blank. must be --auth foo if --auth foo:bar then fails to start. similarly with --authfile) is implemented.

— Reply to this email directly, view it on GitHub https://github.com/jpillora/chisel/pull/369#issuecomment-1630930929, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE2X4YXRVOQFGEFJUDMOBTXPVO23ANCNFSM52HPYV4Q . You are receiving this because you were mentioned.Message ID: @.***>