Skip to content

CA-404460: Expose Stunnel_verify_error for mismatched or corrupted certificate, and expose ssl_verify_error during update syncing #6376

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 21, 2025
Merged

CA-404460: Expose Stunnel_verify_error for mismatched or corrupted certificate, and expose ssl_verify_error during update syncing #6376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bec4ca6
CA-404460: Expose Stunnel_verify_error for mismatched certificate
gangj Mar 20, 2025
659284e
CA-404460: Expose Stunnel_verify_error for corrupted certificate
gangj Mar 20, 2025
95d888e
CA-404460: Fix the exposing of Stunnel_verify_error in check_error
gangj Mar 20, 2025
15df700
CA-404460: expose ssl_verify_error during updates syncing
gangj Mar 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions ocaml/libs/stunnel/stunnel.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,20 +483,30 @@ let check_verify_error line =
let split_1 c s =
match Astring.String.cut ~sep:c s with Some (x, _) -> x | None -> s
in
if Astring.String.is_infix ~affix:"VERIFY ERROR: " line then
match Astring.String.find_sub ~sub:"error=" line with
(* When verified with a mismatched certificate, one line of log from stunnel
* would look like:
SSL_connect: ssl/statem/statem_clnt.c:1889: error:0A000086:SSL routines::certificate verify failed
* in this case, Stunnel_verify_error can be raised with detailed error as
* reason if it can found in the log *)
if Astring.String.is_infix ~affix:"certificate verify failed" line then
match Astring.String.find_sub ~sub:"error:" line with
| Some e ->
raise
(Stunnel_verify_error
(split_1 "," (sub_after (e + String.length "error=") line))
(split_1 "," (sub_after (e + String.length "error:") line))
)
| None ->
raise (Stunnel_verify_error "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the string empty and not providing details? Could at least use __LOC__.

Copy link
Contributor Author

@gangj gangj Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is to expose the error with reason found in the stunnel log. Raise the empty reason when no error can be found in the log.

Copy link
Contributor

@lindig lindig Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

For documentation, could you add in a comment an example for an actual error that we would like to parse? Otherwise it is difficult to assess how the current code works. I would suggest we define a single regular expression (it may contain alternatives (...|...|...) that catches the error and that is easier to maintain than a sequence of matches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment for the mismatched certificate case, but it only looks for log line containing "certificate verify failed", and after "certificate verify failed" is found in the line, it tries to look for detailed error.

Copy link
Contributor

Choose a reason for hiding this comment

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

I still maintain that using Re would be simple; the code below matches the string at the end and extracts the error code.

  4 let rx =
  3   Re.Posix.re {|error:([^:]*).*certificate verify failed|} |> Re.compile
  2 
  1 let str = "SSL_connect: ssl/statem/statem_clnt.c:1889: error:0A000086:SSL routines::certificate verify failed";;
  0                                                                               
  1 let m = Re.exec rx str in Re.Group.get m 1 |> print_endline
  2

else if
Astring.String.is_infix ~affix:"No certificate or private key specified"
line
then
raise (Stunnel_verify_error "The specified certificate is corrupt")
else
()

let check_error s line =
if Astring.String.is_infix ~affix:line s then
if Astring.String.is_infix ~affix:s line then
raise (Stunnel_error s)

let diagnose_failure st_proc =
Expand Down
2 changes: 2 additions & 0 deletions ocaml/xapi/repository.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ let sync ~__context ~self ~token ~token_id ~username ~password =
with
| Api_errors.Server_error (_, _) as e ->
raise e
| Stunnel.Stunnel_verify_error reason ->
raise (Api_errors.Server_error (Api_errors.ssl_verify_error, [reason]))
| e ->
error "Failed to sync with remote YUM repository: %s"
(ExnHelper.string_of_exn e) ;
Expand Down
Loading