OpenPrinting / cups

OpenPrinting CUPS Sources
https://openprinting.github.io/cups
Apache License 2.0
1.08k stars 193 forks source link

Regression: Locally saved certificates corrupted #724

Closed tillkamppeter closed 1 year ago

tillkamppeter commented 1 year ago

Base-64-encoding, via the httpEncode64_2() function, used to save printer/server certificates in files, got broken in commit a521b235a1ab, ending up with corrupted files not readable when loading them again and so the certificates of network printers do not get trusted when a previous copy of them gets loaded from the file. So the first job prints (no certificate file yet, certificate loaded from printer) and every subsequent job does not print, as the printer's certificate does not match the one loaded from the file.

This commit undoes the changes which got applied to the httpEncode64_2() function to make the certificates correctly encoded again.

To quickly test I have modified the file cups/testcreds.c as follows:

--- cups/testcreds.c    2021-03-15 20:35:42.363816864 +0100
+++ cups/testcreds.c    2023-06-09 21:11:09.520404737 +0200
@@ -82,6 +82,8 @@
       printf("    IsValidName: %d\n", httpCredentialsAreValidForName(hcreds, hostname));
       printf("    String: \"%s\"\n", hinfo);

+      httpSaveCredentials(NULL, hcreds, hostname);
+      
       httpFreeCredentials(hcreds);
     }
     else

which saves the certificate. On the first run (no local copy of the certificate is present) the original certificate from the printer is trusted. From the second run on it is not trusted any more, as it is compared with the corrupted local copy.

After applying the fix, the local copy of the certificate gets correctly created and so on the subsequent runs of the test program the certificate is continued to be trusted.

I also recommend the CI/build test program to be updated to include testing the httpSaveCredentials() function.

@michaelrsweet I could commit this fix right away, but probably you had some intention with the original change which I am undoing here, so I would like you to have a look. I have applied the patch in the CUPS Snap for now and there it fixes the problem.

tillkamppeter commented 1 year ago

Additional remark: The printer with which I ran into the problem is the HP OfficeJet Pro 8730.

AreaZR commented 1 year ago

Base-64-encoding, via the httpEncode64_2() function, used to save printer/server certificates in files, got broken in commit a521b23, ending up with corrupted files not readable when loading them again and so the certificates of network printers do not get trusted when a previous copy of them gets loaded from the file. So the first job prints (no certificate file yet, certificate loaded from printer) and every subsequent job does not print, as the printer's certificate does not match the one loaded from the file.

This commit undoes the changes which got applied to the httpEncode64_2() function to make the certificates correctly encoded again.

To quickly test I have modified the file cups/testcreds.c as follows:

--- cups/testcreds.c  2021-03-15 20:35:42.363816864 +0100
+++ cups/testcreds.c  2023-06-09 21:11:09.520404737 +0200
@@ -82,6 +82,8 @@
       printf("    IsValidName: %d\n", httpCredentialsAreValidForName(hcreds, hostname));
       printf("    String: \"%s\"\n", hinfo);

+      httpSaveCredentials(NULL, hcreds, hostname);
+      
       httpFreeCredentials(hcreds);
     }
     else

which saves the certificate. On the first run (no local copy of the certificate is present) the original certificate from the printer is trusted. From the second run on it is not trusted any more, as it is compared with the corrupted local copy.

After applying the fix, the local copy of the certificate gets correctly created and so on the subsequent runs of the test program the certificate is continued to be trusted.

I also recommend the CI/build test program to be updated to include testing the httpSaveCredentials() function.

@michaelrsweet I could commit this fix right away, but probably you had some intention with the original change which I am undoing here, so I would like you to have a look. I have applied the patch in the CUPS Snap for now and there it fixes the problem.

I made the change based on a faulty assumption the &255 was redundant. You can change this back.

tillkamppeter commented 1 year ago

Fixed with PR #726, the simpler patch there already solves the problem. Thanks @AtariDreams !