CESNET / libnetconf2

C NETCONF library
BSD 3-Clause "New" or "Revised" License
203 stars 147 forks source link

Missing password callback #462

Closed awesomenode closed 7 months ago

awesomenode commented 8 months ago

Hi!

I'm trying to update to the latest libnetconf2 version, bit I've noticed that the SSH password callback has been removed, only the interactive callback is usable.

libnetconf now retrieves the password from the datastore, if I'm understanding it correctly, but I'm using a separate app that verifies the password and handles other checks, so the password callback was fairly useful.

I guess the interactive callback could replace it, but I don't need the extra functions of that. Do you perhaps plan to add the regular password callback to libnetconf2? Or we have to use the interactive callback from now on to support the same functionality?

Thanks!

Roytak commented 8 months ago

Hi, you are right, the password callback was removed. Michal may correct me later, but I do not think that we plan to add it back. So I suggest you use the interactive callback, it shouldn't be that much extra work to implement the same functionality.

michalvasko commented 8 months ago

That is correct, we decided that password should always perform just simple password check. You can use keyboard-interactive for any elaborate user password retrieval or something similar.

awesomenode commented 8 months ago

I'm trying to implement the keyboard auth callback, but for some reason it doesn't seem to work.

I attached the example that shows what I'm trying to do. I get the following in the server logs, when I try to connect using netopeer2-cli:

