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

CURLOPT_SSH_HOSTKEY_FUNCTION #7959

Closed
wants to merge 1 commit into from
Closed

CURLOPT_SSH_HOSTKEY_FUNCTION #7959

wants to merge 1 commit into from

Conversation

mickae1
Copy link
Contributor

@mickae1 mickae1 commented Nov 4, 2021

The callback defined by CURLOPT_SSH_HOSTKEYCHECK_FUNCTION is called to check wether or not the connexion should continue.

the host key is passed in argument with a custom handle for the application.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Nov 4, 2021

I don't know how to squash all those commit ...

@bagder
Copy link
Member

@bagder bagder commented Nov 4, 2021

@mickae1 do a git rebase -i origin/master in your branch, squash them all as you see fit and then you force-push the remaining one(s) here again

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Nov 4, 2021

@mickae1 do a git rebase -i origin/master in your branch, squash them all as you see fit and then you force-push the remaining one(s) here again

thanks, it's done :) .

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Nov 4, 2021

I don't get what this error means :
lib1521.c: In function ‘test’: lib1521.c:4346:26: error: ‘ssh_hostkeycheck_cb’ undeclared (first use in this function) 4346 | ssh_hostkeycheck_cb); | ^~~~~~~~~~~~~~~~~~~

Where is this file lib1521.c ?? dynamically created ? From which conf file ?

@bagder
Copy link
Member

@bagder bagder commented Nov 4, 2021

lib1521.c is generated by mk-lib1521.pl

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Nov 5, 2021

lib1521.c is generated by mk-lib1521.pl

Ok, Corrected. But there is a lot of error due to ".libs/libcurlu.a(libcurlu_la-libssh2.o) has no symbols"

Some times :
checksrc: 0 errors and 2 warnings
checksrc: 0 errors and 8 warnings suppressed
*** Error code 5

I don't understand why .....

I've a warning about:
./easyoptions.c:281:88: warning: Longer than 79 columns (LONGLINE)
{"SSH_HOSTKEYCHECK_FUNCTION", CURLOPT_SSH_HOSTKEYCHECK_FUNCTION, CURLOT_FUNCTION, 0},
./setopt.c:2500:85: warning: Longer than 79 columns (LONGLINE)
data->set.ssh_hostkeycheck_func = va_arg(param, curl_ssh_hostkeycheck_callback);

But I don't know how to fix it .....

other error like:
@azure-pipelinesazure-pipelines
/ curl.curl (linux ubuntu default)
Build log #L1148
Bash exited with code '2'.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Nov 5, 2021

Do you think it's possible to put this pull in the version 7.79.2 ?

@bagder
Copy link
Member

@bagder bagder commented Nov 5, 2021

The next release will be 7.80.0 and it has been closed for new features (such as this) for a few weeks already. Your PR's earliest chance is for a 7.81.0 release. Which might ship on January 5, 2022.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Nov 5, 2021

7.81.0

Ok, the files has been modified accordingly .

docs/libcurl/curl_easy_setopt.3 Outdated Show resolved Hide resolved
int keylen); /*length of the key*/

CURLcode curl_easy_setopt(CURL *handle, CURLOPT_SSH_HOSTKEYCHECK_FUNCTION,
ssh_hostkeycheck_callback);
Copy link
Member

@bagder bagder Nov 5, 2021

Choose a reason for hiding this comment

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

There's missing .fi there as a last line of the SYNOPSIS.

Copy link
Contributor Author

@mickae1 mickae1 Nov 5, 2021

Choose a reason for hiding this comment

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

but in this file it's working, there is no .fi
I've clone the file and change only the text.

https://github.com/curl/curl/blob/master/docs/libcurl/opts/CURLOPT_SSH_KEYFUNCTION.3#L62

Copy link
Contributor Author

@mickae1 mickae1 Nov 8, 2021

Choose a reason for hiding this comment

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

I tried :

.SH SYNOPSIS
.nf
#include <curl/curl.h>

CURLcode curl_easy_setopt(CURL *handle, CURLOPT_SSH_HOSTKEYDATA, void *pointer);
.fi

docs/libcurl/opts/CURLOPT_SSH_HOSTKEYCHECK.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_SSH_HOSTKEYCHECK.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_SSH_HOSTKEYCHECK.3 Outdated Show resolved Hide resolved
include/curl/curl.h Outdated Show resolved Hide resolved
lib/setopt.c Outdated Show resolved Hide resolved
lib/vssh/libssh2.c Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_SSH_HOSTKEYCHECK.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_SSH_HOSTKEYCHECK.3 Outdated Show resolved Hide resolved
@mickae1 mickae1 force-pushed the master branch 8 times, most recently from 8212073 to c3b6bb0 Compare Nov 8, 2021
@mickae1 mickae1 changed the title CURLOPT_SSH_HOSTKEYCHECK_FUNCTION CURLOPT_SSH_HOSTKEY_FUNCTION Nov 8, 2021
@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Nov 10, 2021

@bagder , can you give me some hints to make this pull successful ?

Thank you,

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Dec 14, 2021

The next release will be 7.80.0 and it has been closed for new features (such as this) for a few weeks already. Your PR's earliest chance is for a 7.81.0 release. Which might ship on January 5, 2022.

Hi @Badger,

Can you give me some news ? Because I would like to have this feature for the next release :)

Thanks,

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Dec 14, 2021

Can you give me some news ? Because I would like to have this feature for the next release :)

