Replacing sds with another string library?

[bufferoverflow]

I'm unsure if I should ask here or within https://github.com/CyberNinjas/pam_aad .

Nevertheless, I'm interested to know if you have some plans to replace https://github.com/antirez/sds with e.g. plain curl or another alternative. I see no actvity on sds anymore and no Linux distribution packaging it, so I have the feeling this is somehow a blocker for further adoption.

[oxr463]

Hi @bufferoverflow!

I originally released pam_aad while working at that company. But now we are an independent project.

There are no plans to replace sds at the moment. We are using it to make working with strings simpler.

We already use libcurl (See: https://github.com/aad-for-linux/pam_aad/blob/master/pam_aad.c#L1).

[bufferoverflow]

thanks for the insights @oxr463 ! so the answer to my question https://github.com/CyberNinjas/pam_aad/issues/64 is no and the new home is here? If so it would be cool to have that info within the https://github.com/CyberNinjas/pam_aad/blob/master/README.md

regarding sds, I still have the feeling that this is a blocker to package pam_aad within the distros, do you know about some efforts on sds for the distros such as Debian?

[oxr463]

I don't have access to that repository anymore, but I just left a comment on that issue.

I packaged sds for a couple of distributions myself (Alpine, Debian, Gentoo, Ubuntu).

We could easily replace sds if that was needed.

[bufferoverflow]

I tried https://launchpad.net/~lramage/+archive/ubuntu/sds today but using focal was a no success. I really have some doubts about using sds and I would love to see pam_aad without that dependency to get it into popular distros such as Debian and it's derivatives, and of course others as well.

A colleague made an extension to pam_aad but we have been unsure on where we shall contribute to and I have some doubts regarding sds in the long run.

Great to see this one here is actively maintained!

[oxr463]

Please open an issue for the focal issue here: https://launchpad.net/~lramage/+archive/ubuntu/sds or https://gitlab.com/oxr463/sds/-/issues.

Feel free to reach out on Gitter (See: https://gitter.im/aad-for-linux) to discuss, or have him submit a new issue at https://github.com/aad-for-linux/pam_aad/issues.

[bufferoverflow]

I made https://gitlab.com/oxr463/sds/-/issues/3 regarding sds ppa.

Nevertheless, I still believe getting rid of sds is worth to do, getting sds into the distros without ppa is already a huge effort that could be better spent on packaging pam_aad within the distros.

[bufferoverflow]

@oxr463 Do you plan now to replace sds are shall we prepare a PR?

[oxr463]

@oxr463 Do you plan now to replace sds are shall we prepare a PR?

I was planning on vendoring it in the project, but if you can create a PR that would be helpful.

What do you plan on replacing it with?

[bufferoverflow]

I was thinking about curl as it is already available on all distros and used by other pam modules as well.

[oxr463]

I was thinking about curl as it is already available on all distros and used by other pam modules as well.

I think I might be misunderstanding. We already use libcurl for all of the HTTPS requests that the module makes. The sds library is for handling strings? How would you handle strings with libcurl?

[bufferoverflow]

:smile: I'm sorry too busy with other stuff right now! I look into this and come back with a proposal.

[bufferoverflow]

within auth_bearer_request we could e.g., do something like this:

-    sds post_body = sdsnew("resource=" RESOURCE);
-    post_body = sdscat(post_body, "&code=");
-    post_body = sdscat(post_body, code);
-    post_body = sdscat(post_body, "&client_id=");
-    post_body = sdscat(post_body, client_id);
-    post_body = sdscat(post_body, "&grant_type=device_code");
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "resource", CURLFORM_COPYCONTENTS, RESOURCE, CURLFORM_END);
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "code", CURLFORM_COPYCONTENTS, code, CURLFORM_END);
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "client_id", CURLFORM_COPYCONTENTS, client_id, CURLFORM_END);
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "grant_type", CURLFORM_COPYCONTENTS, "device_code", CURLFORM_END);

[oxr463]

within auth_bearer_request we could e.g., do something like this:

-    sds post_body = sdsnew("resource=" RESOURCE);
-    post_body = sdscat(post_body, "&code=");
-    post_body = sdscat(post_body, code);
-    post_body = sdscat(post_body, "&client_id=");
-    post_body = sdscat(post_body, client_id);
-    post_body = sdscat(post_body, "&grant_type=device_code");
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "resource", CURLFORM_COPYCONTENTS, RESOURCE, CURLFORM_END);
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "code", CURLFORM_COPYCONTENTS, code, CURLFORM_END);
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "client_id", CURLFORM_COPYCONTENTS, client_id, CURLFORM_END);
+    curl_formadd (&post_body, &last, CURLFORM_COPYNAME, "grant_type", CURLFORM_COPYCONTENTS, "device_code", CURLFORM_END);

That seems like a good solution. Would it completely remove dependency on sds?

[bufferoverflow]

I did not check all sds use cases right now, but I think this approach could be used in several areas to reduce the sds usage with e.g. a first PR and then maybe have one or two cases where we need another solution such as plain c.

[oxr463]

I did not check all sds use cases right now, but I think this approach could be used in several areas to reduce the sds usage with e.g. a first PR and then maybe have one or two cases where we need another solution such as plain c.

Sounds good to me. Let's see how it goes!

[oxr463]

Alright, here is the plan I have in mind:

• [x] Update sds package for focal (0.0.4)
• [ ] Bundle sds and add autotools option (0.0.6)
• [ ] Slowly rewrite code to remove dependency (1.0.0)

[oxr463]

@suleohis can you open up a pull request to replace sds with curl_formadd like in the comment above?

Reference(s):

• https://curl.se/libcurl/c/curl_formadd.html