[INF]: Listening on 0.0.0.0:10830 for SSH connections. [INF]: Accepted a connection on 0.0.0.0:10830 from 127.0.0.1:48212. [INF]: Received an SSH message "request-service" of subtype "ssh-userauth". [INF]: Received an SSH message "request-auth" of subtype "none". [INF]: Received an SSH message "request-auth" of subtype "interactive". Failed to send an authentication request to client: test [INF]: Failed user "test" authentication attempt (#1). SSH log: ssh_auth_reply_default: ssh_auth_reply_default: Sending a auth failure. methods that can continue: keyboard-interactive SSH log: ssh_socket_unbuffered_write: ssh_socket_unbuffered_write: Enabling POLLOUT for socket SSH log: packet_send2: packet_send2: packet: wrote [type=51, len=32, padding_size=5, comp=26, payload=26] SSH log: ssh_packet_socket_callback: ssh_packet_socket_callback: packet: read type 1 [len=32,padding=11,comp=20,payload=20] SSH log: ssh_packet_process: ssh_packet_process: Dispatching handler for packet type 1 SSH log: ssh_packet_disconnect_callback: ssh_packet_disconnect_callback: Received SSH_MSG_DISCONNECT 11:Bye Bye SSH log: ssh_packet_disconnect_callback: ssh_packet_disconnect_callback: Received SSH_MSG_DISCONNECT: 11:Bye Bye [ERR]: Communication SSH socket unexpectedly closed.

I also tried connecting using ncclient (got 'No authentication methods available' error), and ssh client (got 'Permission denied (keyboard- interactive)', I know plain ssh is not supported properly, but for testing's sake I tried it anyway.

Also, when I specify a password with ncclient the program segfaults, because auth_password_compare_pwd() tries to read stored_pw, which isn't initialized.

Could you give me some advice on what I'm doing wrong? nc_interactive.zip

Roytak commented 8 months ago

Hello, the issue you're facing lies in your interactive callback.

The line Failed to send an authentication request to client: test from the output you provided comes from invalid parameters to the libssh function. I changed int ret = ssh_message_auth_interactive_request(msg, name, NULL, 1, (const char **)&prompt, NULL); to the following:

char *prompt = "Password: ";
char echo[] = {0};
int ret = ssh_message_auth_interactive_request(msg, name, "Enter your token:", 1, (const char **)&prompt, echo);

Another issue comes from the line int n_answers = ssh_userauth_kbdint_getnanswers(ssh_session);. You need to handle receiving the answers to the keyboard interactive challenge yourself in your callback. What I mean is doing something similar to this.

And lastly you forgot to call nc_server_init() for initialization (and to free the configuration data).

All in all, based on your issue we will:

I haven't tested it with clients other than netopeer2-cli, so can't help with that.

awesomenode commented 7 months ago

Thanks, it's working now. I've used nc_server_init_ctx(), doesn't that do the initialization too? Or it only initializes the context?

Roytak commented 7 months ago

Yes, nc_server_init_ctx() only initializes the context (loads the required modules, etc.). It does not initialize the server as a whole though. In the referenced commit I made an internal function public. This function should help with getting the answers to your interactive queries.

awesomenode commented 7 months ago

Thank you very much!

awesomenode commented 7 months ago

That is correct, we decided that password should always perform just simple password check. You can use keyboard-interactive for any elaborate user password retrieval or something similar.

Sadly, replacing the password auth method with keyboard-interactive is not an option, as it's not supported by ncclient (or MG-SOFT NETCONF Browser). I have to provide support for ncclient, because it's one of the most popular netconf clients, and used by a number of vendors, like Cisco, Nokia or Huawei.

Could you please put back the password callback from the older version please? Removing an existing auth method is not feasible for me.

Roytak commented 7 months ago

Hi, if I understand you correctly, then storing the password in the datastore is not an option for you. Well, then if we were to add the password callback back and still be compliant with ietf-netconf-server YANG module, how exactly should this be handled? When it comes to keyboard interactive, we defined our own YANG for this, so that it's possible to deduce which configured user supports it and which does not. As for the password method, if a password callback was set, would that mean that all the configured users must authenticate via the password method?

A section from the SSH client-authentication subtree states the following:

list user {
          key "name";
          description
            "A locally configured user.

             The server SHOULD derive the list of authentication
             'method names' returned to the SSH client from the
             descendant nodes configured herein, per Sections
             5.1 and 5.2 in RFC 4252.

             The authentication methods are unordered.  Clients
             must authenticate to all configured methods.
             Whenever a choice amongst methods arises,
             implementations SHOULD use a default ordering
             that prioritizes automation over human-interaction.";

Suppose password wasn't configured for a user, because you can not store it in the datastore, then:

The server SHOULD derive the list of authentication 'method names' returned to the SSH client from the descendant nodes configured herein, per Sections 5.1 and 5.2 in RFC 4252.

even if password wasn't configured for the user, but the callback was set, we could still send the password method as available to the client. It's just a question of whether the password callback MUST return success for every client in order for the authentication to be successful, or whether it should only apply to a subset of the configured users. I suppose it could be done similarly to the keyboard-interactive method with some extra YANG modelling to tell the difference if need be. Or some dummy value that won't be used could be stored as the user's password, so that it's clear he wishes to support the method.

I agree that a password callback can be useful. We'll discuss and try to find the solution to this with Michal when he is back.

awesomenode commented 7 months ago

I support remote authentication (like TACACS+), so local user configuration in this case is not sufficient. I need a solution that's compatible with remote auth, where we don't store the user locally.

jktjkt commented 7 months ago

@awesomenode , are you OK with using PAM? In my opinion, the "password/passphrase/kbd-interactive" authentication methods should all reach the same backend (PAM in case it was enabled at build time, or traditional UNIX getpwnam/getspuid/... in case of no PAM), and it should not matter whether a client auths via kbd-interactive or password SSH methods. If it was done that way, would it work for you?

Also, the current config implies that all auth methods have to succeed. That's not great for your use csae, and we would like to have an or method, something like this:

<cesnet:any-auth-of>
  <system-password/>
  <system-kbd-interactive/>
  <system-ssh-pubkey/>
</cesnet:any-auth-of>
Roytak commented 7 months ago

Hi, @awesomenode, we discussed this and we thought of a solution that might be plausible for you. The idea is to add new YANG, which makes it possible to configure SSH authentication without configuring the actual users.

These new nodes will be similar to the keyboard interactive nodes in a sense that which authentication backend will be used is up to libnetconf2 to decide, based on the system that is. Like @jktjkt mentioned, PAM will be one of these, so then if you were to configure PAM to authenticate via your desired protocol, it just might work (this is currently in the library, but the users have to be configured).

Currently the idea is that you will be able to either configure the users yourself or not. You will also be able to choose if you wish to use password or keyboard interactive authentication method. After that, it will be left to libnetconf2.

It would be simple just to add a password callback, however, that is not an option for someone who uses netopeer2. But still, this will take some time and is just an initial idea.