The featurewindow for the next release is closed, so this feature didn't make it in for the January 5 release.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Dec 14, 2021

Can you give me some news ? Because I would like to have this feature for the next release :)

The featurewindow for the next release is closed, so this feature didn't make it in for the January 5 release.

Ok, can you tell me what I've to do for the next next release ?

What release would it be ? 7.82.0 ?

@danielgustafsson
Copy link
Member

@danielgustafsson danielgustafsson commented Dec 14, 2021

Can you give me some news ? Because I would like to have this feature for the next release :)

The featurewindow for the next release is closed, so this feature didn't make it in for the January 5 release.

Ok, can you tell me what I've to do for the next next release ?

I haven't been following along so I can't comment on the status of the work, and what might remain. I'll try to find some time to review it to see.

What release would it be ? 7.82.0 ?

Correct, that's the next possible feature release.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Dec 14, 2021

@danielgustafsson thank you,

I've made the change needed, I will wait for your review, thank you !

@MAntoniak
Copy link
Contributor

@MAntoniak MAntoniak commented May 5, 2022

Also run checksrc script locally and resolve all warnings.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented May 5, 2022

@MAntoniak Thanks.

checksrc

I tried to start checksrc.bat on my windows machine, but I got no message :(

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented May 6, 2022

@MAntoniak @bagder, looks OK to me, no ?

Copy link
Member

@bagder bagder left a comment

I think this generally looks good. I only found some small things...

SCP and SFTP
.SH EXAMPLE
.nf
int curl_sshhostkeycallback(void *clientp,/* custom pointer passed with CURLOPT_SSH_HOSTKEYDATA */
Copy link
Member

@bagder bagder May 6, 2022

Choose a reason for hiding this comment

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

I think you should avoid curl_ as a prefix for an example function, as we use that for public API functions.

Copy link
Contributor Author

@mickae1 mickae1 May 6, 2022

Choose a reason for hiding this comment

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

I used the same syntax as this function ;
curl_sshkeycallback in the file curl.h line 867

Copy link
Contributor Author

@mickae1 mickae1 May 6, 2022

Choose a reason for hiding this comment

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

What should I do ?

Copy link
Member

@danielgustafsson danielgustafsson May 10, 2022

Choose a reason for hiding this comment

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

Just rename it without the leading curl_ prefix?

Pass a pointer to your callback function, which should match the prototype
shown above. It overrides CURLOPT_SSH_KNOWNHOSTS.

It gets called to act and decide for libcurl how to proceed.
Copy link
Member

@bagder bagder May 6, 2022

Choose a reason for hiding this comment

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

when is it called?

Copy link
Contributor Author

@mickae1 mickae1 May 6, 2022

Choose a reason for hiding this comment

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

ok => It gets called when the verification of the hostkey is needed.

}
.fi
.SH AVAILABILITY
Added in 7.82.0 , work only with libssh2 backend.
Copy link
Member

@bagder bagder May 6, 2022

Choose a reason for hiding this comment

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

Added in 7.84.0, works only with libssh2 backend.

lib/setopt.c Outdated Show resolved Hide resolved
lib/urldata.h Show resolved Hide resolved
@bagder
Copy link
Member

@bagder bagder commented May 6, 2022

Oh, and we need a test or two.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented May 6, 2022

@bagder I would like to add some test, but I could n't find any test for CURLOPT_SSH_KEYFUNCTION to use a as a model....

@mickae1 mickae1 force-pushed the master branch 2 times, most recently from 8098b08 to dbe4b54 Compare May 6, 2022
@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented May 6, 2022

I squashed every thing, it's better now :)

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented May 10, 2022

@bagder @MAntoniak please :)

SCP and SFTP
.SH EXAMPLE
.nf
int curl_sshhostkeycallback(void *clientp,/* custom pointer passed with CURLOPT_SSH_HOSTKEYDATA */
Copy link
Member

@danielgustafsson danielgustafsson May 10, 2022

Choose a reason for hiding this comment

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

Just rename it without the leading curl_ prefix?

docs/libcurl/opts/CURLOPT_SSH_HOSTKEYDATA.3 Outdated Show resolved Hide resolved
docs/libcurl/opts/CURLOPT_SSH_HOSTKEYFUNCTION.3 Outdated Show resolved Hide resolved
lib/vssh/libssh2.c Show resolved Hide resolved
@mickae1 mickae1 force-pushed the master branch 9 times, most recently from 715b306 to 162d4a5 Compare May 16, 2022
CURLOPT_SSH_HOSTKEYCHECK_FUNCTION

The callback defined by CURLOPT_SSH_HOSTKEYCHECK_FUNCTION is called to check wether or not the connexion should continue.

the host key is passed in argument with a custom handle for the application.

It overrides CURLOPT_SSH_KNOWNHOSTS
@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented May 19, 2022

@bagder please
@danielgustafsson please
@MAntoniak please

pleaase :)

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented May 31, 2022

@bagder @danielgustafsson @MAntoniak ?

please, I know you are busy , but I would like to have it for the next release :(

bagder
bagder approved these changes Jun 2, 2022
@bagder bagder closed this in 1544513 Jun 2, 2022
@bagder
Copy link
Member

@bagder bagder commented Jun 2, 2022

Thanks. I did some minor edits before this landed.

@mickae1
Copy link
Contributor Author

@mickae1 mickae1 commented Jun 2, 2022

thank you very much all of you for your help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants