diff --git a/lxd/certificates.go b/lxd/certificates.go index 42e8775302c3..466264c34e27 100644 --- a/lxd/certificates.go +++ b/lxd/certificates.go @@ -233,9 +233,20 @@ func clusterMemberJoinTokenValid(s *state.State, r *http.Request, projectName st // Depending on whether it's a local operation or not, expiry will either be a time.Time or a string. if s.ServerName == foundOp.Location { - expiry, _ = expiresAt.(time.Time) + expiry, ok = expiresAt.(time.Time) + if !ok { + return nil, fmt.Errorf("Unexpected expiry type in cluster join token operation: %T (%v)", expiresAt, expiresAt) + } } else { - expiry, _ = time.Parse(time.RFC3339Nano, expiresAt.(string)) + expiryStr, ok := expiresAt.(string) + if !ok { + return nil, fmt.Errorf("Unexpected expiry type in cluster join token operation: %T (%v)", expiresAt, expiresAt) + } + + expiry, err = time.Parse(time.RFC3339Nano, expiryStr) + if err != nil { + return nil, fmt.Errorf("Invalid expiry format in cluster join token operation: %w (%q)", err, expiryStr) + } } // Check if token has expired. @@ -277,53 +288,77 @@ func certificateTokenValid(s *state.State, r *http.Request, addToken *api.Certif } } - if foundOp != nil { - // Token is single-use, so cancel it now. - err = operationCancel(s, r, api.ProjectDefaultName, foundOp) - if err != nil { - return nil, fmt.Errorf("Failed to cancel operation %q: %w", foundOp.ID, err) - } + if foundOp == nil { + // No operation found. + return nil, nil + } - expiresAt, ok := foundOp.Metadata["expiresAt"] - if ok { - expiry, _ := expiresAt.(time.Time) + // Token is single-use, so cancel it now. + err = operationCancel(s, r, api.ProjectDefaultName, foundOp) + if err != nil { + return nil, fmt.Errorf("Failed to cancel operation %q: %w", foundOp.ID, err) + } - // Check if token has expired. - if time.Now().After(expiry) { - return nil, api.StatusErrorf(http.StatusForbidden, "Token has expired") + expiresAt, ok := foundOp.Metadata["expiresAt"] + if ok { + var expiry time.Time + + // Depending on whether it's a local operation or not, expiry will either be a time.Time or a string. + if s.ServerName == foundOp.Location { + expiry, ok = expiresAt.(time.Time) + if !ok { + return nil, fmt.Errorf("Unexpected expiry type in certificate add operation: %T (%v)", expiresAt, expiresAt) + } + } else { + expiryStr, ok := expiresAt.(string) + if !ok { + return nil, fmt.Errorf("Unexpected expiry type in certificate add operation: %T (%v)", expiresAt, expiresAt) + } + + expiry, err = time.Parse(time.RFC3339Nano, expiryStr) + if err != nil { + return nil, fmt.Errorf("Invalid expiry format in certificate add operation: %w (%q)", err, expiryStr) } } - // Certificate add tokens must have a request field in the metadata. - tokenReqAny, ok := foundOp.Metadata["request"] - if !ok { - return nil, fmt.Errorf(`Missing "request" key in certificate add operation data`) + // Check if token has expired. + if time.Now().After(expiry) { + return nil, api.StatusErrorf(http.StatusForbidden, "Token has expired") } + } - tokenReq, ok := tokenReqAny.(api.CertificatesPost) - if !ok { - // If the operation is running on another member, the returned metadata will have been unmarshalled - // into a map[string]any. Rather than wrangling type assertions, just marshal and unmarshal the data into - // the correct type. - buf := bytes.NewBuffer(nil) - err := json.NewEncoder(buf).Encode(tokenReqAny) - if err != nil { - return nil, fmt.Errorf("Bad certificate add operation data: %w", err) - } + // Certificate add tokens must have a request field in the metadata. + tokenReqAny, ok := foundOp.Metadata["request"] + if !ok { + return nil, fmt.Errorf(`Missing "request" key in certificate add operation data`) + } - err = json.NewDecoder(buf).Decode(&tokenReq) - if err != nil { - return nil, fmt.Errorf("Bad certificate add operation data: %w", err) - } + var tokenReq api.CertificatesPost - foundOp.Metadata["request"] = tokenReq + // Depending on whether it's a local operation or not, request field will either be a api.CertificatesPost + // or a JSON string of api.CertificatesPost. + if s.ServerName == foundOp.Location { + tokenReq, ok = tokenReqAny.(api.CertificatesPost) + if !ok { + return nil, fmt.Errorf("Unexpected request type in certificate add operation: %T", tokenReqAny) + } + } else { + // If the operation is running on another member, the returned metadata will have been unmarshalled + // into a map[string]any. Rather than wrangling type assertions, just marshal and unmarshal the + // data into the correct type. + buf := bytes.NewBuffer(nil) + err = json.NewEncoder(buf).Encode(tokenReqAny) + if err != nil { + return nil, fmt.Errorf("Bad certificate add operation data: %w", err) } - return &tokenReq, nil + err = json.NewDecoder(buf).Decode(&tokenReq) + if err != nil { + return nil, fmt.Errorf("Bad certificate add operation data: %w", err) + } } - // No operation found. - return nil, nil + return &tokenReq, nil } // swagger:operation POST /1.0/certificates?public certificates certificates_post_untrusted diff --git a/test/suites/clustering.sh b/test/suites/clustering.sh index cf400c9f3636..379ee664b230 100644 --- a/test/suites/clustering.sh +++ b/test/suites/clustering.sh @@ -3905,6 +3905,39 @@ test_clustering_trust_add() { ns2="${prefix}2" spawn_lxd_and_join_cluster "${ns2}" "${bridge}" "${cert}" 2 1 "${LXD_TWO_DIR}" "${LXD_ONE_DIR}" + # Check using token that is expired + + # Set token expiry to 1 seconds + lxc config set core.remote_token_expiry 1S + + # Get a certificate add token from LXD_ONE. The operation will run on LXD_ONE locally. + lxd_one_token="$(LXD_DIR="${LXD_ONE_DIR}" lxc config trust add --name foo --quiet)" + sleep 2 + + # Expect one running token operation. + operation_uuid="$(LXD_DIR="${LXD_ONE_DIR}" lxc operation list --format csv | grep -F "TOKEN,Executing operation,RUNNING" | cut -d, -f1 )" + LXD_DIR="${LXD_TWO_DIR}" lxc operation list --format csv | grep -qF "${operation_uuid},TOKEN,Executing operation,RUNNING" + is_uuid_v4 "${operation_uuid}" + + # Get the address of LXD_TWO. + lxd_two_address="https://$(LXD_DIR="${LXD_TWO_DIR}" lxc config get core.https_address)" + + # Test adding the remote using the address of LXD_TWO with the token operation running on LXD_ONE. + # LXD_TWO does not have the operation running locally, so it should find the UUID of the operation in the database + # and query LXD_ONE for it. LXD_TWO should cancel the operation by sending a DELETE /1.0/operations/{uuid} to LXD_ONE + # and needs to parse the metadata of the operation into the correct type to complete the trust process. + # The expiry time should be parsed and found to be expired so the add action should fail. + ! lxc remote add lxd_two "${lxd_two_address}" --accept-certificate --token "${lxd_one_token}" || false + + # Expect the operation to be cancelled. + LXD_DIR="${LXD_ONE_DIR}" lxc operation list --format csv | grep -qF "${operation_uuid},TOKEN,Executing operation,CANCELLED" + LXD_DIR="${LXD_TWO_DIR}" lxc operation list --format csv | grep -qF "${operation_uuid},TOKEN,Executing operation,CANCELLED" + + # Set token expiry to 1 hour + lxc config set core.remote_token_expiry 1H + + # Check using token that isn't expired + # Get a certificate add token from LXD_ONE. The operation will run on LXD_ONE locally. lxd_one_token="$(LXD_DIR="${LXD_ONE_DIR}" lxc config trust add --name foo --quiet)" @@ -3929,6 +3962,9 @@ test_clustering_trust_add() { # Clean up lxc remote rm lxd_two + # Unset token expiry + lxc config unset core.remote_token_expiry + LXD_DIR="${LXD_TWO_DIR}" lxd shutdown LXD_DIR="${LXD_ONE_DIR}" lxd shutdown sleep 0.5 @@ -3940,4 +3976,4 @@ test_clustering_trust_add() { kill_lxd "${LXD_ONE_DIR}" kill_lxd "${LXD_TWO_DIR}" -} \ No newline at end of file +} diff --git a/test/suites/remote.sh b/test/suites/remote.sh index e5329c7e1cc4..d20ff0d49c73 100644 --- a/test/suites/remote.sh +++ b/test/suites/remote.sh @@ -111,8 +111,8 @@ test_remote_url_with_token() { lxc config trust rm "$(lxc config trust list -f json | jq -r '.[].fingerprint')" - # Set token expiry to 5 seconds - lxc config set core.remote_token_expiry 5S + # Set token expiry to 1 seconds + lxc config set core.remote_token_expiry 1S # Generate new token token="$(lxc config trust add --name foo | tail -n1)" @@ -130,7 +130,7 @@ test_remote_url_with_token() { token="$(lxc config trust add --name foo | tail -n1)" # This will cause the token to expire - sleep 5 + sleep 2 # Try adding remote. This should fail. ! lxc_remote remote add test "${token}" || false