Skip to content

xapi: Improve error reporting when pool join fails on TLS verification#7112

Open
LucienLassalle wants to merge 1 commit into
xapi-project:masterfrom
LucienLassalle:lle_improve_error
Open

xapi: Improve error reporting when pool join fails on TLS verification#7112
LucienLassalle wants to merge 1 commit into
xapi-project:masterfrom
LucienLassalle:lle_improve_error

Conversation

@LucienLassalle
Copy link
Copy Markdown

When a host joins a pool (pool.join_force), the process has two phases:

  1. An unverified TLS connection is used to run pre-join checks and exchange host certificates. The joiner imports the pool bundle.
  2. A verified TLS connection (verifyPeer=yes, SNI=pool) is opened using the freshly-generated pool bundle.

Previously, any failure at Phase 2 surfaced as:
INTERNAL_ERROR(Stunnel.Stunnel_verify_error(

This error is opaque and gives no actionable information to the administrator. The idea is to improve error handling in order to obtain something more precise.

Comment thread ocaml/xapi/xapi_pool.ml Outdated
Server_error
(pool_joining_pool_bundle_empty_after_import, [bundle_path])
)
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this check be done inside the import_joining_pool_certs?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yup, like this ?

Comment thread ocaml/xapi-consts/api_errors.ml
D.error
"import_joining_pool_certs: pool bundle '%s' is empty or missing after \
certificate import. The bundle generation script \
(/opt/xensource/bin/update-ca-bundle.sh) likely failed silently."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does the script leave some errors in daemon.log to be able to investigate? or is the failure actually silent?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

#!/bin/bash
#
# Copyright (c) Citrix Systems 2008. All rights reserved.
#

set -e

regen_bundle () {
  CERTS_DIR="$1"
  BUNDLE="$2"

  mkdir -p "$CERTS_DIR"
  CERTS=$(find "$CERTS_DIR" -not -name '*.new.pem' -name '*.pem')
  NEW_CERTS=$(find "$CERTS_DIR" -name '*.new.pem')

  rm -f "$BUNDLE.tmp"
  touch "$BUNDLE.tmp"
  for NEW_CERT in $NEW_CERTS; do
    # If cat new cert command fails, do not error and exit, just skip it
    if cat "$NEW_CERT" >> "$BUNDLE.tmp"; then
      echo "" >> "$BUNDLE.tmp"
    fi
  done
  for CERT in $CERTS; do
    cat "$CERT" >> "$BUNDLE.tmp"
    echo ""     >> "$BUNDLE.tmp"
  done
  mv "$BUNDLE.tmp" "$BUNDLE"
}

regen_bundle "/etc/stunnel/certs"      "/etc/stunnel/xapi-stunnel-ca-bundle.pem"
regen_bundle "/etc/stunnel/certs-pool" "/etc/stunnel/xapi-pool-ca-bundle.pem"

This script is silent...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks like a fairly simple script, I wonder why would cat new cert possibly fail???

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I don't know..

 verification

When a host joins a pool (pool.join_force), the process has two phases:
1. An unverified TLS connection is used to run pre-join
 checks and exchange host certificates.	The joiner imports the pool bundle.
2. A verified TLS connection (verifyPeer=yes, SNI=pool) is opened using
 the freshly-generated pool bundle.

Previously, any failure at Phase 2 surfaced as:
	INTERNAL_ERROR(Stunnel.Stunnel_verify_error(

This error is opaque and gives no actionable information to the administrator.
The idea is to improve error handling in order to obtain something more precise.

Signed-off-by: Lucas RAVAGNIER <ravagnierlucas@gmail.com>
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.

3 participants