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

Fix libcurl upgrade breaking cloud backups #244

Open
wants to merge 1 commit into
base: 3_5
Choose a base branch
from

Conversation

prajwaltr93
Copy link
Contributor

  • set only required http methods

tested on ubuntu 22.04

certain PUT requests were being set as GET, due to multiple options being set at the same time. this caused a signature mis-match error since there was PUT verb in headers which was signed, and a GET request was being made by libcurl, it is now fixed by setting only required http method.

- set only required http methods
@prajwaltr93
Copy link
Contributor Author

also fixes #213 and should also unblock #214

Copy link

@konidev20 konidev20 left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link

@hubandr hubandr left a comment

Choose a reason for hiding this comment

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

compiled on DEBIAN 10 - working with latest libcurl4/curl now - would close #213 (for deb10)

@gerardkok
Copy link

gerardkok commented Sep 17, 2024

I've rebuilt the amanda packages with these changes in for Ubuntu 24.04, and it mostly works (admitted on a small test setup). The only time it does not work, is when it's time to overwrite a tape, the taper debug log then shows the following:

...
Mon Sep 16 16:23:11.658926102 2024: pid 29390: thd-0xf63334000f70: taper: POST https://<redacted>.s3.amazonaws.com?delete failed with 400/InvalidRequest
Mon Sep 16 16:23:11.659002468 2024: pid 29390: thd-0xf63334000f70: taper: Deleting multiple keys not implemented
Mon Sep 16 16:23:11.790775656 2024: pid 29390: thd-0xaac5e5bed500: taper: Building type TAPESTART header of 0-10485760 bytes with name='test-03' disk='' dumplevel=0 and blocksize=10485760
Mon Sep 16 16:23:11.794848252 2024: pid 29390: thd-0xaac5e5bed500: taper: PUT https://<redacted>.s3.amazonaws.com/test%2Fslot-03special-tapestart failed with 403/SignatureDoesNotMatch
Mon Sep 16 16:23:11.794887933 2024: pid 29390: thd-0xaac5e5bed500: taper: Device s3:<redacted>/test/slot-03 error = 'While writing amanda header: The request signature we calculated does not match the signature you provided. Check your key and signing method. (SignatureDoesNotMatch) (HTTP 403)'
...

I think this is when the files are deleted that are related to the tape amanda will write to. The interesting thing is that this deletion seems to work, despite the error. A subsequent amdump also works. amcheck -w shows the same behaviour, so the workaround I'm currently testing is by running amcheck -w before each amdump.

Let me add that the amanda version on Ubuntu 24.04 is 3.5.1 (with some Ubuntu-specific patches).

@hubandr
Copy link

hubandr commented Sep 19, 2024

After applying the above mentioned commit, I observed the exact same behaviour when amdump starts creating new files after deletion. I guessed it has something to do with the wrong HTTP method. After investigating the code, the libcurl docs, I can't remember exactly, please correct me, but as I understood, the http method could not be changed anymore in the request object of libcurl. Unless you do a reset. Therefore, despite the warning in device-src/s3.c:

    /* We don't call curl_easy_reset here, because doing that in curl
     * < 7.16 blanks the default CA certificate path, and there's no way
     * to get it back. */

I applied following addition to the code, build it again, and it works:

@@ -3417,6 +3438,9 @@ s3_reset(S3Handle *hdl)
         /* We don't call curl_easy_reset here, because doing that in curl
          * < 7.16 blanks the default CA certificate path, and there's no way
          * to get it back. */
+
+       curl_easy_reset(hdl->curl);
+
         if (hdl->last_message) {
             g_free(hdl->last_message);
             hdl->last_message = NULL;

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.

4 participants