Skip to content

dird: fix tls psk connection failure with long job names#1204

Merged
pstorz merged 6 commits intobareos:masterfrom
alaaeddineelamri:dev/alaaeddineelamri/master/s5174-fix-tls-psk-crendential-not-found
Jul 21, 2022
Merged

dird: fix tls psk connection failure with long job names#1204
pstorz merged 6 commits intobareos:masterfrom
alaaeddineelamri:dev/alaaeddineelamri/master/s5174-fix-tls-psk-crendential-not-found

Conversation

@alaaeddineelamri
Copy link
Contributor

@alaaeddineelamri alaaeddineelamri commented Jul 13, 2022

Description

When having very long job names exceeding 98 characters, we start seeing a TLS-PSK failure with error 1115. This is due to a mix of psk identity limitations set by OpenSSL and truncation code we have.
This PR fixes the issue.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
  • Commit descriptions are understandable and well formatted
  • [ ] If backport: add original PR number and target branch at top of this file: Backport of PR#000 to bareos-2x
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems
  • git status should not report modifications in the source tree after building and testing
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault
  • Manually tested dir/sd openSSL3x with fd openSSL1x backup restore status
  • Manually tested dir/sd openSSL1x with fd openSSL3x backup restore status
  • Manually tested dir openSSL3x / sd openSSL1x / fd openSSL1x
  • Manually tested dir openSSL1x / sd openSSL3x / fd openSSL1x

- Test job with Name exceeding 104 char length so jobname got truncated
  while adding the 24 char suffix .2022-06-30_12:12:12_12
- This cause tls-psk authentification between fd and sd to failed with
  Fatal error: Connect failure: ERR=error:14094410:SSL routines:ssl3_read_bytes:sslv3 alert handshake failure
  Fatal error: Bad response to Storage command: wanted 2000 OK storage, got 2902 Bad storage
  Fatal error: Director's comm line to SD dropped.

Signed-off-by: Bruno Friedmann <bruno.friedmann@bareos.com>
(cherry picked from commit 2fd376d)
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/s5174-fix-tls-psk-crendential-not-found branch 2 times, most recently from a1e89aa to e9af49b Compare July 13, 2022 10:24
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/s5174-fix-tls-psk-crendential-not-found branch from e9af49b to 902477c Compare July 13, 2022 10:31
Alaa Eddine Elamri added 4 commits July 13, 2022 13:06
When creating a Job name, we do a truncation to add a suffix and
make sure the string does not pass the 128 character limit.
Later, during TLS-PSK authentication process, a prefix `R_JOB^` is
added to the job name to create the identification key, and another 
truncation needs to happen in order to comply with OpenSSL1.1's 
128 character limit for identity.
This second truncation eats the last part of the Jobname containing
our custom suffix, and thus creates problems.
This issue does not show up with OpenSSL3, as the identity limit was
changed to 256 characters.

This commit then anticipates the extra 6 characters needed for the 
"R_JOB^" prefix added in the psk identity string.
@alaaeddineelamri alaaeddineelamri force-pushed the dev/alaaeddineelamri/master/s5174-fix-tls-psk-crendential-not-found branch from 902477c to d86a394 Compare July 13, 2022 11:06
Copy link
Contributor

@bruno-at-bareos bruno-at-bareos left a comment

Choose a reason for hiding this comment

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

Thanks really good job, I like the approach which is also quite easy to backport if needed.
To be merge once green.

@pstorz pstorz requested review from pstorz and removed request for joergsteffens July 21, 2022 10:33
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Good work!

@pstorz pstorz merged commit f3b636f into bareos:master Jul 21, 2022
@pstorz pstorz deleted the dev/alaaeddineelamri/master/s5174-fix-tls-psk-crendential-not-found branch July 21, 2022 12:12
pstorz added a commit that referenced this pull request Jul 25, 2022
…kport1204-fix-tls-psk-crendential-not-found

backport #1204 dird fix tls psk crendential not found
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants