From b413bf0bf351bb70e4b101df3b12e03791531d4b Mon Sep 17 00:00:00 2001 From: FacuM Date: Fri, 30 Oct 2026 17:53:49 -0300 Subject: [PATCH 1/9] M5: threat-model review, service-hosted WebDAV attach, double-mount diagnosis - docs/design/05-threat-model.md section 12: the M5 implementation review. Per surface (pipes, loopback WebDAV, Dokany requestor check, update signatures, DPAPI, Event Log, service inputs) the claimed property, the code that carries it (file:line) and a verdict. Claims with no code: the host compiles in no update roots, no crash-dump code, no Event 3100, no FUS/RDP session list, no --elevated-view. Proposed TM-R40 to TM-R45. - Global WebDAV attach (02 7.7): the service hosts the export and maps nothing (no_map); each user's agent maps the UNC it gets from the new service method mount.attach_info, at logon and through mount.attach. Only members of service_access get the UNC. The mechanism goes through svc_gw_decide (auto; system/link verdicts are refused, not served as attach). backend = webdav service profiles load again. - sd_webdav service mode: the per-connection owner check yields to the per-request sd_vfs check, and the token's enabled groups reach sd_vfs so group entries of service_access match. Anonymous and Guest callers get 403; LOCAL/NETWORK SERVICE match only by an explicit SID. - The agent's own mount-start cleanup keeps the attached service port (sd_webdav_preserve_port); the explicit start/stop cleanups still remove it. - The Part 10 heap corruption on a second mount was a test double free, fixed by json_drop in e2c3df5; recorded and verified. - docs/dev/lab-results.md appendix: the procedure for the live global Dokany mount on the Win10 VM (not run: the VM was in use). Tests: unit.svc (attach point reported; attach refused outside service_access), unit.webdav (service server admits a member and refuses a non-member, real SSPI). Local gate: five presets pass; repro.x64-modern passes. --- apps/shelldrive-host/CMakeLists.txt | 4 +- apps/shelldrive-host/agent.c | 139 ++++++++++++++++- apps/shelldrive-host/service.c | 112 +++++++++++++- apps/shelldrive-host/svc_access.c | 3 - apps/shelldrive-host/svc_access.h | 5 +- docs/design/05-threat-model.md | 157 +++++++++++++++++++ docs/dev/lab-results.md | 52 +++++++ docs/dev/m5-notes.md | 158 +++++++++++++++++++- src/sd_mount/include/sd_mount/sd_mount.h | 10 ++ src/sd_mount/mount.c | 7 + src/sd_mount/mount_real.c | 2 + src/sd_webdav/include/sd_webdav/sd_webdav.h | 16 ++ src/sd_webdav/wd_auth.c | 67 ++++++++- src/sd_webdav/wd_client.c | 64 +++++++- src/sd_webdav/wd_server.c | 75 +++++++++- src/sd_webdav/wd_server.h | 11 +- tests/apps/CMakeLists.txt | 5 +- tests/unit/sd_mount/fake_backend.c | 20 ++- tests/unit/sd_svc/main.c | 4 + tests/unit/sd_svc/test_access.c | 5 +- tests/unit/sd_svc/test_service.c | 94 +++++++++++- tests/unit/sd_webdav/main.c | 4 + tests/unit/sd_webdav/test_server.c | 92 ++++++++++++ 23 files changed, 1067 insertions(+), 39 deletions(-) diff --git a/apps/shelldrive-host/CMakeLists.txt b/apps/shelldrive-host/CMakeLists.txt index a17ab39..4297e27 100644 --- a/apps/shelldrive-host/CMakeLists.txt +++ b/apps/shelldrive-host/CMakeLists.txt @@ -83,8 +83,10 @@ sd_add_app(shelldrive-host ${CMAKE_CURRENT_SOURCE_DIR}/svc_scm.c ${CMAKE_CURRENT_SOURCE_DIR}/svc_scm.h ${CMAKE_CURRENT_SOURCE_DIR}/svc_repair.c ${CMAKE_CURRENT_SOURCE_DIR}/svc_repair.h ${CMAKE_CURRENT_SOURCE_DIR}/svc_status.c ${CMAKE_CURRENT_SOURCE_DIR}/svc_status.h + ${CMAKE_CURRENT_SOURCE_DIR}/svc_gw.c ${CMAKE_CURRENT_SOURCE_DIR}/svc_gw.h + ${CMAKE_CURRENT_SOURCE_DIR}/svc_gw_attach.c ${CMAKE_CURRENT_SOURCE_DIR}/svc_gw_attach.h "${SD_SVC_EVENTS_DIR}/svc_events.rc" DEPS sd_mount ${_backend_lib} sd_ipc sd_state sd_secret sd_ssh sd_config sd_update sd_policy sd_platform sd_base - sd_l10n sd_l10n_catalog_svc ole32 shell32 user32 advapi32) + sd_l10n sd_l10n_catalog_svc ole32 shell32 user32 advapi32 mpr) target_include_directories(shelldrive-host PRIVATE "${SD_SVC_EVENTS_DIR}") add_dependencies(shelldrive-host sd_svc_events) diff --git a/apps/shelldrive-host/agent.c b/apps/shelldrive-host/agent.c index 38df1c8..07fd930 100644 --- a/apps/shelldrive-host/agent.c +++ b/apps/shelldrive-host/agent.c @@ -45,9 +45,11 @@ #include "sd_secret/sd_secret.h" #include "sd_ssh/sd_known_hosts.h" #include "sd_platform/sd_log.h" +#include "sd_platform/sd_platform.h" #include "sd_platform/sd_thread.h" #include "sd_platform/sd_time.h" +#include "svc_gw_attach.h" #include "log_ring.h" #include "agent_events.h" #include "state_producer.h" @@ -153,7 +155,8 @@ typedef struct host_mount { /* Feature names of the hello reply (03 §7, 2.8): only what this build implements. */ static const char *const k_agent_features[] = { "status", "config.reload", "log.tail", "selftest", "prompt.list", "prompt.queued", - "refresh", "cache.clear", "profile.test", "mount.attach_info", "status.stats", "status.proxy_legs", + "refresh", "cache.clear", "profile.test", "mount.attach_info", "mount.attach", "status.stats", + "status.proxy_legs", "config.changed", "agent.stopping", "update", "update.check", "update.apply", "diag.collect", "events.recent", NULL }; @@ -2308,6 +2311,8 @@ static void start_autostart(const char *id) sd_free(prof); } +static void attach_global_webdav(void); /* mount.attach: the service-hosted global WebDAV drives (02 §7.7) */ + static void work_autostart(void *arg) { autostart_list *l; @@ -2321,6 +2326,7 @@ static void work_autostart(void *arg) for (i = 0; i < l->n; i++) start_autostart(l->id[i]); sd_free(l); + attach_global_webdav(); /* 02 §7.7: the global WebDAV drives of this logon session */ } static sd_err handle_profile_test(sd_ipc_client *client, const sd_json *params, sd_json_writer *res, sd_ipc_error *err) @@ -2687,6 +2693,136 @@ static sd_err handle_attach_info(const sd_json *params, sd_json_writer *res, sd_ return SD_OK; } +/* mount.attach {mount_id | target | profile} (02 §7.7 attach, 03 §1.3, D11): map the global WebDAV drive the + service hosts into this logon session. The service answers with the export only for a caller in that profile's + service_access (mount.attach_info), and the reply carries the UNC with the path token, never a credential. The + mapping is not persistent (no CONNECT_UPDATE_PROFILE); the agent's start, stop and crash paths cancel it with + every other \\localhost@port mapping of this session (TM-R17). */ +/* Maps the export of one mount.attach_info reply in this logon session. SD_E_UNSUPPORTED: nothing to attach (a + Dokany global mount, or an XP session with no http form). *werr carries the WNet error on SD_E_FAILURE. */ +static sd_err attach_map_reply(const sd_json *info, DWORD *werr) +{ + const char *unc = sd_json_get_str(info, "unc"); + const char *xp = sd_json_get_str(info, "xp_url"); + const char *tgt = sd_json_get_str(info, "target"); + wchar_t remote[104]; /* the size of sd_mount_attach.webdav_remote */ + int xp_form = !sd_os_at_least(6, 0, 0) ? 1 : 0; /* XP/2003: the http form and the Web Client Network provider */ + int64_t port = 0; + sd_err e; + if (unc == NULL || unc[0] == 0) + return SD_E_UNSUPPORTED; + if (tgt == NULL || tgt[0] < 'A' || tgt[0] > 'Z' || tgt[1] != ':') + return SD_E_INVALID_PARAM; + if (xp_form && (xp == NULL || xp[0] == 0)) + return SD_E_UNSUPPORTED; + if (!wide_from_utf8(xp_form ? xp : unc, remote, SD_COUNTOF(remote))) + return SD_E_INVALID_PARAM; + e = svc_gw_attach_map(&svc_gw_attach_real_ops, (wchar_t)tgt[0], remote, xp_form, werr); + /* the service's port is live, not stale: a later per-user WebDAV mount of this agent must not cancel this + mapping when it starts its own server (TM-R17 d cleanup); the agent's stop and start cleanups still do */ + if ((e == SD_OK || e == SD_E_EXISTS) && sd_json_get_int64(info, "port", &port) && port > 0 && port <= 65535 && + host_backend()->webdav_preserve_port) + host_backend()->webdav_preserve_port((uint16_t)port, TRUE); + return e; +} + +static sd_err handle_attach(const sd_json *params, sd_json_writer *res, sd_ipc_error *err) +{ + sd_ipc_hello hello; + sd_ipc_conn *c = NULL; + sd_json *r = NULL; + sd_ipc_error serr; + sd_err e; + DWORD werr = 0; + const char *tgt; + memset(&hello, 0, sizeof(hello)); + hello.client_kind = "agent"; + hello.client_version = SD_VERSION_STRING; + hello.lang = "en"; + memset(&serr, 0, sizeof(serr)); + e = sd_ipc_connect(SD_IPC_SERVICE, &hello, 0, 5000, &c); + if (e != SD_OK) + return fail(err, e, "service_unavailable", "the ShellDrive service is not reachable"); + e = sd_ipc_call(c, "mount.attach_info", params, 5000, &r, &serr); + sd_ipc_close(c); + if (e != SD_OK) { + sd_json_free(r); + return fail(err, e, serr.code[0] ? serr.code : "service_unavailable", + serr.code[0] ? serr.detail : "the service did not answer"); + } + e = attach_map_reply(r, &werr); + if (e == SD_E_UNSUPPORTED) { + sd_json_free(r); + return fail(err, e, "not_attachable", "this global mount has nothing to attach in this session"); + } + if (e != SD_OK) { + char detail[96]; + _snprintf_s(detail, sizeof(detail), _TRUNCATE, "the mapping was refused (win32 %lu)", (unsigned long)werr); + sd_json_free(r); + return fail(err, e, e == SD_E_EXISTS ? "target_in_use" : "attach_failed", detail); + } + tgt = sd_json_get_str(r, "target"); + sd_jw_obj_begin(res); + sd_jw_key(res, "mount_id"); sd_jw_cstr(res, sd_json_get_str(r, "mount_id")); + sd_jw_key(res, "target"); sd_jw_cstr(res, tgt); + sd_jw_key(res, "mapped"); sd_jw_bool(res, 1); + sd_jw_obj_end(res); + sd_json_free(r); + return SD_OK; +} + +/* 02 §7.7 attach: at logon, map every global WebDAV drive the service hosts and this user may use. The service's + list already hides the mounts the caller is not in service_access for, and mount.attach_info is the per-mount + check. Best effort: no service, no such drive, or a refused mapping only logs; nothing here fails the agent. */ +static void attach_global_webdav(void) +{ + sd_ipc_hello hello; + sd_ipc_conn *c = NULL; + sd_json *list = NULL, *info = NULL; + sd_ipc_error err; + const sd_json *mounts; + size_t i, n; + memset(&hello, 0, sizeof(hello)); + hello.client_kind = "agent"; + hello.client_version = SD_VERSION_STRING; + hello.lang = "en"; + if (sd_ipc_connect(SD_IPC_SERVICE, &hello, 0, 3000, &c) != SD_OK) + return; /* no service: Dokany drives, if any, are the service's */ + memset(&err, 0, sizeof(err)); + if (sd_ipc_call(c, "list", NULL, 5000, &list, &err) != SD_OK || list == NULL) { + sd_json_free(list); + sd_ipc_close(c); + return; + } + mounts = sd_json_get(list, "mounts"); + n = mounts ? sd_json_count(mounts) : 0; + for (i = 0; i < n; i++) { + const sd_json *m = sd_json_at(mounts, i); + const char *id = sd_json_get_str(m, "mount_id"); + char params[64]; + DWORD werr = 0; + sd_err e; + if (id == NULL || id[0] == 0 || strchr(id, '"') != NULL) + continue; + _snprintf_s(params, sizeof(params), _TRUNCATE, "{\"mount_id\":\"%s\"}", id); + memset(&err, 0, sizeof(err)); + info = NULL; + if (sd_ipc_call_raw(c, "mount.attach_info", params, strlen(params), 5000, &info, &err) != SD_OK) { + continue; + } + e = attach_map_reply(info, &werr); + if (e == SD_OK) + SD_LOGI("agent", "attached global WebDAV mount %s", id); + else if (e != SD_E_UNSUPPORTED) + SD_LOGW("agent", "global WebDAV mount %s: the mapping was refused (%s, win32 %u)", id, sd_err_name(e), + (unsigned)werr); + sd_json_free(info); + info = NULL; + } + sd_json_free(list); + sd_ipc_close(c); +} + /* selftest {}: the in-process checks of `shelldrive-host --selftest` plus the live pipe (03 §6.1). */ static sd_err handle_selftest(sd_json_writer *res) { @@ -3473,6 +3609,7 @@ static sd_err ipc_handler(void *ctx, sd_ipc_client *client, const char *method, if (strcmp(method, "refresh") == 0) return handle_refresh(params, res, err); if (strcmp(method, "cache.clear") == 0) return handle_cache_clear(params, res, err); if (strcmp(method, "mount.attach_info") == 0) return handle_attach_info(params, res, err); + if (strcmp(method, "mount.attach") == 0) return handle_attach(params, res, err); if (strcmp(method, "log.tail") == 0) return handle_log_tail(params, res, err); if (strcmp(method, "config.reload") == 0) return handle_config_reload(params, res, err); if (strcmp(method, "prompt.answer") == 0) return handle_prompt_answer(client, params, res, err); diff --git a/apps/shelldrive-host/service.c b/apps/shelldrive-host/service.c index f905dd5..b9f670c 100644 --- a/apps/shelldrive-host/service.c +++ b/apps/shelldrive-host/service.c @@ -25,6 +25,7 @@ #include "service.h" #include "svc_access.h" #include "svc_events.h" +#include "svc_gw.h" #include "svc_repair.h" #include "svc_scm.h" #include "svc_status.h" @@ -102,6 +103,9 @@ typedef struct svc_mount { const char *refuse_code; /* IPC code of the refused prompt (prompt_unavailable, host_key_unknown) */ char err_code[64]; char err_detail[160]; + /* 02 §7.7 attach: the hosted WebDAV export, filled on MOUNTED, empty for a Dokany mount */ + uint16_t wd_port; + wchar_t wd_remote[104], wd_remote_xp[104]; } svc_mount; typedef struct svc_job { @@ -837,11 +841,25 @@ static void svc_on_state(void *ctx, sd_mount *m, sd_mount_state st, sd_err why) wchar_t letter = 0; svc_mount *r; const char *code = NULL; + sd_mount_attach at; + int have_at = 0; (void)ctx; + /* 02 §7.7: the hosted export exists as soon as the mount is up; read it before taking g.lock (the mount's + own lock must never be held while g.lock is asked for the other way round). */ + memset(&at, 0, sizeof(at)); + if (st == SD_MOUNT_MOUNTED && m != NULL) { + sd_mount_get_attach(m, &at); + have_at = at.webdav_remote[0] != 0; + } sd_mutex_lock(&g.lock); r = find_by_m_locked(m); if (r) { r->st = st; + if (have_at) { + r->wd_port = at.webdav_port; + wcscpy_s(r->wd_remote, SD_COUNTOF(r->wd_remote), at.webdav_remote); + wcscpy_s(r->wd_remote_xp, SD_COUNTOF(r->wd_remote_xp), at.webdav_remote_xp); + } strcpy_s(id, sizeof(id), r->id); letter = r->letter; if (r->prof != NULL) @@ -1936,6 +1954,45 @@ static void svc_hk_release(svc_hk *hk) /* ---- control-thread jobs ---- */ +static svc_gw_client svc_gw_client_of(sd_webdav_client_state s) +{ + switch (s) { + case SD_WD_CLIENT_RUNNING: return SVC_GW_CLIENT_RUNNING; + case SD_WD_CLIENT_STOPPED: return SVC_GW_CLIENT_STOPPED; + case SD_WD_CLIENT_DISABLED: return SVC_GW_CLIENT_DISABLED; + case SD_WD_CLIENT_MISSING: return SVC_GW_CLIENT_MISSING; + default: return SVC_GW_CLIENT_UNKNOWN; + } +} + +/* 02 §7.7, the M5 spike: the mechanism of a global WebDAV mount. Only attach is built; it is what auto resolves to + while the compiled table has no GO cell. A system or link verdict (a lab-flipped cell) is refused here until that + mechanism exists, never served as attach. The mode is always auto: the service does not read settings.ini + (webdav_global_mode needs the checked read of TM-R27 first). NULL = attach, else an English reason (log, usage). */ +static const char *svc_webdav_mechanism(void) +{ + svc_gw_input in; + svc_gw_verdict v; + sd_webdav_client_info ci; + const sd_os_version *ov = sd_os_version_get(); + memset(&in, 0, sizeof(in)); + memset(&ci, 0, sizeof(ci)); + in.family = ov ? svc_gw_family_of(ov->major, ov->minor) : SVC_GW_OS_UNKNOWN; + in.mode = SVC_GW_MODE_AUTO; + in.service_running = 1; + in.client = g.be->webdav_query_client ? svc_gw_client_of(g.be->webdav_query_client(&ci)) : SVC_GW_CLIENT_MISSING; + in.token_only = 0; + svc_gw_decide(&in, &v); + if (v.mech == SVC_GW_ATTACH) + return NULL; + if (v.mech == SVC_GW_REFUSE) { + SD_LOGW("svc", "global WebDAV refused: %s", svc_gw_reason_name(v.reason)); + return svc_gw_reason_name(v.reason); + } + SD_LOGW("svc", "global WebDAV mechanism %s is not built (attach only)", svc_gw_mech_name(v.mech)); + return "this global WebDAV mechanism is not in this build (attach only, 02 7.7)"; +} + static void job_start(void *arg) { svc_job *j = (svc_job *)arg; @@ -2007,7 +2064,11 @@ static void job_start(void *arg) pr.volume_label = labelw; wide_from_utf8(p->name, namew, SD_COUNTOF(namew)); pr.profile_name = namew; - pr.backend = SD_MOUNT_BACKEND_DOKANY; /* global drives are Dokany in this build (1.3, D11) */ + /* D11, 02 §7.7: Dokany is the global path; a profile forced to WebDAV is hosted by the service and + attached per user at logon (attach), never mapped by LocalSystem (mechanism A is a no-go). */ + pr.backend = p->backend == SD_PROFILE_BACKEND_WEBDAV ? SD_MOUNT_BACKEND_WEBDAV : SD_MOUNT_BACKEND_DOKANY; + if (why == NULL && pr.backend == SD_MOUNT_BACKEND_WEBDAV) + why = svc_webdav_mechanism(); pr.global_mount = 1; pr.legacy_policy = p->legacy_algorithms; pr.refuse_terrapin_exposed = p->refuse_terrapin_exposed; @@ -2383,6 +2444,51 @@ static sd_err handle_list(sd_ipc_client *c, sd_json_writer *res) return SD_OK; } +/* mount.attach_info {mount_id | target | profile} (02 §7.7, 03 §2.4): the export a member's agent maps in its own + logon session. Only a caller in that profile's service_access gets it (D35); the reply carries the UNC with the + path token, never a credential. A Dokany global mount has nothing to attach. Read-only, no administrator needed. */ +static sd_err handle_attach_info(sd_ipc_client *c, const sd_json *params, sd_json_writer *res, sd_ipc_error *err) +{ + svc_mount *r; + svc_member mb; + svc_rights rt; + char unc[256] = "", xp[256] = "", tgt[4], id[16]; + int webdav; + sd_mutex_lock(&g.lock); + r = find_mount_locked(params); + if (r == NULL) { + sd_mutex_unlock(&g.lock); + return fail(err, SD_E_NOT_FOUND, "mount_not_found", "no matching mount"); + } + member_of(c, &r->prof->acl, &mb); + svc_decide(&mb, &rt); + if (!rt.use) { + sd_mutex_unlock(&g.lock); + return fail(err, SD_E_ACCESS_DENIED, "access_denied", "the caller is not in service_access for this drive"); + } + webdav = r->prof->p.backend == SD_PROFILE_BACKEND_WEBDAV; + if (webdav) { + utf8_from_wide(r->wd_remote, unc, sizeof(unc)); + utf8_from_wide(r->wd_remote_xp, xp, sizeof(xp)); + } + strcpy_s(id, sizeof(id), r->id); + target_of(r->letter, tgt); + sd_jw_key(res, "mount_id"); sd_jw_cstr(res, id); + sd_jw_key(res, "target"); sd_jw_cstr(res, tgt); + sd_jw_key(res, "label"); sd_jw_cstr(res, r->prof->p.label[0] ? r->prof->p.label : r->prof->p.name); + sd_jw_key(res, "backend"); + sd_jw_cstr(res, sd_mount_backend_kind_name(webdav ? SD_MOUNT_BACKEND_WEBDAV : SD_MOUNT_BACKEND_DOKANY)); + sd_jw_key(res, "state"); sd_jw_cstr(res, sd_mount_state_name(r->st)); + sd_jw_key(res, "attach"); sd_jw_bool(res, unc[0] != 0); + if (unc[0]) { + sd_jw_key(res, "unc"); sd_jw_cstr(res, unc); + sd_jw_key(res, "xp_url"); sd_jw_cstr(res, xp); + sd_jw_key(res, "port"); sd_jw_int(res, r->wd_port); + } + sd_mutex_unlock(&g.lock); + return SD_OK; +} + static sd_err handle_status(sd_ipc_client *c, const sd_json *params, sd_json_writer *res, sd_ipc_error *err) { svc_mount *r; @@ -3097,6 +3203,7 @@ static sd_err svc_ipc_handler(void *ctx, sd_ipc_client *client, const char *meth if (strcmp(method, "status") == 0) return handle_status(client, params, res, err); if (strcmp(method, "mount") == 0) return handle_mount(client, params, res, err); if (strcmp(method, "unmount") == 0) return handle_unmount(client, params, res, err); + if (strcmp(method, "mount.attach_info") == 0) return handle_attach_info(client, params, res, err); if (strcmp(method, "selftest") == 0) return handle_selftest(res); if (strcmp(method, "service.status") == 0) return handle_service_status(client, res, err); if (strcmp(method, "service.repair_data") == 0) return handle_service_repair(client, res, err); @@ -3171,7 +3278,8 @@ static void svc_on_subscribe(void *ctx, sd_ipc_client *client, const char *topic static const char *const k_service_features[] = { "list", "status", "mount", "unmount", "selftest", "secret.set", "secret.delete", "prompt.list", "prompt.answer", "config.reload", "prompt.routing", "hostkey.add", "hostkey.announced.list", - "hostkey.announced.accept", "hostkey.announced.reject", "service.status", "service.repair_data", NULL + "hostkey.announced.accept", "hostkey.announced.reject", "service.status", "service.repair_data", + "mount.attach_info", NULL }; /* ---- the core ---- */ diff --git a/apps/shelldrive-host/svc_access.c b/apps/shelldrive-host/svc_access.c index c8f912b..f1bd9cd 100644 --- a/apps/shelldrive-host/svc_access.c +++ b/apps/shelldrive-host/svc_access.c @@ -205,9 +205,6 @@ sd_err svc_validate_profile(const sd_profile *p, const char **bad_key, const cha } else if (p->auth_methods & SD_PROFILE_AUTH_KBDINT) { k = "auth.methods"; w = "keyboard-interactive needs a user at the console (2.6)"; - } else if (p->backend == SD_PROFILE_BACKEND_WEBDAV) { - k = "mount.backend"; - w = "global WebDAV drives are not in this build (D11)"; } if (k != NULL) { if (bad_key) *bad_key = k; diff --git a/apps/shelldrive-host/svc_access.h b/apps/shelldrive-host/svc_access.h index ccd0d87..c5a6810 100644 --- a/apps/shelldrive-host/svc_access.h +++ b/apps/shelldrive-host/svc_access.h @@ -57,8 +57,9 @@ sd_err svc_acl_build(const sd_profile *p, svc_resolve_fn resolve, void *ctx, svc /* Machine profile checks at load and before every mount (2.6 rule 1, D17, 03 §3.4.1). Returns SD_OK, or SD_E_INVALID_PARAM with *bad_key naming the offending key and *why a short English reason (for logs). Refused: scope other than service, - no user, agent-based authentication (auth.agent = pageant or openssh, D17), kbdint (2.6), WebDAV backend (the global - WebDAV spike is not in this build, D11). Jump hosts and proxies are checked per hop when the chain is built + no user, agent-based authentication (auth.agent = pageant or openssh, D17), kbdint (2.6). A WebDAV backend is + accepted: 02 §7.7 hosts the export in the service and attaches it per user at logon (the mechanism is chosen at + mount time, never mapped by LocalSystem). Jump hosts and proxies are checked per hop when the chain is built (svc_chain_hop_check, svc_proxy_leg_check). */ sd_err svc_validate_profile(const sd_profile *p, const char **bad_key, const char **why); diff --git a/docs/design/05-threat-model.md b/docs/design/05-threat-model.md index 201d657..93fd7ea 100644 --- a/docs/design/05-threat-model.md +++ b/docs/design/05-threat-model.md @@ -947,8 +947,165 @@ All are accepted (D39); "adjusted" names the decision that changed the recommend | TM-R38 | 04 §10, §12.3 | with D38 release gates | | TM-R39 | 04 §11.2, §12.2, §12.3 | | +## 12. M5 implementation review (2026-10-30) + +Status: review, not design. The header of this document asks for a second look at the M5 threat-model review +(00 §6) together with the external crypto review; this section is that review of the **implemented** code. It +checks, per surface, the property sections 3-5 claim, the code that is supposed to carry it, and whether it does. +It adds no normative requirement: every gap below is either an open item already recorded in `docs/dev/m5-notes.md` +or a proposed `TM-R40`-`TM-R45` (section 12.9), and a `TM-R` binds only once the owning document accepts it. + +Scope of this pass: what M5 builds and what the next milestone will lean on — the two pipes, the loopback WebDAV +server, the Dokany requestor check, the global-drive access lists, the update signature path, the secret stores and +the Event Log. It is a code review on one machine (Windows 11 10.0.26100, x64-modern and x64-xp builds); nothing +here replaces the lab criteria of section 7. The one thing that did *not* happen is a run with a real elevated +administrator token or a second logon session, so every verdict below is "the code does this", never "the lab saw it". + +Verdicts: **carried** (code does what the design says, in the path a release build takes), **partial** (code exists +but a stated part is missing or only reachable in tests), **absent** (no code). + +### 12.1 Named pipes (TB-07; T-44, T-45, T-46) + +| Property claimed (03 §2.2) | Code | Verdict | +|---|---|---| +| The service pipe is owned by SYSTEM; interactive users get read/write; `NU` is denied | `src/sd_ipc/ipc_server.c:1119-1127` (SDDL `O:SYD:P(D;;GA;;;NU)(A;;GA;;;SY)(A;;GA;;;BA)(A;;GRGW;;;IU)`) | carried | +| The first instance cannot be squatted, and remote clients cannot connect | `ipc_server.c:128` (`FILE_FLAG_FIRST_PIPE_INSTANCE`), `:131` (`PIPE_REJECT_REMOTE_CLIENTS`, Vista+) | carried | +| The agent pipe accepts only its own SID | `ipc_server.c:430` | carried | +| The client token is read by impersonating the client, then the thread reverts; a failed revert ends the process | `ipc_server.c:445-462` (`classify_at_connect`) | carried | +| Below-medium integrity and AppContainer clients are refused (T-45) | `ipc_server.c:422-441` (`token_checks`) | carried | +| Per-SID quota 4, 32 instances, two reserved for elevated administrators; a non-admin is refused at connect once 30 exist | `apps/shelldrive-host/service.c:3351-3359`, `ipc_server.c:238-251` | carried | +| A non-admin that does not say hello in time is closed | `ipc_server.c:1064-1066`, `service.c:3358` (5 s, this build's value) | carried | +| Request cap 64 KiB for non-admins, 1 MiB for admins; 10 requests/s for non-admins | `service.c:3353-3356`, `ipc_server.c:663-671` | carried | +| Mutating methods need an elevated administrator | `service.c:724-728` (`need_admin`), called by every mutating handler | carried | + +Residual unchanged: the DACL admits every interactive user to a LocalSystem JSON parser (T-46, residual M). The +review finds the TM-R25 mitigations implemented as written; it does not lower the residual. + +### 12.2 Loopback WebDAV server (TB-05; T-31 to T-39) + +| Property claimed (02 §7.3-§7.4) | Code | Verdict | +|---|---|---| +| Loopback only, exclusive bind | `src/sd_webdav/wd_server.c:940-946` (a refused `SO_EXCLUSIVEADDRUSE` aborts the start) | carried | +| Unknown path prefixes answer 404 **before** any authentication challenge | `wd_server.c:565-578` | carried | +| `Host` is checked against the server's port; anything else is 400 | `wd_server.c:543` | carried | +| Browser-origin requests (`Origin`, `Sec-Fetch-*`) are 403 before the token check | `wd_server.c:551-554` | carried | +| SSPI Negotiate/NTLM; `secur32` is never a static import (XP floor) | `wd_server.h:52-57`, `wd_auth.c` (interface resolved at run time) | carried | +| The authenticated user must be the mount owner (per-user), or a member of `service_access` (service) | `wd_server.c` `authenticate()` and `handle()` | **partial at review, fixed in the same round** — the review found one owner SID per server and no token groups passed to `sd_vfs`, so a service server could not express `service_access` (users **and groups**). `m5-notes.md` Part 14 adds a service mode: the owner check yields to `sd_vfs`, the token's enabled groups are passed, anonymous/Guest are refused and LOCAL/NETWORK SERVICE match only by an explicit SID. Unit-tested with real SSPI; the two-user lab test (G2) is still open. | +| Body, `If:`, `Destination`, `Lock-Token` and XML are parsed only after authentication | `wd_server.c:591-605`: `authenticate()` runs before `wd_dispatch()` | carried | +| Token-only is never used for a global mount (TM-R19) | `sd_webdav_token_only_decide` (global → `SD_WD_TOK_FORBIDDEN_GLOBAL`), `wd_tokenonly.c` | carried | +| Crash and start-up cleanup of this session's WebDAV mappings (TM-R17 b/d) | `src/sd_webdav/wd_client.c:259` (`sd_webdav_emergency_unmap`), `:337` (`sd_webdav_cleanup_mappings`), `:484` (start, keeps a preserved attach port); the agent's filter calls the emergency unmap at `apps/shelldrive-host/agent.c:3636-3647` | carried for `\\localhost@PORT` mappings; TM-R17(e) (elevated view) has no code (12.8) | + +### 12.3 Dokany requestor check (TB-06; T-41, T-43) + +| Property claimed (02 §6.7) | Code | Verdict | +|---|---|---| +| A global mount decides from the requestor's token user and enabled, non-deny-only groups; a request with no token is refused | `src/sd_dokan/dokan_mount.c:99-155` (`requestor_identity`), `:483-489` (`cb_create`, global branch) | carried | +| The identity cache is keyed by `TOKEN_STATISTICS` (TokenId, ModifiedId) and the user SID, TTL 60 s | `dokan_mount.c:140-151`, `src/sd_dokan/dokan_cache.h:17` | carried | +| A per-user mount admits its owner, and SYSTEM only with `allow_system` | `dokan_mount.c:60-78` (`requestor_allowed`) | carried | +| Read needs membership in `service_access`; write additionally needs `service_access_write` or an enabled Administrators group | `src/sd_vfs/vfs_access.c:145-167` | carried | +| An empty `service_access` list refuses the mount (D35) | `vfs_access.c:95-97` | carried | +| Mutating operations on a global mount raise Event 1200 with the caller's SID | `vfs_access.c:216-273` (coalesced 60 s per SID/op/path), `service.c:1078-1116` | carried | +| The volume security descriptor encodes the same lists | `service.c:2107-2111`, `svc_access.c` (`svc_volume_sddl`), `dokan_mount.c:462-464` (refuses a global mount without one) | carried | + +Residual unchanged: a real global mount opened by a second session was never observed (lab item in `m5-notes.md`); +TST-19 is not run. + +### 12.4 Update signatures (TB-13, TB-14; T-66 to T-72) + +| Property claimed (04 §9.2-§9.4) | Code | Verdict | +|---|---|---| +| The manifest is signed with ed25519 over `domain‖bytes` at its exact parsed length | `src/sd_update/update_verify.c:305-322`, applied at `:788`; the `sig ed25519` block is mandatory at `:345` | carried in `sd_update` | +| `keyset` rotations need the current roots, and a rotation signed by only one root is refused | `update_verify.c:560-583`, `:677-681`, `:707-737` | carried in `sd_update` | +| Per-channel anti-rollback and an `expires` check (+ 30/14-day freshness) | `update_verify.c:793-801`, `update_state.c:240-256` (a write below the stored serial is refused), constants in `sd_update.h:39` | carried in `sd_update` | +| Downloads only from the manifest bases and the GitHub asset hosts; redirects are followed by hand | `update_winhttp.c:96-114`, allow-list built at `update_engine.c:571-590` | carried in `sd_update` | +| Size and SHA-256/SHA-512 are checked before a staged file is used | `update_engine.c:166-181`, `:596-645` | carried in `sd_update` | +| **The shipping host knows the release roots** | `apps/shelldrive-host/update_ipc.c:30-31`: `k_trust = { NULL, 0, NULL, 0 }`, "No release roots in this build" | **absent** | + +This is the one place where a security property this document relies on has code behind it that the product never +reaches: the verifier is complete, but with an empty root set nothing can be verified, and 04 §9 assumes a +compiled-in release key. Until the roots are compiled in, the update path has *no* signature property at all — it is +not "weak", it is unused. Recorded as a proposed `TM-R40` (12.9), because it is a release blocker for M4 rather than +a defect of the verifier. + +### 12.5 DPAPI and the secret stores (TB-09, TB-15; T-53) + +| Property claimed (03 §4.2-§4.5) | Code | Verdict | +|---|---|---| +| DPAPI with per-target entropy and no UI | `src/sd_secret/secret.c:371-383` (`CryptProtectData`), `:385-414` (unprotect), `:416-421` (entropy = `com.librestatic.shelldrive/secret/v1/` + target) | carried | +| Credential Manager for the user store: `CRED_TYPE_GENERIC`, persistence floor `LOCAL_MACHINE`, `ENTERPRISE` only when roaming is enabled | `secret.c:484-508`, `:536-570` | carried | +| The service never uses Credential Manager; SYSTEM DPAPI only; a user store is redirected | `apps/shelldrive-host/service.c:3287-3299` (`svc_setup_secrets`) | carried | +| `service\keys\` and `service\secrets\` carry a SYSTEM-only DACL | `service.c:331-333` (SDDL `D:PAI(A;OICI;FA;;;SY)`), applied at `:452-455` | carried | +| Secret buffers are wiped | `src/sd_platform/secbuf.c:53` (`SecureZeroMemory` on free), DPAPI outputs and Cred blobs wiped at `secret.c:411`, `:562` | carried | +| Crash dumps drop the stacks of threads that may hold secrets (TM-R30, TST-28) | none: no `MiniDumpWriteDump` and no `sd_dump_mark_sensitive` anywhere in the tree; `agent.c:3643` says "Crash dumps (03 §10.4) come later" | **absent** | + +### 12.6 Event Log (TB-08; T-43, T-46) + +| Property claimed (03 §10.3) | Code | Verdict | +|---|---|---| +| The source and message file are registered in order, and the message file is the host executable | `apps/shelldrive-host/svc_scm.c:360-366`, `service.c:820-831` | carried | +| Entries are written through `ReportEventW` on the service's own source | `service.c:820-836` | carried | +| Event 1200 (mutating op) carries profile, requestor SID, account, operation and remote path | `service.c:1078-1116` | carried | +| Event 2300 (host-key announcement) carries host, new and dropped fingerprints | `service.c:1488-1498` | carried | +| Event 3101 (pipe refusal) carries the SID and the reason, throttled to once per SID per 10 minutes | `service.c:3247-3263`, throttle table `service.c:188` | carried | +| Events 3102 (folder/key refused) carry the path and the reason | `service.c:800-816` | carried | +| Event 3100 (refusal counts) | no occurrence in code, headers or `l10n/svc` | **absent** | +| All text comes from the six-locale catalog compiled into the binary | `l10n/svc/*.po` (six), generated header included at `service.c:39`, message table built in `apps/shelldrive-host/CMakeLists.txt` | carried | + +### 12.7 Service inputs (T-51, TM-R27) + +The checked-read rule is implemented and tested for the paths the service actually reads: identity files +(`svc_folder.c`, `svc_file_read_trusted`), machine profiles (`service.c`, `svc_profile_read`) and the machine +`known_hosts` (`svc_kh_read_check`). Every folder and the file are opened with `FILE_FLAG_OPEN_REPARSE_POINT` and +checked on the handle (owner, DACL, size). Three gaps stay open and are recorded in `m5-notes.md`: + +- `settings.ini` is never read by the service, so there is nothing to check (`m5-notes.md:889-891`). A check on a + read that does not exist would be a fake; the check belongs to the change that introduces the read. +- Between `svc_kh_read_check` and the mount's own open of the machine `known_hosts` the file can change + (`m5-notes.md:916-917`). Closing it needs the mount to take the bytes or keep the checked handle. +- `AllowSystemProxySspi` cannot tell an explicitly configured proxy from a WPAD/PAC-discovered one in the system + configuration the service reads (`m5-notes.md:1077-1080`). + +### 12.8 Claims with no code behind them + +Collecting the "absent" verdicts and the open items that no code addresses: + +| # | Claim | Where it is claimed | State | +|---|---|---|---| +| 1 | Update artifacts are verified against a release key | 04 §9.2; T-66, T-67 | verifier complete, **no roots compiled in** (`update_ipc.c:30-31`) | +| 2 | Crash dumps drop secret-bearing stacks and WER is not invoked after ours | 03 §10.4; TM-R30, TST-28 | no dump code at all | +| 3 | Event 3100 reports refusal counts | 03 §10.3 | no code | +| 4 | A global WebDAV export authorizes each member of `service_access`, groups included | 02 §7.7; T-52, G2/G7 | found absent (one owner SID, no groups, no service attach path); built in the same round (`m5-notes.md` Part 14), lab criteria G1–G3, G5 and the timing half of G7 still open | +| 5 | Fast user switching and RDP sessions are tracked for the attention flow, with an admin session list | 03 §2.6 | session changes are only logged (`service.c:3485-3486`); no list is kept | +| 6 | An elevated `cmd` reaches a WebDAV drive after `--elevated-view` | 03 §1.7.2; TM-R17(e) | the flag has no implementation (only a UI test names it) | +| 7 | `settings.ini`, machine `known_hosts` and machine profile paths are checked as the service reads them | 03 §2.6 rule 1; TM-R27 | profiles and identity files yes; `settings.ini` never read; `known_hosts` has a check-to-use window | +| 8 | `AllowSystemProxySspi` does not apply to a discovered proxy | 01 §5.2; TM-R10 | cannot distinguish discovered from configured proxies | +| 9 | TST-23, TST-25 and TST-26 were executed | section 7 | not run; unit tests cover the rules, not a real admin token, XP, or a second session | + +### 12.9 Proposed additions (need acceptance) + +- **TM-R40** — 04 §9.2, §12.3; compile the release roots into the host (`update_ipc.c`), and fail the build when + the root table is empty in a release configuration. *Rationale:* 12.4; without it the updater has no signature + property. +- **TM-R41** — 02 §7.7; give the WebDAV server a service mode: the server-level owner check yields to the + per-request `sd_vfs` check, and the authenticated connection carries the token's enabled, non-deny-only groups + so that `service_access` group entries match. *Rationale:* 12.2, 12.8 #4; it is the prerequisite of the attach + mechanism the M5 notes call for. *Implemented* in the same round (`m5-notes.md` Part 14); 02 §7.7 already + describes the behaviour, so only the acceptance of this note is needed. +- **TM-R42** — 03 §10.3; either implement Event 3100 or drop it from the table and the catalog. *Rationale:* 12.6. +- **TM-R43** — 03 §2.6; keep the admin session list (logon/logoff, RDP, FUS) for the attention flow, and say what + a global-drive prompt does when no administrator is logged on. *Rationale:* 12.8 #5. +- **TM-R44** — 03 §2.6 rule 1; extend the checked read to `settings.ini` in the change that makes the service read + it, and close the `known_hosts` check-to-use window by passing the checked bytes to the mount. *Rationale:* 12.7. +- **TM-R45** — 01 §5.2; teach the system-proxy resolver to report whether a proxy came from a PAC/WPAD discovery or + from a configured value, so `AllowSystemProxySspi` can be applied to the configured case only. *Rationale:* 12.7. + +--- + ## Changelog +- M5 implementation review 2026-10-30: section 12 (pipes, loopback WebDAV, Dokany requestor check, update + signatures, DPAPI, Event Log, service inputs); eight "no code" findings and six proposed `TM-R40`-`TM-R45`. + The most serious are the empty update root table and the missing crash-dump filtering. - Final reconciliation (threat model) 2026-10-24: outcome note in the header; TST-05 says "never decoded" instead of "rejected"; TST-07, TST-13, TST-20 and TST-23 (per-SID quota, reserved instances, 64 KiB cap) follow D33, D40, D41 and 03 §2.2; decision notes on TM-R04, R13, R14, R24, R30; A-09 per-channel state; new section 11 (Q diff --git a/docs/dev/lab-results.md b/docs/dev/lab-results.md index 0dd3aa2..17a2de8 100644 --- a/docs/dev/lab-results.md +++ b/docs/dev/lab-results.md @@ -318,3 +318,55 @@ Notes for the record: `InstallFrom`) for Win11/Server. - New-LabVM.ps1 still does not set AutomaticStartAction; it was set manually on the recreated VM. + +## Appendix: procedure for the live global Dokany mount (M5, not yet run) + +Written 2026-10-30 because `sd-win10-ltsc21-x64` was held by the M3 acceptance run. Nothing here has been executed; +it is the exact sequence to run as soon as the VM is free, so that the first live global mount is a measurement and +not an exploration. It closes the "a real global mount seen by a second session" item of `docs/dev/m5-notes.md` +(M5 rows 2 and "Left") and the M5 demo (a) of `docs/design/00-integration-review.md` §6 for Windows 10 (Server 2022 +needs a Gen2 guest, still blocked by the host DVD defect of this file's 2026-10-29 night section; Gen1 validates the +code but not Secure Boot). + +Preconditions + +- `sd-win10-ltsc21-x64` restored to its `base` checkpoint (`Invoke-SdLab.ps1` restores before a run). Two local + accounts exist: `sduser1` and `sduser2` (create them with `net user ... /add` inside the guest if the base lacks + them; both standard users, distinct passwords). +- The payload ISO of the build under test is attached (`Invoke-SdLab.ps1` convention: IDE 1:0). +- Dokany 2.1+ is installed in the base (the payload installs it). `sc query dokan2` in the guest reports RUNNING. +- A test service profile. Write `%ProgramData%\LibreStatic\ShellDrive\profiles\global10.ini` on the guest: + `scope = service`, `user = svc`, `backend = dokany`, one `service_access` entry per account + (`service_access = sduser1, sduser2` — resolved to SIDs by the service), `service_access_write` optional, + `letter = Z`, `autostart = boot`, and a `[host]` that reaches the WSL sshd fixture (host-side port forwarded into + the guest, as the M0 smoke runs already do). +- Elevation for the service install and for the two `net user` calls; the mount and the check itself are unelevated. + +Steps + +1. Install the payload from the ISO (`D:\sdlab\staging\sd-win10-wimdisk.vhdx` style staging is not needed; the + payload ISO installs over the base). Record `sc query ShellDriveSvc`: STOPPED before the profile exists. +2. Start the service (`sc start ShellDriveSvc`) and read the Application log for Event 1000/1001 (started) and the + mount events. Expect Event 1001 with the profile count. +3. Confirm the drive at the service side: `shelldrive list --machine` as an elevated administrator shows `Z:` with + state `mounted`; as a non-admin in `service_access` it shows the letter and label but no host/user. +4. Log on as `sduser1` (console). In `cmd`: `dir Z:\` must list the remote directory with no prompt and no + elevation. In Explorer, `Z:` shows the label. Record `whoami`. +5. Fast user switch to `sduser2`. Same check. The drive must already be there (G1), and the server (the WSL sshd + fixture's log) must show **two different remote sessions** for the same service identity — that is the expected + result: the remote identity is shared (T-43), the local access control is `service_access` (G7). +6. Negative access (G7): from `sduser2`, remove `sduser2` from `service_access` in the profile and run + `shelldrive config reload --machine` (elevated). Within 60 s `dir Z:\` must fail with access denied; the audit + Event 1200 names the SID of each mutating operation before the change. +7. Write test: create, rename and delete one file as `sduser1` through `Z:` (with `service_access_write` naming + `sduser1` only, `sduser2` must get access denied on the write). +8. Reboot the guest. After the boot, before any interactive logon, the service must have mounted `Z:` (Event 1001 + + the mount events). Verify at the first logon that the letter is present and usable without user action. +9. Uninstall check: `sc stop ShellDriveSvc` then `sc delete`; confirm no orphaned drive letter remains in either + session (the Dokany volume disappears with the service). +10. Collect: `%ProgramData%\LibreStatic\ShellDrive\logs\*` and the Application log export, and append a result + section to this file (per-OS table: G1/G2/G5/G7 pass/fail, the two remote session identities seen by sshd). + +What would fail the acceptance: the drive absent before logon, a mount that only appears after a user runs +`shelldrive mount`, a `dir Z:` prompt or 403 for a `service_access` member, data visible to a removed member after +the 60 s window, or writes accepted from a member outside `service_access_write`. diff --git a/docs/dev/m5-notes.md b/docs/dev/m5-notes.md index a557b16..dacf065 100644 --- a/docs/dev/m5-notes.md +++ b/docs/dev/m5-notes.md @@ -922,7 +922,10 @@ for its 30 raw instances and its administrator, since an unelevated test process a real client race. - The hello deadline (5 s) is this build's choice; 03 §2.2 gives none. A slow client on a loaded machine is the case to measure in the lab (TST-23 in 05 §7). -- **Heap corruption on a second mount of the same profile (unit test, not isolated).** In `unit.svc`, two mounts of the +- **Heap corruption on a second mount of the same profile — CLOSED 2026-10-30, see "Part 14" below: it was a double + free in the test harness (`wait_state` kept a status reply that the next call freed again), fixed by `json_drop` in + `e2c3df5`, with the regression test `svc_one_profile_mounted_twice_keeps_both_announcements`. The original record:** + In `unit.svc`, two mounts of the `rotate` profile one after the other (each raising an UpdateHostKeys announcement, each then unmounted) end the test process with 0xC0000374 after the second announcement callback, before the second `wait_state`. It happens with the policy ask (1) in both mounts, with no reject, and with the machine-store write stubbed out, so it is not the @@ -1178,3 +1181,156 @@ The mapping mirrors `lang_from_langid` in `src/sd_l10n/l10n.c`, which selects th `.bin` files are named MSG by mc.exe and are not listed as build outputs. `unit.svc` checks that the table has every LANGID of the required list (including es-ES 0x0C0A, es-MX 0x080A, pt-PT 0x0816, fr-CA 0x0C0C, de-AT 0x0C07, it-CH 0x0810, en-GB 0x0809) and that each id resolves in each LANGID to the text of its block. + +## Part 14: threat-model review, the double-mount diagnosis, the service attach stack (2026-10-30) + +This round closes three of the four items the M5 brief ranked before the live lab run: the threat-model review that +`00-integration-review.md` §4/§6 asks for, the undiagnosed heap corruption of Part 10, and the service half of the +WebDAV attach path (02 §7.7). The live global Dokany mount still needs `sd-win10-ltsc21-x64`, which `m3-lab` holds. + +### 1. Threat-model review (05 §12) + +`docs/design/05-threat-model.md` gains section 12: per surface — named pipes, loopback WebDAV, the Dokany requestor +check, update signatures, DPAPI, the Event Log, and the service's own inputs — the property the design claims, the +code that carries it with `file:line`, and a verdict: carried, partial or absent. Nothing in it is normative; the +gaps become eight "no code" findings and six proposed `TM-R40`-`TM-R45` that the owning documents must accept. + +The findings that matter most: + +- **The updater has no trust roots in this build.** `apps/shelldrive-host/update_ipc.c:30-31` passes + `{ NULL, 0, NULL, 0 }`. The verifier in `sd_update` is complete (ed25519 over the exact manifest bytes, keyset, + channel, `expires`, anti-rollback, host allow-list, size and digest), but with an empty root set no manifest can + verify, so the product has no update signature property at all. Proposed `TM-R40`: compile the roots in and fail + the build when a release configuration has none. +- **Crash dumps do not exist.** No `MiniDumpWriteDump` and no `sd_dump_mark_sensitive` anywhere; `agent.c:3643` says + "come later". 03 §10.4, TM-R30 and TST-28 have nothing behind them. +- **Event 3100 has no code.** 1200, 2300, 3101 and 3102 are written and localized; 3100 appears in no source file, + header or catalog entry. +- **The WebDAV server cannot authorize a group.** `wd_server.c:466` (before this change) compared the authenticated SID with a single + owner SID and `:541` passed no token groups, so a `service_access` entry that names a group could never match. + That was the blocking gap for the attach path and is fixed here (section 3). +- Fast user switching/RDP sessions are logged only (`service.c:3485-3486`), the admin session list of 2.6 does + not exist, `--elevated-view` has no implementation, `settings.ini` is never read by the service, and the machine + `known_hosts` keeps its check-to-use window. These were already "Left" items; the review now records them as + claims with no code behind them rather than as work in progress. + +### 2. The double mount of Part 10 was a test-harness double free + +Root cause: `wait_state(c, id, want, timeout, &st)` kept the status reply in a static and freed the previous one; +tests that called it twice without dropping `st` in between (the announcement tests did) freed the same `sd_json` +twice, and the debug CRT ended the process with 0xC0000374 *before* the second `wait_state` — exactly the "after the +second announcement callback" signature Part 10 recorded. It was in the test process only; `service.c` and +`sd_mount` were not involved, which is why the machine-store stub and the reject path made no difference. + +`e2c3df5` added `json_drop(&st)` (free and clear the pointer) and the regression test +`svc_one_profile_mounted_twice_keeps_both_announcements`, which mounts the `rotate` profile twice, checks that both +mount ids and both announcement ids differ, and unmounts both. Verified 2026-10-30: that test passes on its own and +the whole `sd-test-svc.exe` suite passes in a Debug build (97 tests, 0 failed) — see section 4. + +### 3. The service attach stack (02 §7.7, 03 §1.3, D11) + +Implemented in the shape the spike chose: the service hosts the export, each user's agent maps it in its own logon +session. `system` and `link` stay unbuilt and the compiled table still has no GO cell (`svc_gw.c`), so `auto` +resolves to attach. + +**What was built** + +| Path | What | +|---|---| +| `src/sd_webdav/sd_webdav.h`, `wd_server.c`, `wd_server.h`, `wd_auth.c`, `wd_client.c` | `service_mode` on the server: the connection-level owner check yields to the per-request `sd_vfs` check, and the authenticated connection carries the token's enabled, non-deny-only groups (`wd_auth_step` grows `groups`/`n_groups`, `wd_groups_free`); `no_map` on the mount (host the export, map nothing); `sd_webdav_mount_remote` for the export's UNC | +| `src/sd_mount/sd_mount.h`, `mount.c`, `mount_real.c` | `webdav_remote` backend entry; `sd_mount_attach.webdav_remote` / `.webdav_remote_xp`; a global WebDAV mount is hosted with `no_map` and reports its export | +| `apps/shelldrive-host/svc_access.c` | `backend = webdav` is a valid service profile again (the mechanism is chosen at mount time) | +| `apps/shelldrive-host/service.c` | `job_start` uses the WebDAV backend for a profile that asks for it; `svc_on_state` records the export; `mount.attach_info` for a member of `service_access`; feature name | +| `apps/shelldrive-host/agent.c` | `mount.attach` (asks the service, maps with `svc_gw_attach_real_ops`) and `attach_global_webdav()` at logon (lists the service's mounts, attaches each `unc`) | +| `apps/shelldrive-host/CMakeLists.txt`, `tests/apps/CMakeLists.txt` | `svc_gw_attach.c` and `mpr` in the host and in the fake-host test | +| `tests/unit/sd_svc/test_service.c`, `test_access.c`, `main.c`; `tests/unit/sd_mount/fake_backend.c` | two fixture profiles, two tests, the fake backend's `webdav_remote`, the profile count 18 → 20 | + +**Flow.** At logon the agent lists the service's mounts (the reply already hides the ones the caller is not in +`service_access` for), asks `mount.attach_info` for each, and maps the `unc`/`xp_url` it gets back. The service +decides from `svc_decide(use)`, so a non-member gets `access_denied` and no token leaves the service. The mapping is +not persistent; it is cancelled with every other `\\localhost@port` mapping of the session at agent start and at +stop (`cleanup_webdav_mappings`). The server binds loopback only, and every request is checked per export by +`sd_vfs` against `service_access`/`service_access_write` with the caller's SID and groups (02 §6.7 rules, G7). + +**Three rules added after review** (before the commit, on a second read of the first draft): + +- *The mount-start cleanup would have cancelled the attach.* `sd_webdav_mount_start` removes every + `\\localhost@port` mapping of the session before it binds the agent's own server (TM-R17 d). A user who mounted a + per-user WebDAV profile after logon would have lost the global drive. `sd_webdav_preserve_port` (backend entry + `webdav_preserve_port`) now registers the service's port after a successful attach, and only that internal + cleanup skips it; the agent's explicit start and stop cleanups still remove the mapping. An agent crash leaves + the attach mapping in place, which is not the T-32 squat case: the port belongs to the running service, not to + the dead agent. The next agent start removes it and attaches again. +- *Anonymous, Guest, LOCAL SERVICE, NETWORK SERVICE* (02 §7.7 "rejected always ... unless listed"). In service mode + `wd_server.c` refuses with 403 an anonymous token, a Guest account (RID 501) and any member of `BUILTIN\Guests`, + before `sd_vfs` sees them. LOCAL SERVICE and NETWORK SERVICE keep their user SID but lose their groups, so a group + entry (Everyone, Users) never admits them; only an explicit SID entry in `service_access` does. +- *The mechanism decision is wired.* `job_start` asks `svc_gw_decide` (mode `auto`: the service does not read + `settings.ini`) before a WebDAV global mount. Attach is used; a refusal (OS family unknown, WebClient missing or + disabled) fails the mount with `usage` and the reason; a `system` or `link` verdict — possible only after a lab run + flips a table cell — is refused as "not in this build" instead of being served as attach. `svc_gw.c` is now part + of the shipping host. + +**G1-G7, what is covered without a second logon** + +| # | Criterion | State | +|---|---|---| +| G1 | Visibility in every session | not testable here (needs a second logon); attach is per session by construction | +| G2 | The server sees each user's own SID, no shared connection | unit: `mount.attach_info` returns the export only to a member, and the server keeps one SSPI context per TCP connection (`wd_auth`); **the two-user test is a lab item** | +| G3 | WebClient/service restart, sleep/resume | no: lab | +| G4 | No security-setting changes | attach changes none; WebClient start is the existing path | +| G5 | Function (1 GiB copy, Office locks) | no: lab. The export is the same `sd_webdav` the per-user path uses | +| G6 | Before logon | not claimed for attach (D11: only Dokany is pre-logon) | +| G7 | Negative access | unit: a caller outside `service_access` gets `access_denied` from the service and `403` from the server; groups now reach `sd_vfs`. Membership-change timing (60 s) and the session-0 case: lab | + +**Tests added** (`unit.svc`, all in-process, no second session): + +- `svc_webdav_mount_reports_its_attach_point`: a `backend = webdav` global profile mounts, `mount.attach_info` + returns `backend = webdav`, `attach = true`, the `\\localhost@49321\DavWWWRoot\s-<26>` UNC, the XP `http://` form + and the port; a Dokany global mount answers `attach = false` with no UNC. +- `svc_attach_info_refuses_a_caller_outside_service_access`: a profile whose `service_access` names a SID this + process does not carry mounts, and `mount.attach_info` answers `access_denied` with no UNC. + +And in `unit.webdav`, against a real loopback server with real SSPI (the test user's own token): + +- `webdav_service_server_admits_a_member`: a global vfs whose `service_access` is the test user, a server owned by + SYSTEM in service mode: the authenticated PROPFIND answers 207 (the per-user server answers 403 for the same + owner, `webdav_server_rejects_other_sid`). +- `webdav_service_server_refuses_a_non_member`: the same server with `service_access` naming LOCAL SERVICE: + authentication succeeds and `sd_vfs` refuses, so no listing reaches a non-member. + +**Not done, and why** + +- The service's own `AcceptSecurityContext` as LocalSystem, and the token groups of a real user token, are not + exercised: the tests use the fake backend and no real SSPI session. This is the lab half of G2/G7. +- `system`/`link` mechanisms, `--elevated-view`, and the per-request fallback of 02 §7.7 (switch to + `Connection: close` on an OS where the redirector shares connections) are untouched. +- The XP form (`http://localhost:PORT/s-`, provider "Web Client Network") is compiled but unrun; `attach` on + XP is a lab item. +- `sd_webdav` and `sd_mount` are outside the paths the brief listed as this lane's; the two changes there are the + minimum the 02 §7.7 design needs (the server authorization gap of 05 §12.2 and the hosted-not-mapped export). + `apps/shelldrive-ui`, `src/sd_update`, `apps/shelldrive-setup` and `packaging/` were not touched. + +### 4. Verification + +- `x64-modern` Debug: full build, 0 errors, 0 warnings; `unit.webdav` 28/28 (26 existing, 2 new) after the service-mode change (the + first run caught a double free of the user SID in the 403 branch, fixed). +- `sd-test-svc.exe` (Debug): 97 tests, 0 failed, including both new tests and the double-mount regression test. +- `logpriv.source_check`: no problems (one `%lu` in a new agent log line was changed to `%u`). +- `unit.webdav` adds the two service-server tests above (real SSPI against a real loopback server). +- Local gate, 2026-10-30, `tools\ci-local.ps1 -Pipeline` (Release and Debug, `-LE "matrix|bench|repro"`): x64-modern, + x64-xp, x86-ia32, x86-sse2 and x64-analysis all pass, every test passes; the only skips are the expected + `dokany.mount` ones under WOW64 on the two x86 flavours. `repro.x64-modern` (the label the gate excludes and CI + runs) passes too. +- Gate trap: the WSL sshd fixture is shared between checkouts. When it already runs, `start.sh` exits before it + writes this checkout's `tests/fixtures/wsl-sshd/state/`, the interop tests skip, and `ci-local.ps1` fails the + preset on the skip. Copying `env` and the two client keys from the checkout that started the fixture (the + folder is gitignored) fixes it without restarting a fixture other sessions use. + +### 5. Left after this round + +- The live global Dokany mount on `sd-win10-ltsc21-x64` (the VM is `m3-lab`'s); the procedure is in + `docs/dev/lab-results.md` (appendix) so it can run as soon as the VM is free. +- Everything in 05 §12.8 that has no code: release roots, crash dumps, Event 3100, the FUS/RDP session list, + `--elevated-view`, `settings.ini` checks, discovered-proxy detection. +- The per-user identity test (two users, each mapping the other's UNC) and the rest of G1-G7. diff --git a/src/sd_mount/include/sd_mount/sd_mount.h b/src/sd_mount/include/sd_mount/sd_mount.h index 3be4a69..5d0dffe 100644 --- a/src/sd_mount/include/sd_mount/sd_mount.h +++ b/src/sd_mount/include/sd_mount/sd_mount.h @@ -111,6 +111,10 @@ typedef struct sd_mount_attach { wchar_t letter; int mounted; uint16_t webdav_port; /* 0 unless the backend is WebDAV and the export exists */ + /* 02 §7.7 attach: the export's remote name (\\localhost@PORT\DavWWWRoot\s-, or the XP http form), + empty when there is none. A service mount is hosted and not mapped; this is what a user's agent maps. */ + wchar_t webdav_remote[104]; + wchar_t webdav_remote_xp[104]; /* the XP form (http://localhost:PORT/s-), empty when none */ } sd_mount_attach; /* A host key as the host keeps it (UpdateHostKeys announcements and hostkey add). */ @@ -182,6 +186,12 @@ typedef struct sd_mount_backend { sd_err (*hk_conn_fetch)(sd_ssh_conn *c, uint32_t hop_index, const char *const *types, uint32_t ntypes, sd_ssh_hostkeys_cb done, void *ctx); void (*hk_conn_destroy)(sd_ssh_conn *c); + /* 02 §7.7 attach (M5, added last, NULL in older fakes): the remote name of a hosted WebDAV export, which the + service hands to a user's agent over mount.attach_info. */ + sd_err (*webdav_remote)(const sd_webdav_mount *m, BOOL xp_form, wchar_t *out, size_t cap); + /* 02 §7.7 attach, agent side (NULL in fakes: nothing is preserved): keep the service's port out of the + mount-start cleanup of this process (sd_webdav_preserve_port). */ + void (*webdav_preserve_port)(uint16_t port, BOOL on); } sd_mount_backend; const char *sd_mount_backend_kind_name(sd_mount_backend_kind k); /* "auto", "dokany", "webdav" */ diff --git a/src/sd_mount/mount.c b/src/sd_mount/mount.c index ec0c84e..910d02a 100644 --- a/src/sd_mount/mount.c +++ b/src/sd_mount/mount.c @@ -737,6 +737,9 @@ static sd_err webdav_start(sd_mount *m) wc.label = m->label ? m->label : m->host; wc.owner_sid = m->owner_sid; wc.global = m->global_mount ? TRUE : FALSE; /* TM-R19: token-only is always refused for global mounts */ + /* 02 §7.7 attach: the service hosts the export and maps no drive of its own; each user's agent maps the UNC + that mount.attach_info returns. Dokany stays the pre-logon path (D11). */ + wc.no_map = m->global_mount ? TRUE : FALSE; wc.token_only = m->token_only ? TRUE : FALSE; wc.token_only_confirmed = m->token_only_confirmed ? TRUE : FALSE; e = m->be->webdav_mount_start(m->vfs, &wc, &m->wm); @@ -2435,6 +2438,10 @@ void sd_mount_get_attach(const sd_mount *cm, sd_mount_attach *out) out->letter = m->letter; out->mounted = m->state == SD_MOUNT_MOUNTED; out->webdav_port = m->wport; + if (m->wm && m->be->webdav_remote) { + (void)m->be->webdav_remote(m->wm, FALSE, out->webdav_remote, SD_COUNTOF(out->webdav_remote)); + (void)m->be->webdav_remote(m->wm, TRUE, out->webdav_remote_xp, SD_COUNTOF(out->webdav_remote_xp)); + } sd_mutex_unlock(&m->lock); } diff --git a/src/sd_mount/mount_real.c b/src/sd_mount/mount_real.c index b720d4c..0ba141c 100644 --- a/src/sd_mount/mount_real.c +++ b/src/sd_mount/mount_real.c @@ -143,6 +143,8 @@ static const sd_mount_backend k_real = { real_hk_conn_connect, real_hk_conn_fetch, real_hk_conn_destroy, + sd_webdav_mount_remote, + sd_webdav_preserve_port, }; const sd_mount_backend *sd_mount_backend_real(void) diff --git a/src/sd_webdav/include/sd_webdav/sd_webdav.h b/src/sd_webdav/include/sd_webdav/sd_webdav.h index 268cdc3..00d73e1 100644 --- a/src/sd_webdav/include/sd_webdav/sd_webdav.h +++ b/src/sd_webdav/include/sd_webdav/sd_webdav.h @@ -73,6 +73,11 @@ typedef struct sd_webdav_server_cfg { uint32_t max_unauth_per_process; /* 4 */ uint32_t max_connections; /* 64 */ BOOL token_only; /* TM-R19: started for a token-only export, which needs no SSPI to start */ + /* 02 §7.7: a service (global) server. The owner check is not applied at the connection: any user that + authenticates reaches the per-request sd_vfs check, which decides per export from service_access. The + caller's enabled, non-deny-only token groups are passed to that check, so group entries match. Never set + together with token_only, which TM-R19 forbids for global mounts. */ + BOOL service_mode; } sd_webdav_server_cfg; sd_err sd_webdav_server_start(const sd_webdav_server_cfg *cfg, sd_webdav_server **out); @@ -107,6 +112,11 @@ sd_err sd_webdav_cleanup_mappings(uint16_t keep_port, uint16_t only_port, uint32 /* Crash filter (TM-R17 b): cancels the mappings made by this process. Safe to call in an exception filter (no allocation, no locks beyond a try-lock). */ void sd_webdav_emergency_unmap(void); +/* 02 §7.7 attach: the port of a server this process does not own (the service's) whose mappings are live, not + stale. The cleanup that sd_webdav_mount_start runs before it binds skips these ports (up to 4); the explicit + sd_webdav_cleanup_mappings calls of the host (start, stop, watchers) do not, so a stopping agent still removes + them. on = 0 forgets the port. Any thread. */ +void sd_webdav_preserve_port(uint16_t port, BOOL on); /* ---- one-call mount used by sd_mount (process-wide server, started on first use) ---- */ @@ -121,6 +131,9 @@ typedef struct sd_webdav_mount_cfg { BOOL global; /* a service (global) mount: token-only is always refused */ BOOL token_only; /* request token-only mode (TM-R19); the host asked the user first */ BOOL token_only_confirmed; /* the user confirmed this enable in the host dialog (03 7.5) */ + /* 02 §7.7 attach: the service hosts the export and maps nothing. The drive letter is still carried (it is + the letter users see); no connection is made and no MountPoints2 entry is written. */ + BOOL no_map; } sd_webdav_mount_cfg; /* Starts the process server if needed (cleaning stale mappings first), adds the export, maps the @@ -129,6 +142,9 @@ sd_err sd_webdav_mount_start(sd_vfs *vfs, const sd_webdav_mount_cfg *cfg, sd_web /* Unmaps, removes the export, stops the server with the last mount. Frees m. */ sd_err sd_webdav_mount_stop(sd_webdav_mount *m); uint16_t sd_webdav_mount_port(const sd_webdav_mount *m); +/* The remote name of the export: Vista+ \\localhost@PORT\DavWWWRoot\s-, XP http://localhost:PORT/s-. + A service mount hands this to the agent over mount.attach_info; the token is the capability. */ +sd_err sd_webdav_mount_remote(const sd_webdav_mount *m, BOOL xp_form, wchar_t *out, size_t cap); /* ---- token-only mode (TM-R19, 02 §7.4) ---- Used when the redirector cannot authenticate. Never switched on by sd_webdav itself: the host asks the diff --git a/src/sd_webdav/wd_auth.c b/src/sd_webdav/wd_auth.c index 777ea6b..50ce3e5 100644 --- a/src/sd_webdav/wd_auth.c +++ b/src/sd_webdav/wd_auth.c @@ -85,7 +85,66 @@ static PSID token_user(HANDLE tok) return copy; } -int wd_auth_step(wd_auth *a, const char *value, size_t n, char *out, size_t cap, PSID *user) +void wd_groups_free(PSID *groups, UINT n_groups) +{ + UINT i; + if (groups == NULL) + return; + for (i = 0; i < n_groups; i++) + sd_free(groups[i]); + sd_free(groups); +} + +/* Enabled, non-deny-only groups of the token (02 §7.7: service_access may name a group). A failure to copy one + entry drops that entry, never the whole set: the user SID still matches a user entry, and a missing group can + only deny. Returns 0 when the group set could not be read at all (out stays empty). */ +static int token_groups(HANDLE tok, PSID **out, UINT *n_out) +{ + DWORD len = 0, i, n = 0; + BYTE *buf; + TOKEN_GROUPS *tg; + PSID *arr; + *out = NULL; + *n_out = 0; + if (!GetTokenInformation(tok, TokenGroups, NULL, 0, &len) && GetLastError() != ERROR_INSUFFICIENT_BUFFER) + return 0; + if (len == 0) + return 0; + buf = (BYTE *)sd_alloc(len); + if (buf == NULL) + return 0; + if (!GetTokenInformation(tok, TokenGroups, buf, len, &len)) { + sd_free(buf); + return 0; + } + tg = (TOKEN_GROUPS *)buf; + arr = (PSID *)sd_calloc(tg->GroupCount ? tg->GroupCount : 1, sizeof(PSID)); + if (arr == NULL) { + sd_free(buf); + return 0; + } + for (i = 0; i < tg->GroupCount; i++) { + DWORD a = tg->Groups[i].Attributes, sl; + PSID s; + if (!(a & SE_GROUP_ENABLED) || (a & SE_GROUP_USE_FOR_DENY_ONLY)) + continue; + s = tg->Groups[i].Sid; + sl = GetLengthSid(s); + arr[n] = sd_alloc(sl); + if (arr[n] == NULL || !CopySid(sl, arr[n], s)) { + sd_free(arr[n]); + continue; + } + n++; + } + sd_free(buf); + *out = arr; + *n_out = n; + return 1; +} + +int wd_auth_step(wd_auth *a, const char *value, size_t n, char *out, size_t cap, PSID *user, + PSID **groups, UINT *n_groups) { size_t sl = 0, tl = 0; const char *scheme; @@ -98,6 +157,10 @@ int wd_auth_step(wd_auth *a, const char *value, size_t n, char *out, size_t cap, int rc = WD_AUTH_FAIL; out[0] = 0; *user = NULL; + if (groups) + *groups = NULL; + if (n_groups) + *n_groups = 0; if (!wd_auth_available()) return WD_AUTH_FAIL; while (sl < n && value[sl] != ' ') @@ -149,6 +212,8 @@ int wd_auth_step(wd_auth *a, const char *value, size_t n, char *out, size_t cap, HANDLE tok = NULL; if (g_ft->QuerySecurityContextToken((PCtxtHandle)a->ctx, &tok) == SEC_E_OK && tok) { *user = token_user(tok); + if (*user && groups && n_groups) + (void)token_groups(tok, groups, n_groups); CloseHandle(tok); } rc = *user ? WD_AUTH_OK : WD_AUTH_FAIL; diff --git a/src/sd_webdav/wd_client.c b/src/sd_webdav/wd_client.c index 3e478e1..c1d60f2 100644 --- a/src/sd_webdav/wd_client.c +++ b/src/sd_webdav/wd_client.c @@ -296,11 +296,50 @@ static int ours(const wchar_t *r, uint16_t *port) return 1; } +/* 02 §7.7 attach: ports of foreign (service) servers whose mappings the mount-start cleanup keeps. */ +#define WD_PRESERVE_MAX 4 +static volatile LONG g_preserve[WD_PRESERVE_MAX]; + +void sd_webdav_preserve_port(uint16_t port, BOOL on) +{ + int i; + if (port == 0) + return; + for (i = 0; i < WD_PRESERVE_MAX; i++) { + if (on && InterlockedCompareExchange(&g_preserve[i], 0, 0) == (LONG)port) + return; /* already kept */ + if (!on && InterlockedCompareExchange(&g_preserve[i], 0, (LONG)port) == (LONG)port) + return; + } + if (!on) + return; + for (i = 0; i < WD_PRESERVE_MAX; i++) + if (InterlockedCompareExchange(&g_preserve[i], (LONG)port, 0) == 0) + return; + SD_LOGW(WD_LOG, "more than %d preserved WebDAV ports; port %u is not kept", WD_PRESERVE_MAX, (unsigned)port); +} + +static int preserved(uint16_t port) +{ + int i; + for (i = 0; i < WD_PRESERVE_MAX; i++) + if (InterlockedCompareExchange(&g_preserve[i], 0, 0) == (LONG)port) + return 1; + return 0; +} + typedef DWORD (APIENTRY *pfn_WNetOpenEnumW)(DWORD, DWORD, DWORD, LPNETRESOURCEW, LPHANDLE); typedef DWORD (APIENTRY *pfn_WNetEnumResourceW)(HANDLE, LPDWORD, LPVOID, LPDWORD); typedef DWORD (APIENTRY *pfn_WNetCloseEnum)(HANDLE); +static sd_err cleanup_impl(uint16_t keep_port, uint16_t only_port, int keep_preserved, uint32_t *removed); + sd_err sd_webdav_cleanup_mappings(uint16_t keep_port, uint16_t only_port, uint32_t *removed) +{ + return cleanup_impl(keep_port, only_port, 0, removed); +} + +static sd_err cleanup_impl(uint16_t keep_port, uint16_t only_port, int keep_preserved, uint32_t *removed) { pfn_WNetOpenEnumW oe = (pfn_WNetOpenEnumW)sd_platform_resolve_by_name(L"mpr.dll", "WNetOpenEnumW"); pfn_WNetEnumResourceW er = (pfn_WNetEnumResourceW)sd_platform_resolve_by_name(L"mpr.dll", "WNetEnumResourceW"); @@ -322,7 +361,8 @@ sd_err sd_webdav_cleanup_mappings(uint16_t keep_port, uint16_t only_port, uint32 for (i = 0; i < n; i++) { NETRESOURCEW *r = &((NETRESOURCEW *)buf)[i]; uint16_t port; - if (r->lpRemoteName && ours(r->lpRemoteName, &port) && port != keep_port && (!only_port || port == only_port)) { + if (r->lpRemoteName && ours(r->lpRemoteName, &port) && port != keep_port && (!only_port || port == only_port) && + !(keep_preserved && preserved(port))) { const wchar_t *name = (r->lpLocalName && r->lpLocalName[0]) ? r->lpLocalName : r->lpRemoteName; if (WNetCancelConnection2W(name, 0, TRUE) == NO_ERROR) { count++; @@ -350,7 +390,7 @@ sd_err sd_webdav_cleanup_mappings(uint16_t keep_port, uint16_t only_port, uint32 unc[k] = name[k] == L'#' ? L'\\' : name[k]; unc[k] = 0; if (ours(unc, &port) && port != keep_port && (!only_port || port == only_port) && - RegDeleteKeyW(mp2, name) == ERROR_SUCCESS) + !(keep_preserved && preserved(port)) && RegDeleteKeyW(mp2, name) == ERROR_SUCCESS) continue; /* same index now holds the next key */ idx++; } @@ -365,6 +405,7 @@ sd_err sd_webdav_cleanup_mappings(uint16_t keep_port, uint16_t only_port, uint32 struct sd_webdav_mount { sd_webdav_export *ex; wchar_t letter; + BOOL no_map; /* 02 §7.7 attach: the service hosts the export and maps nothing */ }; static sd_once g_mount_once = SD_ONCE_INIT; @@ -438,12 +479,15 @@ sd_err sd_webdav_mount_start(sd_vfs *vfs, const sd_webdav_mount_cfg *cfg, sd_web if (g_srv == NULL) { sd_webdav_server_cfg sc; uint32_t removed = 0; - /* TM-R17 d: before binding, remove our stale mappings from earlier runs (any port) */ - sd_webdav_cleanup_mappings(0, 0, &removed); + /* TM-R17 d: before binding, remove our stale mappings from earlier runs (any port); a live attach of the + service's drive (02 §7.7, sd_webdav_preserve_port) is not stale and stays */ + cleanup_impl(0, 0, 1, &removed); memset(&sc, 0, sizeof(sc)); sc.struct_size = sizeof(sc); sc.owner_sid = cfg->owner_sid; sc.token_only = cfg->token_only; + /* 02 §7.7: a hosted (no_map) mount is a service server: sd_vfs decides per request, groups included */ + sc.service_mode = cfg->no_map ? TRUE : FALSE; e = sd_webdav_server_start(&sc, &g_srv); if (e == SD_OK) warn_redirector_limits(); @@ -464,7 +508,7 @@ sd_err sd_webdav_mount_start(sd_vfs *vfs, const sd_webdav_mount_cfg *cfg, sd_web /* the redirector and the self-test connect back to our server while this runs: no lock held */ if (cfg->token_only) e = sd_webdav_token_selftest(port, sd_webdav_export_prefix(m->ex)); - if (e == SD_OK) + if (e == SD_OK && !cfg->no_map) e = sd_webdav_map(m->ex, cfg->letter, &werr); sd_mutex_lock(&g_mount_mu); if (e != SD_OK) { @@ -483,6 +527,7 @@ sd_err sd_webdav_mount_start(sd_vfs *vfs, const sd_webdav_mount_cfg *cfg, sd_web return e; } m->letter = cfg->letter; + m->no_map = cfg->no_map ? TRUE : FALSE; *out = m; return SD_OK; } @@ -492,7 +537,7 @@ sd_err sd_webdav_mount_stop(sd_webdav_mount *m) sd_err e; if (m == NULL) return SD_E_INVALID_PARAM; - e = sd_webdav_unmap(m->ex, m->letter); + e = m->no_map ? SD_OK : sd_webdav_unmap(m->ex, m->letter); sd_mutex_lock(&g_mount_mu); sd_webdav_export_remove(m->ex); if (--g_refs == 0 && g_srv) { @@ -508,3 +553,10 @@ uint16_t sd_webdav_mount_port(const sd_webdav_mount *m) { return (m && m->ex) ? m->ex->srv->port : 0; } + +sd_err sd_webdav_mount_remote(const sd_webdav_mount *m, BOOL xp_form, wchar_t *out, size_t cap) +{ + if (m == NULL || m->ex == NULL) + return SD_E_INVALID_PARAM; + return sd_webdav_export_remote_name(m->ex, xp_form, out, cap); +} diff --git a/src/sd_webdav/wd_server.c b/src/sd_webdav/wd_server.c index ff03762..4a4b2b3 100644 --- a/src/sd_webdav/wd_server.c +++ b/src/sd_webdav/wd_server.c @@ -413,11 +413,14 @@ static void mark_token_conn(wd_conn *c) sd_mutex_unlock(&s->mu); } -static void set_authed(wd_conn *c, PSID user) +static void set_authed(wd_conn *c, PSID user, PSID *groups, UINT n_groups) { sd_webdav_server *s = c->srv; sd_free(c->user); c->user = user; + wd_groups_free(c->groups, c->n_groups); + c->groups = groups; + c->n_groups = n_groups; if (!c->authed) { c->authed = 1; sd_mutex_lock(&s->mu); @@ -429,6 +432,50 @@ static void set_authed(wd_conn *c, PSID user) } } +/* 1 when sid is in the NT authority (S-1-5-...) with exactly n sub-authorities, the first one rid0 and, for n == 2, + the second one rid1. */ +static int sid_is_nt(PSID sid, DWORD n, DWORD rid0, DWORD rid1) +{ + static const BYTE nt[6] = { 0, 0, 0, 0, 0, 5 }; /* SECURITY_NT_AUTHORITY */ + if (sid == NULL || !IsValidSid(sid) || *GetSidSubAuthorityCount(sid) != n) + return 0; + if (memcmp(GetSidIdentifierAuthority(sid)->Value, nt, sizeof(nt)) != 0 || *GetSidSubAuthority(sid, 0) != rid0) + return 0; + return n == 1 || *GetSidSubAuthority(sid, 1) == rid1; +} + +/* A Guest account of the machine or a domain: S-1-5-21----501. */ +static int sid_is_guest(PSID sid) +{ + static const BYTE nt[6] = { 0, 0, 0, 0, 0, 5 }; + DWORD n; + if (sid == NULL || !IsValidSid(sid) || (n = *GetSidSubAuthorityCount(sid)) < 2) + return 0; + return memcmp(GetSidIdentifierAuthority(sid)->Value, nt, sizeof(nt)) == 0 && + *GetSidSubAuthority(sid, 0) == SECURITY_NT_NON_UNIQUE && *GetSidSubAuthority(sid, n - 1) == DOMAIN_USER_RID_GUEST; +} + +/* 02 §7.7, service servers. Returns 0 for a caller that never reaches the per-request check: anonymous, a Guest + account, a member of BUILTIN\Guests. LOCAL SERVICE and NETWORK SERVICE are admitted "only if listed": their groups + are dropped (*groups NULL, *n_groups 0), so a group entry such as Everyone or Users never lets them in and only an + explicit SID entry in service_access does. SYSTEM goes on to sd_vfs, which admits it only with allow_system. */ +static int service_caller_ok(PSID user, PSID **groups, UINT *n_groups) +{ + UINT i; + if (sid_is_nt(user, 1, SECURITY_ANONYMOUS_LOGON_RID, 0) || sid_is_guest(user)) + return 0; + for (i = 0; *groups && i < *n_groups; i++) + if (sid_is_nt((*groups)[i], 2, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_GUESTS) || + sid_is_nt((*groups)[i], 1, SECURITY_ANONYMOUS_LOGON_RID, 0)) + return 0; + if (sid_is_nt(user, 1, SECURITY_LOCAL_SERVICE_RID, 0) || sid_is_nt(user, 1, SECURITY_NETWORK_SERVICE_RID, 0)) { + wd_groups_free(*groups, *n_groups); + *groups = NULL; + *n_groups = 0; + } + return 1; +} + /* Returns 1 when the request may proceed. */ static int authenticate(wd_rq *q) { @@ -437,7 +484,8 @@ static int authenticate(wd_rq *q) const wd_hdr *ah = wd_req_header(&q->r, "Authorization", &cnt); enum { OUT_CAP = 1024 * 64 / 3 + 64 }; char *out; - PSID user = NULL; + PSID user = NULL, *groups = NULL; + UINT n_groups = 0; int rc, ok = 0; if (cnt > 1) { respond_simple(q, 400, 1); @@ -456,23 +504,30 @@ static int authenticate(wd_rq *q) respond_simple(q, 500, 1); return 0; } - rc = wd_auth_step(&c->auth, ah->val, ah->vlen, out, OUT_CAP, &user); + rc = wd_auth_step(&c->auth, ah->val, ah->vlen, out, OUT_CAP, &user, &groups, &n_groups); if (rc == WD_AUTH_CONTINUE) { challenge(q, out); } else if (rc != WD_AUTH_OK) { c->authed = 0; q->close_after = 1; challenge(q, NULL); - } else if (!EqualSid(user, c->srv->owner)) { + } else if (!c->srv->service_mode && !EqualSid(user, c->srv->owner)) { SD_LOGW(WD_LOG, "rejected a caller whose SID is not the mount owner"); - sd_free(user); + respond_simple(q, 403, 1); /* user and groups are freed below, where every path frees them */ + } else if (c->srv->service_mode && !service_caller_ok(user, &groups, &n_groups)) { + SD_LOGW(WD_LOG, "rejected an anonymous or guest caller of a service server"); respond_simple(q, 403, 1); } else { if (out[0] && strlen(out) < sizeof(c->auth_reply)) memcpy(c->auth_reply, out, strlen(out) + 1); - set_authed(c, user); + set_authed(c, user, groups, n_groups); /* ownership moves to the connection */ + user = NULL; + groups = NULL; + n_groups = 0; ok = 1; } + wd_groups_free(groups, n_groups); + sd_free(user); sd_free(out); return ok; } @@ -538,8 +593,10 @@ static void handle(wd_rq *q) q->caller.sid = c->user; } q->caller.can_write = q->ex ? !q->ex->read_only : FALSE; - q->caller.groups = NULL; /* M5: no token groups on the per-user SSPI path (matched by SID) */ - q->caller.n_groups = 0; + /* 02 §7.7: a service server hands the token's enabled groups to sd_vfs, so service_access may name a group. + A per-user server keeps matching by SID only: the owner is the only caller it admits. */ + q->caller.groups = c->srv->service_mode ? c->groups : NULL; + q->caller.n_groups = c->srv->service_mode ? (uint32_t)c->n_groups : 0; q->vpath = wd_path_to_vfs(&q->path, 1); if (q->vpath == NULL) { respond_simple(q, 400, 0); @@ -646,6 +703,7 @@ static unsigned __stdcall conn_main(void *arg) conn_unlink(c); wd_auth_reset(&c->auth); sd_free(c->user); + wd_groups_free(c->groups, c->n_groups); sd_free(c->ib); sd_free(c); if (InterlockedDecrement(&s->live) == 0) @@ -874,6 +932,7 @@ sd_err sd_webdav_server_start(const sd_webdav_server_cfg *cfg, sd_webdav_server s->max_unauth = cfg->max_unauth ? cfg->max_unauth : 8u; s->max_unauth_pp = cfg->max_unauth_per_process ? cfg->max_unauth_per_process : 4u; s->max_conn = cfg->max_connections ? cfg->max_connections : 64u; + s->service_mode = cfg->service_mode ? TRUE : FALSE; for (attempt = 0; attempt < 16; attempt++) { struct sockaddr_in a; int al = sizeof(a); diff --git a/src/sd_webdav/wd_server.h b/src/sd_webdav/wd_server.h index 4130832..6040f9b 100644 --- a/src/sd_webdav/wd_server.h +++ b/src/sd_webdav/wd_server.h @@ -49,6 +49,7 @@ struct sd_webdav_server { SOCKET ls[2]; int nls; uint16_t port; PSID owner; + BOOL service_mode; /* 02 §7.7: no owner check at the connection; sd_vfs decides per request */ uint32_t header_ms, auth_ms, idle_ms, max_unauth, max_unauth_pp, max_conn; sd_thread accept_thr; volatile LONG stop; @@ -66,8 +67,12 @@ enum { WD_AUTH_FAIL = -1, WD_AUTH_CONTINUE = 0, WD_AUTH_OK = 1 }; typedef struct wd_auth { void *ctx; int have; } wd_auth; /* ctx: CtxtHandle (opaque here) */ int wd_auth_available(void); /* value = "Negotiate " or "NTLM ". On CONTINUE/OK with a token, out = " " - (NUL-terminated, empty when no token). On OK *user is the token user SID (sd_alloc). */ -int wd_auth_step(wd_auth *a, const char *value, size_t n, char *out, size_t cap, PSID *user); + (NUL-terminated, empty when no token). On OK *user is the token user SID (sd_alloc) and, when *groups is + not NULL, *groups/*n_groups are the token's enabled, non-deny-only group SIDs (one sd_alloc buffer plus an + index array, freed with wd_groups_free). The service server needs them: service_access may name a group. */ +int wd_auth_step(wd_auth *a, const char *value, size_t n, char *out, size_t cap, PSID *user, + PSID **groups, UINT *n_groups); +void wd_groups_free(PSID *groups, UINT n_groups); void wd_auth_reset(wd_auth *a); /* ---- connection ---- */ @@ -84,6 +89,8 @@ typedef struct wd_conn { volatile LONG evict; wd_auth auth; PSID user; + PSID *groups; /* enabled, non-deny-only groups of the authenticated token (service mode) */ + UINT n_groups; char auth_reply[1024]; /* final mutual-auth token for the next response, rarely used */ char *ib; size_t ilen, ipos; } wd_conn; diff --git a/tests/apps/CMakeLists.txt b/tests/apps/CMakeLists.txt index 9506d14..0253a11 100644 --- a/tests/apps/CMakeLists.txt +++ b/tests/apps/CMakeLists.txt @@ -13,12 +13,13 @@ add_executable(sd-test-host-fake "${_host_dir}/main.c" "${_host_dir}/agent.c" "${_host_dir}/log_ring.c" "${_host_dir}/state_producer.c" "${_host_dir}/explorer_data.c" "${_host_dir}/config_watch.c" "${_host_dir}/update_ipc.c" "${_host_dir}/host.h" host_fake_backend.c "${_host_dir}/installer_hooks.c" "${_host_dir}/service.c" "${_host_dir}/svc_access.c" "${_host_dir}/svc_folder.c" - "${_host_dir}/svc_scm.c" "${_host_dir}/svc_repair.c" "${_host_dir}/svc_status.c" "${_host_dir}/agent_events.c") + "${_host_dir}/svc_scm.c" "${_host_dir}/svc_repair.c" "${_host_dir}/svc_status.c" "${_host_dir}/svc_gw.c" "${_host_dir}/svc_gw_attach.c" + "${_host_dir}/agent_events.c") target_compile_definitions(sd-test-host-fake PRIVATE SD_HOST_TEST_HOOKS=1) target_include_directories(sd-test-host-fake PRIVATE "${_host_dir}" "${SD_GENERATED_DIR}/svc_events/$") add_dependencies(sd-test-host-fake sd_svc_events) target_link_libraries(sd-test-host-fake PRIVATE sd_headers sd_mount_fake_backend sd_mount sd_ipc sd_state sd_secret sd_config sd_update sd_policy - sd_l10n sd_l10n_catalog_svc sd_ssh sd_platform sd_base ole32 shell32 user32 advapi32) + sd_l10n sd_l10n_catalog_svc sd_ssh sd_platform sd_base ole32 shell32 user32 advapi32 mpr) set_target_properties(sd-test-host-fake PROPERTIES FOLDER "tests") add_executable(sd-test-cli "${_cli_dir}/main.c" ${SD_CLI_SOURCES}) diff --git a/tests/unit/sd_mount/fake_backend.c b/tests/unit/sd_mount/fake_backend.c index b51641a..a51825d 100644 --- a/tests/unit/sd_mount/fake_backend.c +++ b/tests/unit/sd_mount/fake_backend.c @@ -1108,6 +1108,20 @@ static uint16_t f_webdav_port(const sd_webdav_mount *m) return 49321; } +/* 02 §7.7 attach: a well-formed remote name so that the service's reply can be checked without a server. */ +static sd_err f_webdav_remote(const sd_webdav_mount *m, BOOL xp_form, wchar_t *out, size_t cap) +{ + SD_UNUSED(m); + if (out == NULL || cap == 0) + return SD_E_INVALID_PARAM; + if (xp_form) + wcscpy_s(out, cap, L"http://localhost:49321/s-aaaaaaaaaaaaaaaaaaaaaaaaaa"); + else + + wcscpy_s(out, cap, L"\\\\localhost@49321\\DavWWWRoot\\s-aaaaaaaaaaaaaaaaaaaaaaaaaa"); + return SD_OK; +} + void fake_backend_set_wb_dirty(UINT64 bytes) { InterlockedExchange(&g_wb_dirty, (LONG)bytes); } uint32_t fake_backend_refresh_calls(void) { return (uint32_t)g_refresh_calls; } uint32_t fake_backend_cache_clears(int *disk_too) { if (disk_too) *disk_too = (int)g_clear_disk; return (uint32_t)g_cache_clears; } @@ -1123,7 +1137,7 @@ static sd_err f_session_create_wrap(const sd_ssh_config *ssh, const sd_ssh_callb return e; } -static const sd_mount_backend k_fake = { +static sd_mount_backend k_fake = { "fake", f_global_init, f_kh_open, @@ -1160,11 +1174,14 @@ static const sd_mount_backend k_fake = { f_vfs_cache_clear, f_wb_bytes, f_webdav_port, + /* hk_conn_create .. hk_conn_destroy and webdav_remote: zero here, set by the hostkey table and by the + initializer below (positional C initialization cannot skip, so the tail is filled after k_fake). */ }; const sd_mount_backend *fake_backend(void) { ensure(); + k_fake.webdav_remote = f_webdav_remote; /* the tail of the table, after the positional initializer */ return &k_fake; } @@ -1394,6 +1411,7 @@ static void fk_destroy(sd_ssh_conn *cc) const sd_mount_backend *fake_backend_hostkeys(void) { ensure(); + k_fake.webdav_remote = f_webdav_remote; /* the table is copied below; the tail is set here */ if (!k_fake_hk_ready) { k_fake_hk_table = k_fake; k_fake_hk_table.prompt_answer_hostkey = fk_prompt_answer_hostkey; diff --git a/tests/unit/sd_svc/main.c b/tests/unit/sd_svc/main.c index 66d63ee..6f86c8f 100644 --- a/tests/unit/sd_svc/main.c +++ b/tests/unit/sd_svc/main.c @@ -37,6 +37,8 @@ SD_TEST_DECL(svc_system_proxy_refuses_basic_auth_on_its_leg); SD_TEST_DECL(svc_system_proxy_policy_is_read_from_the_machine_values); SD_TEST_DECL(svc_secret_proxy_set_and_delete_through_the_pipe); SD_TEST_DECL(svc_hostkey_add_through_an_unknown_jump_host_is_refused); +SD_TEST_DECL(svc_webdav_mount_reports_its_attach_point); +SD_TEST_DECL(svc_attach_info_refuses_a_caller_outside_service_access); SD_TEST_DECL(svc_one_profile_mounted_twice_keeps_both_announcements); SD_TEST_DECL(svc_core_stops_and_leaves_nothing_behind); SD_TEST_DECL(refuse_log_is_once_per_sid_per_window); @@ -134,6 +136,8 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(svc_system_proxy_policy_is_read_from_the_machine_values), SD_TEST_ENTRY(svc_secret_proxy_set_and_delete_through_the_pipe), SD_TEST_ENTRY(svc_hostkey_add_through_an_unknown_jump_host_is_refused), + SD_TEST_ENTRY(svc_webdav_mount_reports_its_attach_point), + SD_TEST_ENTRY(svc_attach_info_refuses_a_caller_outside_service_access), SD_TEST_ENTRY(svc_one_profile_mounted_twice_keeps_both_announcements), SD_TEST_ENTRY(svc_core_stops_and_leaves_nothing_behind), SD_TEST_ENTRY(refuse_log_is_once_per_sid_per_window), diff --git a/tests/unit/sd_svc/test_access.c b/tests/unit/sd_svc/test_access.c index ce6131b..304caa3 100644 --- a/tests/unit/sd_svc/test_access.c +++ b/tests/unit/sd_svc/test_access.c @@ -160,10 +160,9 @@ SD_TEST(validate_profile_refusals) p.jump[0] = 0; p.proxy[0].kind = SD_PROFILE_PROXY_NONE; - /* not in this build: global WebDAV (D11) */ + /* M5 (02 §7.7): a global WebDAV profile loads; the mechanism (attach in this build) is chosen at mount time */ p.backend = SD_PROFILE_BACKEND_WEBDAV; - SD_CHECK_EQ_INT(svc_validate_profile(&p, &key, &why), SD_E_INVALID_PARAM); - SD_CHECK_STR(key, "mount.backend"); + SD_CHECK_EQ_INT(svc_validate_profile(&p, &key, &why), SD_OK); p.backend = SD_PROFILE_BACKEND_AUTO; /* D35: no service_access, no drive */ diff --git a/tests/unit/sd_svc/test_service.c b/tests/unit/sd_svc/test_service.c index 716c020..0c5108c 100644 --- a/tests/unit/sd_svc/test_service.c +++ b/tests/unit/sd_svc/test_service.c @@ -86,6 +86,8 @@ static int make_chain_profiles(void); #define T_ID_SYSPROXY "00000000000000000000000000000016" #define T_ID_SYSBASIC "00000000000000000000000000000017" #define T_ID_HKCHAIN "00000000000000000000000000000018" +#define T_ID_WEBDAV "00000000000000000000000000000019" +#define T_ID_WEBDAVDENY "0000000000000000000000000000001a" /* proxy = system seam (svc_opts.sysproxy_ops): a stub machine configuration; the test changes allow to refuse by policy */ static svc_sysproxy_ops g_sysops; @@ -307,6 +309,20 @@ static int make_chain_profiles(void) identity_of(&p); strcpy_s(p.jump, sizeof(p.jump), "hkjump.untrusted.test"); if (!write_profile(L"t-hkchain.ini", &p)) return 0; + + /* 02 §7.7 attach: a global WebDAV profile (hosted by the service, mapped per user) and one whose service_access + names nobody this test can be (the attach reply must refuse it) */ + p = base_profile(T_ID_WEBDAV, "webdav", "webdav.example"); + identity_of(&p); + p.backend = SD_PROFILE_BACKEND_WEBDAV; + if (!write_profile(L"u-webdav.ini", &p)) return 0; + fake_backend_pre_trust("webdav.example"); + + p = base_profile(T_ID_WEBDAVDENY, "webdavdeny", "webdav.example"); + identity_of(&p); + p.backend = SD_PROFILE_BACKEND_WEBDAV; + strcpy_s(p.service_access, sizeof(p.service_access), "S-1-5-21-1-2-3-4"); /* valid SID, matches no token */ + if (!write_profile(L"v-webdavdeny.ini", &p)) return 0; return 1; } @@ -559,16 +575,16 @@ SD_TEST(svc_loads_only_valid_service_profiles) const sd_json *loaded; SD_REQUIRE(ensure_core()); SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); - /* 18 valid: mounted, nosub, tofu, pwprompt, adminonly, the M5 hostkey and rotate profiles and the M5 chain and proxy - profiles (a chain is checked per hop at mount time, not at load). Refused: pageant (D17). Not loaded: the user - profile. */ + /* 20 valid: mounted, nosub, tofu, pwprompt, adminonly, the M5 hostkey and rotate profiles, the M5 chain and proxy + profiles and the two global WebDAV profiles (a chain is checked per hop at mount time, not at load). Refused: + pageant (D17). Not loaded: the user profile. */ res = call(c, "config.reload", NULL, &err, &e); SD_REQUIRE(res != NULL); loaded = sd_json_get(res, "profiles"); { int64_t n = -1; SD_CHECK(sd_json_get_int64(loaded, "loaded", &n)); - SD_CHECK_EQ_INT(n, 18); + SD_CHECK_EQ_INT(n, 20); } sd_json_free(res); sd_ipc_close(c); @@ -1474,7 +1490,7 @@ SD_TEST(svc_status_method_reports_the_scm_the_pipe_and_the_machine_folder) SD_CHECK(sd_json_get_int64(o, "prompts", &n) && n == 0); o = sd_json_get(res, "machine"); SD_CHECK_STR(sd_json_get_str(o, "state"), "ok"); - SD_CHECK(sd_json_get_int64(o, "profiles", &n) && n == 18); /* the fixture's valid profiles (svc_loads_only_valid_service_profiles) */ + SD_CHECK(sd_json_get_int64(o, "profiles", &n) && n == 20); /* the fixture's valid profiles (svc_loads_only_valid_service_profiles) */ SD_CHECK(sd_json_get_str(o, "reason") == NULL); sd_json_free(res); sd_ipc_close(c); @@ -1533,7 +1549,7 @@ SD_TEST(svc_repair_is_busy_while_a_global_drive_exists) SD_REQUIRE(res != NULL); o = sd_json_get(res, "machine"); SD_CHECK_STR(sd_json_get_str(o, "state"), "ok"); - SD_CHECK(sd_json_get_int64(o, "profiles", &n) && n == 18); /* the fixture's valid profiles (svc_loads_only_valid_service_profiles) */ + SD_CHECK(sd_json_get_int64(o, "profiles", &n) && n == 20); /* the fixture's valid profiles (svc_loads_only_valid_service_profiles) */ sd_json_free(res); SD_CHECK(unmount_id(c, mid)); SD_CHECK(wait_state(c, mid, "gone", 10000, NULL)); @@ -1810,6 +1826,72 @@ SD_TEST(svc_hostkey_add_through_an_unknown_jump_host_is_refused) sd_ipc_close(c); } +/* 02 §7.7 attach (G2/G7 shape): a global WebDAV profile is hosted by the service and reports the export a member's + agent maps. The reply carries the UNC with the path token and the letter, and no credential. The mount is hosted, + not mapped, so it needs no second session. */ +SD_TEST(svc_webdav_mount_reports_its_attach_point) +{ + sd_ipc_conn *c = NULL; + sd_ipc_error err; + sd_err e; + sd_json *res, *st = NULL; + char mid[16] = ""; + int attach = 0; + int64_t port = 0; + SD_REQUIRE(ensure_core()); + SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); + fake_backend_set_webclient(SD_WD_CLIENT_RUNNING, 1); + SD_REQUIRE(mount_and_wait(c, "webdav", "mounted", mid, sizeof(mid), &st)); + json_drop(&st); + res = call(c, "mount.attach_info", "{\"profile\":\"webdav\"}", &err, &e); + SD_REQUIRE(res != NULL); + SD_CHECK_STR(sd_json_get_str(res, "backend"), "webdav"); + SD_CHECK(sd_json_get_bool(res, "attach", &attach) && attach); + SD_CHECK_STR(sd_json_get_str(res, "unc"), "\\\\localhost@49321\\DavWWWRoot\\s-aaaaaaaaaaaaaaaaaaaaaaaaaa"); + SD_CHECK_STR(sd_json_get_str(res, "xp_url"), "http://localhost:49321/s-aaaaaaaaaaaaaaaaaaaaaaaaaa"); + SD_CHECK(sd_json_get_int64(res, "port", &port) && port == 49321); + sd_json_free(res); + /* a Dokany global mount has no export: the same method reports nothing to attach */ + SD_CHECK(unmount_id(c, mid)); + SD_CHECK(wait_state(c, mid, "gone", 10000, NULL)); + SD_REQUIRE(mount_and_wait(c, "mounted", "mounted", mid, sizeof(mid), &st)); + json_drop(&st); + res = call(c, "mount.attach_info", "{\"profile\":\"mounted\"}", &err, &e); + SD_REQUIRE(res != NULL); + SD_CHECK_STR(sd_json_get_str(res, "backend"), "dokany"); + attach = 1; + SD_CHECK(sd_json_get_bool(res, "attach", &attach) && attach == 0); + SD_CHECK(sd_json_get(res, "unc") == NULL); + sd_json_free(res); + SD_CHECK(unmount_id(c, mid)); + SD_CHECK(wait_state(c, mid, "gone", 10000, NULL)); + sd_ipc_close(c); +} + +/* 02 §7.7 G7 / TM-R18: the attach information is only for a caller in service_access. A profile whose list names a + SID this process does not carry answers access_denied, and no UNC leaves the service. */ +SD_TEST(svc_attach_info_refuses_a_caller_outside_service_access) +{ + sd_ipc_conn *c = NULL; + sd_ipc_error err; + sd_err e; + sd_json *res, *st = NULL; + char mid[16] = ""; + SD_REQUIRE(ensure_core()); + SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); + fake_backend_set_webclient(SD_WD_CLIENT_RUNNING, 1); + SD_REQUIRE(mount_and_wait(c, "webdavdeny", "mounted", mid, sizeof(mid), &st)); + json_drop(&st); + res = call(c, "mount.attach_info", "{\"profile\":\"webdavdeny\"}", &err, &e); + SD_CHECK(res == NULL); + if (res) + sd_json_free(res); + SD_CHECK_STR(err.code, "access_denied"); + SD_CHECK(unmount_id(c, mid)); + SD_CHECK(wait_state(c, mid, "gone", 10000, NULL)); + sd_ipc_close(c); +} + /* Regression: one profile mounted twice in one process, each mount raising its own UpdateHostKeys announcement (the mount slot, the announcement records and the status replies of the first mount must not leak into the second). */ SD_TEST(svc_one_profile_mounted_twice_keeps_both_announcements) diff --git a/tests/unit/sd_webdav/main.c b/tests/unit/sd_webdav/main.c index d07c0c9..d844d75 100644 --- a/tests/unit/sd_webdav/main.c +++ b/tests/unit/sd_webdav/main.c @@ -14,6 +14,8 @@ SD_TEST_DECL(webdav_values); SD_TEST_DECL(webdav_server_gate_before_auth); SD_TEST_DECL(webdav_server_auth_and_methods); SD_TEST_DECL(webdav_server_rejects_other_sid); +SD_TEST_DECL(webdav_service_server_admits_a_member); +SD_TEST_DECL(webdav_service_server_refuses_a_non_member); SD_TEST_DECL(webdav_server_read_only_export); SD_TEST_DECL(webdav_server_slowloris_and_quotas); SD_TEST_DECL(webdav_token_only_decision_matrix); @@ -42,6 +44,8 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(webdav_server_gate_before_auth), SD_TEST_ENTRY(webdav_server_auth_and_methods), SD_TEST_ENTRY(webdav_server_rejects_other_sid), + SD_TEST_ENTRY(webdav_service_server_admits_a_member), + SD_TEST_ENTRY(webdav_service_server_refuses_a_non_member), SD_TEST_ENTRY(webdav_server_read_only_export), SD_TEST_ENTRY(webdav_server_slowloris_and_quotas), SD_TEST_ENTRY(webdav_server_ipv6_loopback), diff --git a/tests/unit/sd_webdav/test_server.c b/tests/unit/sd_webdav/test_server.c index 4a15b03..6a466c2 100644 --- a/tests/unit/sd_webdav/test_server.c +++ b/tests/unit/sd_webdav/test_server.c @@ -441,6 +441,98 @@ SD_TEST(webdav_server_rejects_other_sid) fx_down(&f); } +/* 02 §7.7: a service (global) server. The vfs is global with access_sid as the only service_access entry, the + server's owner is SYSTEM and service_mode is on, so the connection-level owner check is off and sd_vfs decides. */ +static int fx_up_service(fixture *f, PSID access_sid) +{ + sd_vfs_options o; + sd_webdav_server_cfg sc; + sd_webdav_export_cfg ec; + static uint8_t system_sid[SECURITY_MAX_SID_SIZE]; + DWORD sl = sizeof(system_sid); + PSID acc[1]; + memset(f, 0, sizeof(*f)); + if (!CreateWellKnownSid(WinLocalSystemSid, NULL, system_sid, &sl)) + return -1; + f->s = fake_session_create(NULL); + fake_put(f->s, "/home/user/hello.txt", "hello world", 11, 0644); + memset(&o, 0, sizeof(o)); + o.struct_size = sizeof(o); + o.request_timeout_ms = 50000; + o.volume_label = L"SD Global"; + o.global_mount = TRUE; + acc[0] = access_sid; + o.access_sids = acc; + o.n_access = 1; + if (sd_vfs_mount(f->s, &o, NULL, NULL, &f->v) != SD_VFS_OK) + return -1; + memset(&sc, 0, sizeof(sc)); + sc.struct_size = sizeof(sc); + sc.owner_sid = (PSID)system_sid; + sc.service_mode = TRUE; + if (sd_webdav_server_start(&sc, &f->srv) != SD_OK) + return -1; + f->port = sd_webdav_server_port(f->srv); + memset(&ec, 0, sizeof(ec)); + ec.struct_size = sizeof(ec); + ec.label = L"Global"; + if (sd_webdav_export_add(f->srv, f->v, &ec, &f->ex) != SD_OK) + return -1; + strcpy_s(f->prefix, sizeof(f->prefix), sd_webdav_export_prefix(f->ex)); + return 0; +} + +/* The token user of this process, in a static buffer. */ +static PSID self_sid(void) +{ + static uint8_t buf[256]; + HANDLE tok = NULL; + DWORD n = 0; + PSID s = NULL; + if (OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &tok)) { + if (GetTokenInformation(tok, TokenUser, buf, sizeof(buf), &n)) + s = ((TOKEN_USER *)buf)->User.Sid; + CloseHandle(tok); + } + return s; +} + +/* 02 §7.7, G7 shape: a member of service_access reaches the export of a service server whose owner is SYSTEM (the + per-user server answers 403 here, webdav_server_rejects_other_sid). */ +SD_TEST(webdav_service_server_admits_a_member) +{ + fixture f; + tc_conn *c; + PSID me = self_sid(); + SD_REQUIRE(me != NULL); + SD_REQUIRE(fx_up_service(&f, me) == 0); + c = conn_new(f.port); + SD_REQUIRE(c != NULL); + SD_CHECK_EQ_INT(tc_authenticate(c, f.port, f.prefix), 207); + conn_free(c); + fx_down(&f); +} + +/* 02 §7.7, G7: the same server, a service_access list that names someone else. Authentication succeeds (the server + no longer checks an owner), and sd_vfs refuses the request: no listing reaches a non-member. */ +SD_TEST(webdav_service_server_refuses_a_non_member) +{ + fixture f; + tc_conn *c; + uint8_t other[SECURITY_MAX_SID_SIZE]; + DWORD sl = sizeof(other); + int st; + SD_REQUIRE(CreateWellKnownSid(WinLocalServiceSid, NULL, other, &sl)); + SD_REQUIRE(fx_up_service(&f, (PSID)other) == 0); + c = conn_new(f.port); + SD_REQUIRE(c != NULL); + st = tc_authenticate(c, f.port, f.prefix); + SD_CHECK(st == 403 || st == 404); + SD_CHECK(st != 207); + conn_free(c); + fx_down(&f); +} + SD_TEST(webdav_server_read_only_export) { fixture f; From 58d6b68fc0fadb05662da5d038c06bbb7ba15031 Mon Sep 17 00:00:00 2001 From: FacuM Date: Fri, 30 Oct 2026 23:40:00 -0300 Subject: [PATCH 2/9] service: open the service with SERVICE_START to set its restart actions ChangeServiceConfig2 with SERVICE_CONFIG_FAILURE_ACTIONS needs SERVICE_START on the handle when an action is SC_ACTION_RESTART. scm_set_failure_actions opened the service with SERVICE_CHANGE_CONFIG only, so every install failed with ERROR_ACCESS_DENIED after CreateService, rolled the service back, and the CLI reported it as "needs an elevated console" (exit 9), even as SYSTEM. Found by the live global mount run on the Win10 lab VM: with this change the service installs, starts and mounts the global drive. --- apps/shelldrive-host/svc_scm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/shelldrive-host/svc_scm.c b/apps/shelldrive-host/svc_scm.c index 31f1391..284221c 100644 --- a/apps/shelldrive-host/svc_scm.c +++ b/apps/shelldrive-host/svc_scm.c @@ -231,7 +231,9 @@ static DWORD scm_set_failure_actions(void *ctx, const svc_install_spec *s) fa.dwResetPeriod = s->reset_period_s; fa.cActions = SVC_SCM_RESTART_COUNT; fa.lpsaActions = acts; - e = open_service(SC_MANAGER_CONNECT, SERVICE_CHANGE_CONFIG, &scm, &svc); + /* SC_ACTION_RESTART needs SERVICE_START on the handle as well, or ChangeServiceConfig2 fails with + ERROR_ACCESS_DENIED even for SYSTEM (and the install rolls back) */ + e = open_service(SC_MANAGER_CONNECT, SERVICE_CHANGE_CONFIG | SERVICE_START, &scm, &svc); if (e != 0) return e; if (!ChangeServiceConfig2W(svc, SERVICE_CONFIG_FAILURE_ACTIONS, &fa)) From f853144fb5b36ee735d608cc8fb416ba5c259efc Mon Sep 17 00:00:00 2001 From: FacuM Date: Sat, 31 Oct 2026 00:03:25 -0300 Subject: [PATCH 3/9] docs: record the M5 live global mount run --- docs/dev/lab-results.md | 55 +++++++++++++++++++++++++++++++++++++---- docs/dev/m5-notes.md | 27 ++++++++++++++++++-- 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/docs/dev/lab-results.md b/docs/dev/lab-results.md index 17a2de8..8748fb5 100644 --- a/docs/dev/lab-results.md +++ b/docs/dev/lab-results.md @@ -319,11 +319,56 @@ Notes for the record: - New-LabVM.ps1 still does not set AutomaticStartAction; it was set manually on the recreated VM. -## Appendix: procedure for the live global Dokany mount (M5, not yet run) - -Written 2026-10-30 because `sd-win10-ltsc21-x64` was held by the M3 acceptance run. Nothing here has been executed; -it is the exact sequence to run as soon as the VM is free, so that the first live global mount is a measurement and -not an exploration. It closes the "a real global mount seen by a second session" item of `docs/dev/m5-notes.md` +## 2026-10-30: first live run — M5 global Dokany mount on Win10 LTSC (attempt 13) + +Attempt 13, log `D:\sdlab\logs\m5-20261030-233947.log` (2026-10-30, 23:39 local). Guest `sd-win10-ltsc21-x64` +(Windows 10 IoT Enterprise LTSC 2021, build 19044, generation 1), now on the NVMe SSD (`C:\sdlab-vms`) — boots +in about 1 min instead of about 8. Payload: x64-modern Release built from this branch with the svc_scm fix +`58d6b68` (the banner reports b413bf0), Dokany 2.1 (`dokan2` RUNNING). Machine profile `global10.ini`: +`scope = service`, `backend = dokany`, `letter = Z`, `label = GlobalDokan`, `autostart = boot`, +`service_access = sduser1, sduser2`, `service_access_write = sduser1`; fixture sshd at `[172.20.128.1]:2222`. +The harness lives in `D:\sdlab\m5` (host-only, not in the repo). Phases: A (install, batch logons), B1/B2 +(reboot + console logon, B2 with a concurrent batch session), final (G7 mutation, G3, uninstall). Results per +acceptance item, step names as the run log prints them: + +- Service install/start as SYSTEM: OK — `service-install-start OK ShellDriveSvc RUNNING`. Before `58d6b68` (the + service is opened with SERVICE_START so its restart actions can be set) every install rolled back with + ACCESS_DENIED surfaced as "needs an elevated console", exit 9. +- Machine profile load and machine host key add (`--from-server --expect`): OK — the service log shows + `service starting; 1 machine profiles`; `hostkey-add OK [172.20.128.1]:2222 ssh-ed25519 …`. +- Global Dokany mount Z: — `service status` (admin) answers `Z: GlobalDokan mounted`; Event 1100 `Global drive + for profile 9e1a0c5d3b7f4e2a8c6d1f0b2a4c6e81 is mounted at Z:.`. +- G1, batch logon of `sduser1` (standard, not elevated): `dir Z:` rc=0 with no mount command run ("Volume in + drive Z is GlobalDokan"); write, rename and delete all rc=0; one Event 1200 audit entry per change carrying + the user SID: `… by SD-10-X64\sduser1 (SID S-1-5-21-159632063-432193879-889666275-1002): write_open \big.bin.`. +- G5, 1 GiB through Z:: write 74.6 s, read 17.6 s, `hash_match=True` (attempt 13's own values; attempt 12, + `m5-20261030-230247.log`, measured 79.9 s / 17.7 s). +- G7, read-only member: `sduser2`'s write is denied (`write_rc=1 (want nonzero) [cmd : Access is denied. …]`) and + it cannot read `sduser1`'s file. G7 mutation: profile edited to drop `sduser2`, service restart to reload — + `g7-u2-denied OK` (`dir Z:` now `Access is denied.`) and `g7-u1-still-ok OK`; profile restored, remount rc=0. +- Console logons: B1 — the `sduser1` console session sees Z: (`dirZ_rc=0`). B2 — the `sduser2` console session + plus a concurrent `sduser1` batch session (two live sessions) both see and use Z:, 1 GiB round trip OK + (write 69.7 s, read 18 s, hash match). +- G3: service stop removes Z: cleanly (`dirZ-after-stop=[The system cannot find the path specified.]`), start + + remount OK; VM save/resume (sleep proxy) keeps the mount. +- Uninstall: `uninstall OK … ShellDrive service removed.`; `sc query` answers 1060 (no such service) and Z: is + gone (`dirZ-after-uninstall=[The system cannot find the path specified.]`). +- Expected by design: SYSTEM itself gets Access denied on Z: (it is not in `service_access`) — the run log marks + this probe `service-mount FAIL Z: not readable: Access is denied.`. + +Defects and gaps found by the run are listed under the M5 "Left" items of `docs/dev/m5-notes.md` (Part 14 §5): +the service does not apply `autostart = boot`, a standard user's `shelldrive service list` exits 8 while the +service runs, `service mount` for an unknown profile prints an empty name placeholder, `scm_error` maps every +ACCESS_DENIED to "needs an elevated console", and Get-Volume does not list the Dokany Z: (`dir`/Explorer work). +Not covered: RDP/FUS second interactive session (G2 proper), Server 2022 + Secure Boot (the Gen2 media problem +of the 2026-10-29 night section), pre-logon mount at boot (blocked by the `autostart` gap). + +## Appendix: procedure for the live global Dokany mount (M5; first run 2026-10-30, see above) + +Written 2026-10-30 while `sd-win10-ltsc21-x64` was still held by the M3 acceptance run; executed the same day +(attempt 13, results in the 2026-10-30 section above, which supersedes this appendix wherever they differ — +step 8's pre-logon mount is exactly the `autostart = boot` gap recorded there). The sequence is kept as written +for future runs. It closes the "a real global mount seen by a second session" item of `docs/dev/m5-notes.md` (M5 rows 2 and "Left") and the M5 demo (a) of `docs/design/00-integration-review.md` §6 for Windows 10 (Server 2022 needs a Gen2 guest, still blocked by the host DVD defect of this file's 2026-10-29 night section; Gen1 validates the code but not Secure Boot). diff --git a/docs/dev/m5-notes.md b/docs/dev/m5-notes.md index dacf065..b24f56f 100644 --- a/docs/dev/m5-notes.md +++ b/docs/dev/m5-notes.md @@ -1329,8 +1329,31 @@ And in `unit.webdav`, against a real loopback server with real SSPI (the test us ### 5. Left after this round -- The live global Dokany mount on `sd-win10-ltsc21-x64` (the VM is `m3-lab`'s); the procedure is in - `docs/dev/lab-results.md` (appendix) so it can run as soon as the VM is free. +- The live global Dokany mount on `sd-win10-ltsc21-x64` ran on 2026-10-30, attempt 13, log + `D:\sdlab\logs\m5-20261030-233947.log` (the procedure is the appendix of `docs/dev/lab-results.md`): PASS on + install/start as SYSTEM (after `58d6b68`), machine profile load and host key, the global mount (Event 1100), + G1 batch and B1/B2 console logons (two live sessions), G5 (1 GiB write 74.6 s, read 17.6 s, SHA-256 match), + G7 including the drop-member mutation, G3 stop/start and VM save/resume, and uninstall. Per-item results with + the quoted log lines are in the 2026-10-30 section of `docs/dev/lab-results.md`. The run left these product + gaps, to triage: + - `autostart = boot` is not applied by the service. After every reboot `shelldrive service list` was empty + until a manual `service mount` (03 §3.4.1 says the service applies it); both phase-B reboots log + `pre-logon-state OK … list=[No global drives are mounted.]` and `remount OK remounted by hand (boot + autostart absent)`. Product gap; it blocks the pre-logon (G6) claim for Dokany. + - A standard user's `shelldrive service list` exits 8, "The ShellDrive service is not running or not + installed.", while the service runs: the same user gets rc=0 and `Z: GlobalDokan mounted` from its console + session but rc=8 from a batch session. Pipe access for non-admin members, or the CLI mis-mapping access + denied — triage. + - `service mount ` for an unknown profile prints "There is no profile named ." — the name placeholder + is empty. + - `scm_error` (`apps/shelldrive/cli_service.c:280`) maps every ERROR_ACCESS_DENIED to "needs an elevated + console" — misleading when the caller is elevated; that message is what the pre-`58d6b68` install attempts + reported as they rolled back (exit 9). + - Note only: Get-Volume (MSFT_Volume) does not list the Dokany Z: ("No MSFT_Volume objects found with + property 'DriveLetter' equal to 'Z'."); `dir` and Explorer work. + - Not covered by the run: RDP/FUS second interactive session (G2 proper), Server 2022 + Secure Boot (the Gen2 + media problem of lab-results' 2026-10-29 night section), pre-logon mount at boot (blocked by the + `autostart` gap above). - Everything in 05 §12.8 that has no code: release roots, crash dumps, Event 3100, the FUS/RDP session list, `--elevated-view`, `settings.ini` checks, discovered-proxy detection. - The per-user identity test (two users, each mapping the other's UNC) and the rest of G1-G7. From 5780848aff7f2d0401f20032f0057f7fec8ca564 Mon Sep 17 00:00:00 2001 From: FacuM Date: Sat, 31 Oct 2026 00:49:46 -0300 Subject: [PATCH 4/9] service: mount autostart=boot profiles when the service starts Machine profiles of scope service with [mount] autostart = boot were never mounted by the service (03 3.4.1): the live lab run found every reboot with an empty service list until a manual service mount. The body of the mount handler becomes svc_mount_queue, shared by the IPC "mount" request and the new svc_autostart_boot_mounts, which runs after the profiles are loaded at start, on config reload and after a data-folder repair. The SCM start is not blocked: the mounts go through the control thread's queue with sd_mount's reconnect backoff, so a network that is not up yet only delays them. An already mounted profile is left alone, so a reload never double-mounts. --- apps/shelldrive-host/service.c | 119 +++++++++++---- tests/unit/sd_svc/main.c | 4 + tests/unit/sd_svc/test_service.c | 239 +++++++++++++++++++++++++++++++ 3 files changed, 335 insertions(+), 27 deletions(-) diff --git a/apps/shelldrive-host/service.c b/apps/shelldrive-host/service.c index b9f670c..c5a7e44 100644 --- a/apps/shelldrive-host/service.c +++ b/apps/shelldrive-host/service.c @@ -2513,44 +2513,47 @@ static sd_err handle_status(sd_ipc_client *c, const sd_json *params, sd_json_wri return SD_OK; } -static sd_err handle_mount(sd_ipc_client *c, const sd_json *params, sd_json_writer *res, sd_ipc_error *err) +/* The one code path that brings a global mount up (2.4 "mount"; 1.8 and 3.4.1 boot autostart): finds the loaded + service profile `key`, refuses a duplicate, picks the drive letter and publishes the record, then queues the + start job for the control thread. Takes g.lock. Returns SD_OK and fills out_id and out_target; on failure + returns the sd_err of the reply, with *code set (NULL for the plain errors) and detail filled. */ +static sd_err svc_mount_queue(const char *key, const char *tgt, char out_id[16], char out_target[4], + const char **code, char detail[200]) { - const char *key = sd_json_get_str(params, "profile"); - const char *tgt = sd_json_get_str(params, "target"); svc_prof *src; svc_prof *copy = NULL; svc_mount *r = NULL; - sd_err e; wchar_t letter = 0; svc_job *job; uint32_t i, slot = SVC_MAX_MOUNTS; const char *bad = NULL, *why = NULL; - char detail[200]; - e = need_admin(c, err); - if (e != SD_OK) - return e; - if (key == NULL) - return fail(err, SD_E_INVALID_PARAM, "usage", "mount needs a profile"); sd_mutex_lock(&g.lock); src = find_profile_locked(key); if (src == NULL) { sd_mutex_unlock(&g.lock); - return fail(err, SD_E_NOT_FOUND, "profile_not_found", "no machine profile of scope service with that id or name"); + *code = "profile_not_found"; + strcpy_s(detail, 200, "no machine profile of scope service with that id or name"); + return SD_E_NOT_FOUND; } if (svc_validate_profile(&src->p, &bad, &why) != SD_OK) { sd_mutex_unlock(&g.lock); - _snprintf_s(detail, sizeof(detail), _TRUNCATE, "%s: %s", bad ? bad : "profile", why ? why : "refused"); - return fail(err, SD_E_INVALID_PARAM, "usage", detail); + *code = "usage"; + _snprintf_s(detail, 200, _TRUNCATE, "%s: %s", bad ? bad : "profile", why ? why : "refused"); + return SD_E_INVALID_PARAM; } for (i = 0; i < SVC_MAX_MOUNTS; i++) if (g.mounts[i] && strcmp(g.mounts[i]->prof->p.id, src->p.id) == 0) { sd_mutex_unlock(&g.lock); - return fail(err, SD_E_EXISTS, "already_mounted", "this profile is already mounted"); + *code = "already_mounted"; + strcpy_s(detail, 200, "this profile is already mounted"); + return SD_E_EXISTS; } copy = (svc_prof *)sd_alloc(sizeof(*copy)); if (copy == NULL) { sd_mutex_unlock(&g.lock); - return fail(err, SD_E_NO_MEMORY, NULL, "out of memory"); + *code = NULL; + strcpy_s(detail, 200, "out of memory"); + return SD_E_NO_MEMORY; } memcpy(copy, src, sizeof(*copy)); /* the requested target wins over the profile's; "auto" (or nothing) picks a free letter (3.4.1 mount.target) */ @@ -2563,7 +2566,9 @@ static sd_err handle_mount(sd_ipc_client *c, const sd_json *params, sd_json_writ if (letter == 0) { sd_mutex_unlock(&g.lock); sd_free(copy); - return fail(err, SD_E_INVALID_PARAM, "invalid_target", "the target is not a drive letter"); + *code = "invalid_target"; + strcpy_s(detail, 200, "the target is not a drive letter"); + return SD_E_INVALID_PARAM; } } } @@ -2572,7 +2577,9 @@ static sd_err handle_mount(sd_ipc_client *c, const sd_json *params, sd_json_writ if (letter == 0 || letter_taken_locked(letter) || (GetLogicalDrives() & (1ul << (letter - L'A')))) { sd_mutex_unlock(&g.lock); sd_free(copy); - return fail(err, SD_E_EXISTS, "target_in_use", "no free drive letter for this mount"); + *code = "target_in_use"; + strcpy_s(detail, 200, "no free drive letter for this mount"); + return SD_E_EXISTS; } for (i = 0; i < SVC_MAX_MOUNTS; i++) if (g.mounts[i] == NULL) { @@ -2582,13 +2589,17 @@ static sd_err handle_mount(sd_ipc_client *c, const sd_json *params, sd_json_writ if (slot == SVC_MAX_MOUNTS) { sd_mutex_unlock(&g.lock); sd_free(copy); - return fail(err, SD_E_BUSY, "busy", "too many global mounts"); + *code = "busy"; + strcpy_s(detail, 200, "too many global mounts"); + return SD_E_BUSY; } r = (svc_mount *)sd_calloc(1, sizeof(*r)); if (r == NULL) { sd_mutex_unlock(&g.lock); sd_free(copy); - return fail(err, SD_E_NO_MEMORY, NULL, "out of memory"); + *code = NULL; + strcpy_s(detail, 200, "out of memory"); + return SD_E_NO_MEMORY; } g.next_mount++; _snprintf_s(r->id, sizeof(r->id), _TRUNCATE, "g-%u", (unsigned)g.next_mount); @@ -2601,23 +2612,73 @@ static sd_err handle_mount(sd_ipc_client *c, const sd_json *params, sd_json_writ sd_mutex_unlock(&g.lock); sd_free(r->prof); sd_free(r); - return fail(err, SD_E_NO_MEMORY, NULL, "out of memory"); + *code = NULL; + strcpy_s(detail, 200, "out of memory"); + return SD_E_NO_MEMORY; } strcpy_s(job->id, sizeof(job->id), r->id); + strcpy_s(out_id, 16, r->id); + target_of(letter, out_target); g.mounts[slot] = r; sd_mutex_unlock(&g.lock); SD_LOGI("svc", "mount %s queued for %c:", r->id, (char)letter); svc_enqueue(job_start, job); - sd_jw_key(res, "mount_id"); sd_jw_cstr(res, r->id); - { - char tgt2[4]; - target_of(letter, tgt2); - sd_jw_key(res, "target"); sd_jw_cstr(res, tgt2); - } + return SD_OK; +} + +static sd_err handle_mount(sd_ipc_client *c, const sd_json *params, sd_json_writer *res, sd_ipc_error *err) +{ + const char *key = sd_json_get_str(params, "profile"); + const char *tgt = sd_json_get_str(params, "target"); + sd_err e; + char id[16], target[4]; + const char *code = NULL; + char detail[200]; + e = need_admin(c, err); + if (e != SD_OK) + return e; + if (key == NULL) + return fail(err, SD_E_INVALID_PARAM, "usage", "mount needs a profile"); + e = svc_mount_queue(key, tgt, id, target, &code, detail); + if (e != SD_OK) + return fail(err, e, code, detail); + sd_jw_key(res, "mount_id"); sd_jw_cstr(res, id); + sd_jw_key(res, "target"); sd_jw_cstr(res, target); sd_jw_key(res, "state"); sd_jw_cstr(res, "connecting"); return SD_OK; } +/* 1.8 Boot: brings up every loaded machine profile whose [mount] autostart is boot (3.4.1) when the profiles + were loaded or reloaded. The mounts go through the same queue as a "mount" request, so the SCM start is not + blocked: RUNNING is reported first and the control thread starts each mount afterwards, each with its own + sd_mount reconnect backoff — a network that is not up yet only delays the mount. A profile that is already + mounted is left alone (a reload never double-mounts); every auto-mounted profile is logged at INFO. */ +static void svc_autostart_boot_mounts(void) +{ + char ids[SVC_MAX_PROFILES][33], names[SVC_MAX_PROFILES][128]; + uint32_t n = 0, i; + sd_mutex_lock(&g.lock); + for (i = 0; i < g.nprofs && n < SVC_MAX_PROFILES; i++) { + if (g.profs[i]->p.autostart != SD_PROFILE_AUTOSTART_BOOT) + continue; + strcpy_s(ids[n], sizeof(ids[n]), g.profs[i]->p.id); + strcpy_s(names[n], sizeof(names[n]), g.profs[i]->p.name); + n++; + } + sd_mutex_unlock(&g.lock); + for (i = 0; i < n; i++) { + sd_err e; + const char *code = NULL; + char id[16], target[4], detail[200]; + e = svc_mount_queue(ids[i], NULL, id, target, &code, detail); + if (e == SD_OK) + SD_LOGI("svc", "autostart boot: profile %s (%s) mounts as %s", ids[i], names[i], target); + else if (e != SD_E_EXISTS) /* already mounted: a reload keeps the one mount */ + SD_LOGW("svc", "autostart boot: profile %s not mounted: %s (%s)", ids[i], sd_err_name(e), + code ? code : detail[0] ? detail : ""); + } +} + static sd_err handle_unmount(sd_ipc_client *c, const sd_json *params, sd_json_writer *res, sd_ipc_error *err) { svc_mount *r; @@ -2857,6 +2918,7 @@ static sd_err handle_config_reload(sd_ipc_client *c, sd_json_writer *res, sd_ipc if (e != SD_OK) return e; n = svc_load_profiles(); + svc_autostart_boot_mounts(); sd_jw_key(res, "profiles"); sd_jw_obj_begin(res); sd_jw_key(res, "loaded"); sd_jw_int(res, n); @@ -3176,8 +3238,10 @@ static sd_err handle_service_repair(sd_ipc_client *c, sd_json_writer *res, sd_ip healthy = g.idle ? 0 : 1; g.repairing = 0; sd_mutex_unlock(&g.lock); - if (healthy) + if (healthy) { (void)svc_load_profiles(); + svc_autostart_boot_mounts(); + } svc_log_info("machine data folder repair: error %u, %u folders, %s, %s", (unsigned)we, (unsigned)r.folders, r.moved ? "moved aside" : "kept", healthy ? "trusted" : "still refused"); @@ -3343,6 +3407,7 @@ int svc_run(const svc_opts *o) svc_setup_secrets(o && o->secret_dir ? o->secret_dir : g.service_dir); (void)svc_load_profiles(); svc_log_info("service starting; %u machine profiles", (unsigned)g.nprofs); + svc_autostart_boot_mounts(); memset(&cfg, 0, sizeof(cfg)); cfg.struct_size = sizeof(cfg); diff --git a/tests/unit/sd_svc/main.c b/tests/unit/sd_svc/main.c index 6f86c8f..ae1dc04 100644 --- a/tests/unit/sd_svc/main.c +++ b/tests/unit/sd_svc/main.c @@ -45,6 +45,8 @@ SD_TEST_DECL(refuse_log_is_once_per_sid_per_window); SD_TEST_DECL(svc_refuses_a_machine_folder_with_a_foreign_owner_at_start); SD_TEST_DECL(svc_status_names_a_refused_machine_folder_for_admins_only); SD_TEST_DECL(svc_repair_moves_the_folder_aside_and_the_service_recovers); +SD_TEST_DECL(svc_autostart_boot_is_mounted_at_start_and_logon_is_not); +SD_TEST_DECL(svc_autostart_reload_does_not_double_mount); SD_TEST_DECL(scm_install_registers_the_service_in_order); SD_TEST_DECL(scm_install_skips_the_sid_type_before_vista); SD_TEST_DECL(scm_install_refuses_without_elevation); @@ -144,6 +146,8 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(svc_refuses_a_machine_folder_with_a_foreign_owner_at_start), SD_TEST_ENTRY(svc_status_names_a_refused_machine_folder_for_admins_only), SD_TEST_ENTRY(svc_repair_moves_the_folder_aside_and_the_service_recovers), + SD_TEST_ENTRY(svc_autostart_boot_is_mounted_at_start_and_logon_is_not), + SD_TEST_ENTRY(svc_autostart_reload_does_not_double_mount), SD_TEST_ENTRY(scm_install_registers_the_service_in_order), SD_TEST_ENTRY(scm_install_skips_the_sid_type_before_vista), SD_TEST_ENTRY(scm_install_refuses_without_elevation), diff --git a/tests/unit/sd_svc/test_service.c b/tests/unit/sd_svc/test_service.c index 0c5108c..5d8d22c 100644 --- a/tests/unit/sd_svc/test_service.c +++ b/tests/unit/sd_svc/test_service.c @@ -2048,3 +2048,242 @@ SD_TEST(svc_refuses_a_machine_folder_with_a_foreign_owner_at_start) SD_CHECK(GetFileAttributesW(sub) == INVALID_FILE_ATTRIBUTES); RemoveDirectoryW(root); } + +/* ---- boot autostart (1.8, 3.4.1): the service mounts autostart = boot profiles at start ---- + Each test runs a core of its own on a fresh machine root after the shared one has stopped: the profiles are + in place before the start, so what the test sees is the start-time autostart. */ + +#define T_ID_BOOTAUTO "00000000000000000000000000000020" +#define T_ID_BOOTLOGON "00000000000000000000000000000021" + +/* write_profile plus the [mount] autostart key, which sd_profile_to_ini does not write (the editor writes the + key into the file itself): the service reads it back when it loads the profiles. */ +static int write_profile_autostart(const wchar_t *root, const wchar_t *file, const sd_profile *p, + const char *autostart) +{ + sd_ini *ini = NULL; + sd_wr w; + wchar_t path[MAX_PATH]; + int ok = 0; + _snwprintf_s(path, MAX_PATH, _TRUNCATE, L"%s\\profiles\\%s", root, file); + sd_wr_init(&w, 4096); + if (sd_profile_to_ini(p, &ini) == SD_OK && sd_ini_set(ini, "mount", "autostart", autostart) == SD_OK && + sd_ini_serialize(ini, &w) == SD_OK) + ok = write_file(path, (const char *)w.p, w.len); + sd_ini_free(ini); + sd_wr_free(&w); + return ok; +} + +static HANDLE g_boot_running; +static HANDLE g_boot_thread; +static int g_boot_rc = -1; + +static void boot_core_running(void *ctx) +{ + SetEvent((HANDLE)ctx); +} + +static DWORD WINAPI boot_core_main(LPVOID arg) +{ + g_boot_rc = svc_run((svc_opts *)arg); + return 0; +} + +/* Starts a core of its own on root (the seams of the shared core) and returns once it listens; the test talks + to it over the pipe and stops it with stop_boot_core. */ +static int start_boot_core(svc_opts *o, const wchar_t *root) +{ + wchar_t sid[256], suffix[64]; + memset(o, 0, sizeof(*o)); + o->backend = fake_backend_hostkeys(); + o->machine_root = root; + o->open_log = 0; + o->owner_trusted = owner_trust_all; + o->test_all_admin = 1; + o->dacl_extra_ace = L"(A;OICI;FA;;;OW)"; + o->on_running = boot_core_running; + if (sd_ipc_current_identity(sid, SD_COUNTOF(sid), NULL) != SD_OK) + return 0; + _snwprintf_s(suffix, SD_COUNTOF(suffix), _TRUNCATE, L"svc%lu", (unsigned long)GetCurrentProcessId()); + sd_ipc_set_instance_suffix(suffix); + sd_ipc_test_set_expected_owner(sid); + sd_ipc_test_set_service_owner(sid); + g_boot_running = CreateEventW(NULL, TRUE, FALSE, NULL); + if (g_boot_running == NULL) + return 0; + o->ctx = g_boot_running; + g_boot_rc = -1; + g_boot_thread = CreateThread(NULL, 0, boot_core_main, o, 0, NULL); + if (g_boot_thread == NULL) { + CloseHandle(g_boot_running); + g_boot_running = NULL; + return 0; + } + return WaitForSingleObject(g_boot_running, 15000) == WAIT_OBJECT_0; +} + +static void stop_boot_core(void) +{ + svc_request_stop(); + (void)WaitForSingleObject(g_boot_thread, 30000); + CloseHandle(g_boot_thread); + g_boot_thread = NULL; + CloseHandle(g_boot_running); + g_boot_running = NULL; +} + +/* Counts the records of the two autostart profiles in a list reply; mid keeps the mount id of the boot one. */ +static void boot_list_scan(sd_json *res, int *n_boot, int *n_logon, char *mid, size_t mid_cap) +{ + const sd_json *mounts = sd_json_get(res, "mounts"); + int i; + *n_boot = 0; + *n_logon = 0; + for (i = 0; mounts && i < (int)sd_json_count(mounts); i++) { + const sd_json *m = sd_json_at(mounts, i); + const char *pid = sd_json_get_str(m, "profile"); + if (pid == NULL) + continue; + if (strcmp(pid, T_ID_BOOTAUTO) == 0) { + copy_str(mid, mid_cap, sd_json_get_str(m, "mount_id")); + (*n_boot)++; + } else if (strcmp(pid, T_ID_BOOTLOGON) == 0) { + (*n_logon)++; + } + } +} + +/* Waits until the boot profile is mounted through the start autostart (the mount comes up asynchronously) and + the logon profile has no record; scans the list each round. Returns 1 when seen. */ +static int wait_boot_list(sd_ipc_conn *c, int *n_boot, int *n_logon, char *mid, size_t mid_cap) +{ + sd_ipc_error err; + sd_err e; + uint32_t waited = 0; + for (;;) { + sd_json *res = call(c, "list", NULL, &err, &e); + int seen = 0; + if (res != NULL) { + boot_list_scan(res, n_boot, n_logon, mid, mid_cap); + sd_json_free(res); + seen = *n_boot == 1 && *n_logon == 0 && mid[0] != 0; + if (seen) { + /* the record exists; mounted is what makes it the autostart the docs ask for */ + char params[64]; + sd_json *st; + _snprintf_s(params, sizeof(params), _TRUNCATE, "{\"mount_id\":\"%s\"}", mid); + st = call(c, "status", params, &err, &e); + if (st != NULL) { + const char *got = sd_json_get_str(st, "state"); + seen = got != NULL && strcmp(got, "mounted") == 0; + sd_json_free(st); + } + } + } + if (seen) + return 1; + if (waited >= 10000) + return 0; + Sleep(100); + waited += 100; + } +} + +/* A fresh machine root for one boot-autostart core: the profiles folder and the service key the profiles carry. + The key must live under this root's own service folder (TM-R27): an identity outside it is refused at mount. */ +static int make_boot_root(const wchar_t *root, wchar_t *key, size_t key_cch) +{ + wchar_t d[MAX_PATH]; + if (!make_run_root((wchar_t *)root, MAX_PATH)) + return 0; + _snwprintf_s(d, MAX_PATH, _TRUNCATE, L"%s\\profiles", root); + if (!CreateDirectoryW(d, NULL)) + return 0; + _snwprintf_s(d, MAX_PATH, _TRUNCATE, L"%s\\service", root); + if (!CreateDirectoryW(d, NULL)) + return 0; + _snwprintf_s(d, MAX_PATH, _TRUNCATE, L"%s\\service\\keys", root); + if (!CreateDirectoryW(d, NULL)) + return 0; + _snwprintf_s(key, key_cch, _TRUNCATE, L"%s\\service\\keys\\key1.ppk", root); + return write_file(key, "fixture-key", 11); +} + +/* A service profile of the boot fixture: the shared base, the identity under root's own service folder and the + [mount] autostart value. */ +static int write_boot_profile(const wchar_t *root, const wchar_t *file, const char *id, const char *name, + const char *autostart) +{ + sd_profile p = base_profile(id, name, "trusted.example"); + char utf8[MAX_PATH * 3]; + wchar_t key[MAX_PATH]; + _snwprintf_s(key, MAX_PATH, _TRUNCATE, L"%s\\service\\keys\\key1.ppk", root); + utf8_of(key, utf8, sizeof(utf8)); + strcpy_s(p.identity, sizeof(p.identity), utf8); + return write_profile_autostart(root, file, &p, autostart); +} + +SD_TEST(svc_autostart_boot_is_mounted_at_start_and_logon_is_not) +{ + static svc_opts o; + wchar_t root[MAX_PATH], key[MAX_PATH]; + sd_ipc_conn *c = NULL; + sd_ipc_error err; + sd_err e; + sd_json *res; + int n_boot = -1, n_logon = -1; + char mid[16] = ""; + SD_REQUIRE(make_boot_root(root, key, SD_COUNTOF(key))); + SD_REQUIRE(write_boot_profile(root, L"a-bootauto.ini", T_ID_BOOTAUTO, "bootauto", "boot")); + SD_REQUIRE(write_boot_profile(root, L"b-bootlogon.ini", T_ID_BOOTLOGON, "bootlogon", "logon")); + SD_REQUIRE(start_boot_core(&o, root)); + SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); + /* the service mounts the boot profile by itself; autostart = logon is the agent's business (3.4.1) */ + SD_CHECK(wait_boot_list(c, &n_boot, &n_logon, mid, sizeof(mid))); + SD_CHECK_EQ_INT(n_boot, 1); + SD_CHECK_EQ_INT(n_logon, 0); + /* the autostart record is the one mount: a second one for the same profile is refused */ + res = mount_profile(c, T_ID_BOOTAUTO, &err, &e); + SD_CHECK(res == NULL); + SD_CHECK_STR(err.code, "already_mounted"); + sd_ipc_close(c); + stop_boot_core(); + SD_CHECK_EQ_INT(g_boot_rc, 0); + RemoveDirectoryW(root); /* best effort: the root holds the profile files */ +} + +SD_TEST(svc_autostart_reload_does_not_double_mount) +{ + static svc_opts o; + wchar_t root[MAX_PATH], key[MAX_PATH]; + sd_ipc_conn *c = NULL; + sd_ipc_error err; + sd_err e; + sd_json *res; + int n_boot = -1, n_logon = -1; + char mid[16] = "", mid2[16] = ""; + SD_REQUIRE(make_boot_root(root, key, SD_COUNTOF(key))); + SD_REQUIRE(write_boot_profile(root, L"a-bootauto.ini", T_ID_BOOTAUTO, "bootauto", "boot")); + SD_REQUIRE(start_boot_core(&o, root)); + SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); + SD_REQUIRE(wait_boot_list(c, &n_boot, &n_logon, mid, sizeof(mid))); + /* config.reload loads the profiles again: the mounted profile keeps its one mount, id included */ + res = call(c, "config.reload", NULL, &err, &e); + SD_REQUIRE(res != NULL); + sd_json_free(res); + res = call(c, "list", NULL, &err, &e); + SD_REQUIRE(res != NULL); + boot_list_scan(res, &n_boot, &n_logon, mid2, sizeof(mid2)); + sd_json_free(res); + SD_CHECK_EQ_INT(n_boot, 1); + SD_CHECK_STR(mid2, mid); + /* and the manual mount of the same profile is still refused */ + res = mount_profile(c, T_ID_BOOTAUTO, &err, &e); + SD_CHECK(res == NULL); + SD_CHECK_STR(err.code, "already_mounted"); + sd_ipc_close(c); + stop_boot_core(); + SD_CHECK_EQ_INT(g_boot_rc, 0); + RemoveDirectoryW(root); +} From 7f4538c358c473465fec3d9f861905df3cdd5ad0 Mon Sep 17 00:00:00 2001 From: FacuM Date: Sat, 31 Oct 2026 00:42:19 -0300 Subject: [PATCH 5/9] cli: name the profile in service errors and tell SCM denials from missing elevation service mount, unmount and status now pass the requested name to fail_ipc, so "There is no profile named {name}." shows the profile instead of an empty placeholder (m5-notes known defect). scm_error splits ERROR_ACCESS_DENIED by cli_is_elevated: a caller that is not elevated keeps the elevation message and exit 9 (new cli.service.manage_need_admin, "Managing the ShellDrive service ...", more accurate for install, uninstall, start and stop), while an elevated console whose SCM call was refused gets cli.service.scm_failed with the Win32 code and exit 1, as 03 6.1 assigns. SHELLDRIVE_TEST_SCM=denied drives both branches without touching the real SCM; a smoke test and an e2e test cover them and the named profile. --- apps/shelldrive/cli_service.c | 72 ++++++++++++++++++++++++++++++----- l10n/cli/de.po | 5 +++ l10n/cli/en.po | 5 +++ l10n/cli/es.po | 5 +++ l10n/cli/fr.po | 5 +++ l10n/cli/it.po | 5 +++ l10n/cli/pt-BR.po | 5 +++ tests/apps/main.c | 2 + tests/apps/test_smoke.c | 25 ++++++++++++ tests/e2e/main.c | 2 + tests/e2e/test_cli_service.c | 10 +++++ 11 files changed, 132 insertions(+), 9 deletions(-) diff --git a/apps/shelldrive/cli_service.c b/apps/shelldrive/cli_service.c index f63d8f2..56852fa 100644 --- a/apps/shelldrive/cli_service.c +++ b/apps/shelldrive/cli_service.c @@ -145,7 +145,10 @@ static int run_status(const cli_args *a, sd_ipc_conn *c) sd_wr w; sd_ipc_error err; sd_json *res = NULL; + err_ctx ec; int rc; + memset(&ec, 0, sizeof(ec)); + ec.name = a->arg2; if (key_request(a, 0, &w) != SD_OK) { sd_wr_free(&w); return fail_code(a, "internal", "request", NULL); @@ -153,7 +156,7 @@ static int run_status(const cli_args *a, sd_ipc_conn *c) rc = sd_ipc_call_raw(c, "status", (const char *)w.p, w.len, CALL_TIMEOUT, &res, &err) == SD_OK ? 0 : -1; sd_wr_free(&w); if (rc != 0) - return fail_ipc(a, &err, NULL); + return fail_ipc(a, &err, &ec); if (a->json) json_ok(res); else @@ -167,7 +170,10 @@ static int run_unmount(const cli_args *a, sd_ipc_conn *c) sd_wr w; sd_ipc_error err; sd_json *res = NULL; + err_ctx ec; int rc; + memset(&ec, 0, sizeof(ec)); + ec.name = a->arg2; if (key_request(a, a->force, &w) != SD_OK) { sd_wr_free(&w); return fail_code(a, "internal", "request", NULL); @@ -175,7 +181,7 @@ static int run_unmount(const cli_args *a, sd_ipc_conn *c) rc = sd_ipc_call_raw(c, "unmount", (const char *)w.p, w.len, CALL_TIMEOUT, &res, &err) == SD_OK ? 0 : -1; sd_wr_free(&w); if (rc != 0) - return fail_ipc(a, &err, NULL); + return fail_ipc(a, &err, &ec); if (a->json) { json_ok(res); } else { @@ -197,8 +203,11 @@ static int run_mount(const cli_args *a, sd_ipc_conn *c) sd_ipc_error err; sd_json *res = NULL, *st = NULL; const char *mid, *tgt; + err_ctx ec; DWORD waited = 0; utf8_of(a->arg2, profile, sizeof(profile)); + memset(&ec, 0, sizeof(ec)); + ec.name = a->arg2; sd_wr_init(&w, 1024); sd_jw_init(&jw, &w); sd_jw_obj_begin(&jw); @@ -211,7 +220,7 @@ static int run_mount(const cli_args *a, sd_ipc_conn *c) } if (sd_ipc_call_raw(c, "mount", (const char *)w.p, w.len, CALL_TIMEOUT, &res, &err) != SD_OK) { sd_wr_free(&w); - return fail_ipc(a, &err, NULL); + return fail_ipc(a, &err, &ec); } sd_wr_free(&w); mid = sd_json_get_str(res, "mount_id"); @@ -232,7 +241,7 @@ static int run_mount(const cli_args *a, sd_ipc_conn *c) const char *state; int rc = 0; if (sd_ipc_call_raw(c, "status", params, strlen(params), CALL_TIMEOUT, &st, &err) != SD_OK) { - rc = fail_ipc(a, &err, NULL); + rc = fail_ipc(a, &err, &ec); sd_json_free(res); return rc; } @@ -272,14 +281,21 @@ static int is_verb(const wchar_t *v, const wchar_t *name) return v && wcscmp(v, name) == 0; } -/* Maps a Win32 error of the Service Control Manager to a message and exit code (non-zero result). */ +/* Maps a Win32 error of the Service Control Manager to a message and exit code (non-zero result). ERROR_ACCESS_DENIED + is split by cli_is_elevated: the svc_scm_* rules deny a caller that is not elevated before any SCM call (exit 9, + elevation message), while an elevated console whose SCM call was refused gets the SCM message with the code (exit 1). */ static int scm_error(DWORD e) { sd_arg arg; switch (e) { case ERROR_ACCESS_DENIED: - say(SD_MSG_CLI_SERVICE_NEED_ADMIN, NULL, 0, 1); - return 9; + if (!cli_is_elevated()) { + say(SD_MSG_CLI_SERVICE_MANAGE_NEED_ADMIN, NULL, 0, 1); + return 9; + } + arg.name = "code"; arg.type = SD_ARG_INT; arg.v.i = (long long)e; + say(SD_MSG_CLI_SERVICE_SCM_FAILED, &arg, 1, 1); + return 1; case ERROR_SERVICE_EXISTS: say(SD_MSG_CLI_SERVICE_ALREADY_INSTALLED, NULL, 0, 1); return 1; @@ -299,6 +315,11 @@ static int scm_error(DWORD e) } } +#ifdef SD_CLI_TEST_HOOKS +/* Test seam, defined with the status report below: SHELLDRIVE_TEST_SCM applies to the service verbs too. */ +static void test_scm_seam(svc_scm_ops *ops); +#endif + /* install, uninstall, start and stop (M5, 03 §1.3, §6.1). The rules and the SCM calls are in svc_scm.c; this only prints. The description is the localized text of this process (the cli domain). */ static int scm_verb(const wchar_t *verb) @@ -308,6 +329,9 @@ static int scm_verb(const wchar_t *verb) sd_msg_id done; DWORD e; svc_scm_real_ops(&ops); +#ifdef SD_CLI_TEST_HOOKS + test_scm_seam(&ops); +#endif if (is_verb(verb, L"install")) { e = svc_scm_host_spec(&spec, sd_l10n_get(SD_MSG_CLI_SERVICE_DESCRIPTION)); if (e == 0) @@ -454,8 +478,9 @@ static void print_report_json(const svc_status_report *r, const svc_pipe_report #ifdef SD_CLI_TEST_HOOKS /* Test seam (test builds only): SHELLDRIVE_TEST_SCM=running (installed and running) or absent (not installed) replaces - the two read-only SCM queries, so the pipe path of service status runs on a machine without ShellDriveSvc. Any other - value, or no value, leaves the real SCM. */ + the two read-only SCM queries, so the pipe path of service status runs on a machine without ShellDriveSvc; + =denied replaces the install, uninstall, start and stop calls with refusals (scm_verb). Any other value, or no + value, leaves the real SCM. */ static DWORD test_query_service(void *ctx, svc_status_report *out) { wchar_t v[16]; @@ -479,11 +504,40 @@ static DWORD test_query_source(void *ctx, const wchar_t *source, wchar_t *messag return ERROR_FILE_NOT_FOUND; } +static DWORD test_scm_refused(void *ctx) +{ + (void)ctx; + return ERROR_ACCESS_DENIED; +} + +static DWORD test_create_refused(void *ctx, const svc_install_spec *s) +{ + (void)ctx; + (void)s; + return ERROR_ACCESS_DENIED; +} + +/* Follows SHELLDRIVE_TEST_ELEVATED, so scm_error sees both callers: the rules functions deny the one that is not + elevated, the fake SCM calls deny the elevated one. No real service is created, started, stopped or removed. */ +static int test_scm_elevated(void *ctx) +{ + (void)ctx; + return cli_is_elevated(); +} + static void test_scm_seam(svc_scm_ops *ops) { wchar_t v[16]; if (GetEnvironmentVariableW(L"SHELLDRIVE_TEST_SCM", v, SD_COUNTOF(v)) == 0) return; + if (wcscmp(v, L"denied") == 0) { + ops->create_service = test_create_refused; + ops->delete_service = test_scm_refused; + ops->start_service = test_scm_refused; + ops->stop_service = test_scm_refused; + ops->is_elevated = test_scm_elevated; + return; + } if (wcscmp(v, L"running") != 0 && wcscmp(v, L"absent") != 0) return; ops->query_service = test_query_service; diff --git a/l10n/cli/de.po b/l10n/cli/de.po index 656fc89..66d10e9 100644 --- a/l10n/cli/de.po +++ b/l10n/cli/de.po @@ -821,6 +821,11 @@ msgctxt "cli.service.need_admin" msgid "Changing global drives needs an elevated console. Run it as administrator." msgstr "Zum Ändern globaler Laufwerke ist eine erhöhte Konsole nötig. Starte das Programm als Administrator." +#. Printed by "shelldrive service install, uninstall, start or stop" from a console that is not elevated (exit 9). +msgctxt "cli.service.manage_need_admin" +msgid "Managing the ShellDrive service needs an elevated console. Run it as administrator." +msgstr "Zum Verwalten des ShellDrive-Dienstes ist eine erhöhte Konsole nötig. Starte das Programm als Administrator." + msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "Der ShellDrive-Dienst läuft nicht oder ist nicht installiert." diff --git a/l10n/cli/en.po b/l10n/cli/en.po index cdc7f30..a69ee1f 100644 --- a/l10n/cli/en.po +++ b/l10n/cli/en.po @@ -890,6 +890,11 @@ msgctxt "cli.service.need_admin" msgid "Changing global drives needs an elevated console. Run it as administrator." msgstr "" +#. Printed by "shelldrive service install, uninstall, start or stop" from a console that is not elevated (exit 9). +msgctxt "cli.service.manage_need_admin" +msgid "Managing the ShellDrive service needs an elevated console. Run it as administrator." +msgstr "" + #. Printed when the service pipe cannot be reached (exit 8). msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." diff --git a/l10n/cli/es.po b/l10n/cli/es.po index e4599f7..4bf64d3 100644 --- a/l10n/cli/es.po +++ b/l10n/cli/es.po @@ -830,6 +830,11 @@ msgctxt "cli.service.need_admin" msgid "Changing global drives needs an elevated console. Run it as administrator." msgstr "Cambiar unidades globales requiere una consola elevada. Ejecutá el programa como administrador." +#. Printed by "shelldrive service install, uninstall, start or stop" from a console that is not elevated (exit 9). +msgctxt "cli.service.manage_need_admin" +msgid "Managing the ShellDrive service needs an elevated console. Run it as administrator." +msgstr "Administrar el servicio de ShellDrive requiere una consola elevada. Ejecutá el programa como administrador." + msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "El servicio de ShellDrive no está en ejecución o no está instalado." diff --git a/l10n/cli/fr.po b/l10n/cli/fr.po index 4495116..6646cfa 100644 --- a/l10n/cli/fr.po +++ b/l10n/cli/fr.po @@ -830,6 +830,11 @@ msgctxt "cli.service.need_admin" msgid "Changing global drives needs an elevated console. Run it as administrator." msgstr "Modifier les lecteurs globaux demande une console élevée. Exécutez-la en tant qu'administrateur." +#. Printed by "shelldrive service install, uninstall, start or stop" from a console that is not elevated (exit 9). +msgctxt "cli.service.manage_need_admin" +msgid "Managing the ShellDrive service needs an elevated console. Run it as administrator." +msgstr "La gestion du service ShellDrive demande une console élevée. Exécutez-la en tant qu'administrateur." + msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "Le service ShellDrive n'est pas démarré ou n'est pas installé." diff --git a/l10n/cli/it.po b/l10n/cli/it.po index 32cfe3d..0b4cea2 100644 --- a/l10n/cli/it.po +++ b/l10n/cli/it.po @@ -830,6 +830,11 @@ msgctxt "cli.service.need_admin" msgid "Changing global drives needs an elevated console. Run it as administrator." msgstr "Modificare le unità globali richiede una console elevata. Esegui il programma come amministratore." +#. Printed by "shelldrive service install, uninstall, start or stop" from a console that is not elevated (exit 9). +msgctxt "cli.service.manage_need_admin" +msgid "Managing the ShellDrive service needs an elevated console. Run it as administrator." +msgstr "Gestire il servizio ShellDrive richiede una console elevata. Esegui il programma come amministratore." + msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "Il servizio ShellDrive non è in esecuzione o non è installato." diff --git a/l10n/cli/pt-BR.po b/l10n/cli/pt-BR.po index f5b7dd7..936161f 100644 --- a/l10n/cli/pt-BR.po +++ b/l10n/cli/pt-BR.po @@ -830,6 +830,11 @@ msgctxt "cli.service.need_admin" msgid "Changing global drives needs an elevated console. Run it as administrator." msgstr "Alterar unidades globais exige um console elevado. Execute o programa como administrador." +#. Printed by "shelldrive service install, uninstall, start or stop" from a console that is not elevated (exit 9). +msgctxt "cli.service.manage_need_admin" +msgid "Managing the ShellDrive service needs an elevated console. Run it as administrator." +msgstr "Gerenciar o serviço ShellDrive exige um console elevado. Execute o programa como administrador." + msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "O serviço do ShellDrive não está em execução ou não está instalado." diff --git a/tests/apps/main.c b/tests/apps/main.c index 683b727..a7d164e 100644 --- a/tests/apps/main.c +++ b/tests/apps/main.c @@ -59,6 +59,7 @@ SD_TEST_DECL(diag_deny_parse_truncates_at_the_capacity); SD_TEST_DECL(diag_netloc_compare_token_lists); SD_TEST_DECL(cli_parse_location_verbs); SD_TEST_DECL(smoke_location_verbs); +SD_TEST_DECL(smoke_service_scm_denials); static const sd_test_case k_tests[] = { SD_TEST_ENTRY(cli_split_follows_msvc_rules), @@ -120,6 +121,7 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(diag_netloc_compare_token_lists), SD_TEST_ENTRY(cli_parse_location_verbs), SD_TEST_ENTRY(smoke_location_verbs), + SD_TEST_ENTRY(smoke_service_scm_denials), }; int main(int argc, char **argv) diff --git a/tests/apps/test_smoke.c b/tests/apps/test_smoke.c index 178977e..b5cee14 100644 --- a/tests/apps/test_smoke.c +++ b/tests/apps/test_smoke.c @@ -1186,3 +1186,28 @@ SD_TEST(smoke_location_verbs) SetEnvironmentVariableW(L"SHELLDRIVE_TEST_NETHOOD", NULL); DeleteFileW(other); } + +/* service install | uninstall | start | stop with the SCM seam forced to deny (SHELLDRIVE_TEST_SCM=denied): the + rules functions deny a caller that is not elevated with the elevation message (exit 9), while an elevated console + whose SCM call is refused gets the SCM message with the Win32 code (exit 1). No real service is touched. */ +SD_TEST(smoke_service_scm_denials) +{ + run_result r; + SD_REQUIRE(setup()); + SetEnvironmentVariableW(L"SHELLDRIVE_TEST_SCM", L"denied"); + + SetEnvironmentVariableW(L"SHELLDRIVE_TEST_ELEVATED", L"0"); + EXPECT(RUN(L"service start", NULL) == 9 && strstr(r.err, "Managing the ShellDrive service") != NULL, + "a non-elevated start is denied by the rules with exit 9 and the elevation message"); + EXPECT(RUN(L"service uninstall", NULL) == 9, "a non-elevated uninstall is denied with exit 9"); + + SetEnvironmentVariableW(L"SHELLDRIVE_TEST_ELEVATED", L"1"); + EXPECT(RUN(L"service start", NULL) == 1 && strstr(r.err, "refused the request (error 5)") != NULL, + "an elevated start whose SCM call is denied names the Win32 code with exit 1"); + EXPECT(RUN(L"service install", NULL) == 1 && strstr(r.err, "refused the request (error 5)") != NULL, + "an elevated install whose SCM call is denied names the Win32 code with exit 1"); + EXPECT(RUN(L"service stop", NULL) == 1, "an elevated stop whose SCM call is denied exits 1"); + + SetEnvironmentVariableW(L"SHELLDRIVE_TEST_ELEVATED", NULL); + SetEnvironmentVariableW(L"SHELLDRIVE_TEST_SCM", NULL); +} diff --git a/tests/e2e/main.c b/tests/e2e/main.c index 5d1d241..a585812 100644 --- a/tests/e2e/main.c +++ b/tests/e2e/main.c @@ -6,6 +6,7 @@ SD_TEST_DECL(e2e_status_without_an_installed_service_does_not_query_the_pipe); SD_TEST_DECL(e2e_service_mount_and_unmount_need_elevation); SD_TEST_DECL(e2e_hostkey_offer_is_listed_and_accepted_into_the_machine_store); SD_TEST_DECL(e2e_secret_set_and_delete_machine_proxy_through_the_service); +SD_TEST_DECL(e2e_service_mount_error_names_the_profile); SD_TEST_DECL(e2e_core_stops_and_the_cli_no_longer_reaches_the_pipe); static const sd_test_case k_tests[] = { @@ -14,6 +15,7 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(e2e_service_mount_and_unmount_need_elevation), SD_TEST_ENTRY(e2e_hostkey_offer_is_listed_and_accepted_into_the_machine_store), SD_TEST_ENTRY(e2e_secret_set_and_delete_machine_proxy_through_the_service), + SD_TEST_ENTRY(e2e_service_mount_error_names_the_profile), SD_TEST_ENTRY(e2e_core_stops_and_the_cli_no_longer_reaches_the_pipe), }; diff --git a/tests/e2e/test_cli_service.c b/tests/e2e/test_cli_service.c index ebcb19f..b28405e 100644 --- a/tests/e2e/test_cli_service.c +++ b/tests/e2e/test_cli_service.c @@ -119,6 +119,16 @@ SD_TEST(e2e_secret_set_and_delete_machine_proxy_through_the_service) SD_CHECK(!found); } +SD_TEST(e2e_service_mount_error_names_the_profile) +{ + e2e_cli_result r; + SD_REQUIRE(e2e_core_start()); + /* a profile the service does not have: the error names it (exit 3) */ + SD_REQUIRE(e2e_run_cli(L"service mount nosuch", L"1", NULL, NULL, &r)); + SD_CHECK_EQ_INT(r.exit_code, 3); + SD_CHECK(has(r.out, "no profile named nosuch")); +} + SD_TEST(e2e_core_stops_and_the_cli_no_longer_reaches_the_pipe) { e2e_cli_result r; From b478b45f251b74188e9774e6be86c13114ac930e Mon Sep 17 00:00:00 2001 From: FacuM Date: Sat, 31 Oct 2026 01:04:29 -0300 Subject: [PATCH 6/9] cli: report a refused service-pipe connect as access denied, not 'not running' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The service pipe's DACL grants read/write to interactive logons only (03 §2.2, carried in the threat model), so a member of service_access in a batch logon is denied at CreateFileW. open_pipe() folded that refusal into SD_E_NOT_FOUND and connect_service() printed 'not running or not installed' with exit 8. open_pipe() now returns SD_E_ACCESS_DENIED on ERROR_ACCESS_DENIED, connect_service() returns the sd_err, and the CLI reports access_denied with exit 9 through a new localized message in all six catalogs. Unit tests pin the service pipe DACL to the design and the new error mapping; a smoke test covers the CLI exit codes and JSON. --- apps/shelldrive/cli_cmds2.c | 29 +++++---- apps/shelldrive/cli_common.c | 16 +++++ apps/shelldrive/cli_int.h | 8 ++- apps/shelldrive/cli_service.c | 21 +++--- l10n/cli/de.po | 5 ++ l10n/cli/en.po | 5 ++ l10n/cli/es.po | 5 ++ l10n/cli/fr.po | 5 ++ l10n/cli/it.po | 5 ++ l10n/cli/pt-BR.po | 5 ++ src/sd_ipc/ipc_client.c | 8 ++- tests/apps/main.c | 2 + tests/apps/test_smoke.c | 33 ++++++++++ tests/unit/sd_ipc/main.c | 4 ++ tests/unit/sd_ipc/test_ipc.c | 119 ++++++++++++++++++++++++++++++++++ 15 files changed, 243 insertions(+), 27 deletions(-) diff --git a/apps/shelldrive/cli_cmds2.c b/apps/shelldrive/cli_cmds2.c index 78c3564..beae4a3 100644 --- a/apps/shelldrive/cli_cmds2.c +++ b/apps/shelldrive/cli_cmds2.c @@ -640,9 +640,10 @@ static int hk_announced(const cli_args *a, int machine) sd_json *res = NULL; int rc = 0; if (machine) { - if (!connect_service(&c)) { - say(SD_MSG_CLI_SERVICE_UNAVAILABLE, NULL, 0, 1); - return 8; + { + sd_err ce = connect_service(&c); + if (ce != SD_OK) + return service_connect_fail(a, ce); } } else if (sd_ipc_connect(SD_IPC_AGENT, quiet_hello(&hello), 0, 5000, &c) != SD_OK) { /* no agent: nobody holds an announcement */ @@ -907,9 +908,10 @@ static int hk_accept(const cli_args *a, int machine) int rc; sd_err e; if (machine) { - if (!connect_service(&c)) { - say(SD_MSG_CLI_SERVICE_UNAVAILABLE, NULL, 0, 1); - return 8; + { + sd_err ce = connect_service(&c); + if (ce != SD_OK) + return service_connect_fail(a, ce); } } else if (sd_ipc_connect(SD_IPC_AGENT, quiet_hello(&hello), 0, 5000, &c) != SD_OK) { return fail_code(a, "agent_unavailable", "cannot connect to the agent", NULL); @@ -997,9 +999,10 @@ static int hk_accept_offer(const cli_args *a, const char *id, int machine) sd_err e; int rc; if (machine) { - if (!connect_service(&c)) { - say(SD_MSG_CLI_SERVICE_UNAVAILABLE, NULL, 0, 1); - return 8; + { + sd_err ce = connect_service(&c); + if (ce != SD_OK) + return service_connect_fail(a, ce); } } else { rc = connect_agent(a, 1, &c, NULL); @@ -1043,7 +1046,7 @@ static void hk_reject_offer(const cli_args *a, const char *id, int machine) sd_err e; int rc; if (machine) { - if (!connect_service(&c)) + if (connect_service(&c) != SD_OK) return; } else { rc = connect_agent(a, 1, &c, NULL); @@ -1101,11 +1104,11 @@ static int hk_add_user(const cli_args *a, int machine) memset(&ec, 0, sizeof(ec)); ec.host = host; /* UTF-8, for {host} */ if (machine) { - if (!connect_service(&c)) { + e = connect_service(&c); + if (e != SD_OK) { sd_free(type8); sd_free(expect8); - say(SD_MSG_CLI_SERVICE_UNAVAILABLE, NULL, 0, 1); - return 8; + return service_connect_fail(a, e); } } else { rc = connect_agent(a, 1, &c, &ec); diff --git a/apps/shelldrive/cli_common.c b/apps/shelldrive/cli_common.c index 466733e..adfc1d8 100644 --- a/apps/shelldrive/cli_common.c +++ b/apps/shelldrive/cli_common.c @@ -224,6 +224,22 @@ int fail_code(const cli_args *a, const char *code, const char *detail, const err return xc; } +/* Reports a failed connect_service (03 §6.2). The service pipe's DACL (2.2) admits interactive sessions only, so a + script in a batch logon gets a refusal, and it must not read as "not running or not installed" (exit 8, TM): the + exit code is 9, the JSON code access_denied. */ +int service_connect_fail(const cli_args *a, sd_err e) +{ + if (e == SD_E_ACCESS_DENIED) { + if (a->json) + json_err("access_denied", "the ShellDrive service pipe denies this logon session", 9); + else + say(SD_MSG_CLI_SERVICE_ACCESS_DENIED, NULL, 0, 1); + return 9; + } + say(SD_MSG_CLI_SERVICE_UNAVAILABLE, NULL, 0, 1); + return 8; +} + /* ---- elevation ---- */ /* Changing a global drive or the machine store needs an elevated token (6.1). On XP the query is unavailable: diff --git a/apps/shelldrive/cli_int.h b/apps/shelldrive/cli_int.h index cd44039..d566737 100644 --- a/apps/shelldrive/cli_int.h +++ b/apps/shelldrive/cli_int.h @@ -46,8 +46,12 @@ int can_prompt(const cli_args *a); /* Answers one prompt.request event of this connection (cli_cmds.c; mount and profile test). */ void handle_prompt(const cli_args *a, sd_ipc_conn *c, const sd_json *d); int connect_agent(const cli_args *a, int start, sd_ipc_conn **c, const err_ctx *ec); -/* The service control pipe (cli_service.c), for the --machine commands: 1 when connected, 0 when it is not running. */ -int connect_service(sd_ipc_conn **c); +/* The service control pipe (cli_service.c), for the --machine commands: SD_OK when connected, otherwise the reason + (SD_E_ACCESS_DENIED when the pipe's DACL refuses this logon session, 2.2). */ +sd_err connect_service(sd_ipc_conn **c); +/* Reports a failed connect_service (03 §6.2) and returns the exit code: the pipe refusing this session is its own + outcome (access_denied, 9); every other reason is "not running or not installed" (8). */ +int service_connect_fail(const cli_args *a, sd_err e); int subscribe(sd_ipc_conn *c, const char *topic); const sd_ipc_hello *quiet_hello(sd_ipc_hello *h); /* Sends prompt.answer. save: keep the secret (with the localized comment) after a successful login. */ diff --git a/apps/shelldrive/cli_service.c b/apps/shelldrive/cli_service.c index 56852fa..6374dfa 100644 --- a/apps/shelldrive/cli_service.c +++ b/apps/shelldrive/cli_service.c @@ -47,7 +47,7 @@ static void utf8_of(const wchar_t *w, char *out, size_t cch) } -int connect_service(sd_ipc_conn **c) +sd_err connect_service(sd_ipc_conn **c) { sd_ipc_hello h; memset(&h, 0, sizeof(h)); @@ -55,7 +55,7 @@ int connect_service(sd_ipc_conn **c) h.client_version = SD_VERSION_STRING; h.lang = sd_l10n_lang_code(sd_l10n_lang()); h.caps = 0; - return sd_ipc_connect(SD_IPC_SERVICE, &h, 0, CALL_TIMEOUT, c) == SD_OK; + return sd_ipc_connect(SD_IPC_SERVICE, &h, 0, CALL_TIMEOUT, c); } /* Request body naming a drive ("X:" or "X:\") or a profile (built with the JSON writer: names may hold any text). */ @@ -406,7 +406,7 @@ static void query_pipe(svc_pipe_report *p) sd_json *res = NULL, *ev = NULL; int healthy = 0; memset(p, 0, sizeof(*p)); - if (!connect_service(&c)) + if (connect_service(&c) != SD_OK) return; p->reachable = 1; if (sd_ipc_call_raw(c, "selftest", NULL, 0, CALL_TIMEOUT, &res, &err) == SD_OK) { @@ -659,6 +659,7 @@ int cli_run_secret(const cli_args *a) char spec[400], pw[1024]; int set = a->positional != NULL && wcscmp(a->positional, L"set") == 0; int rc; + sd_err e_connect; pw[0] = 0; if (!cli_is_elevated()) { say(SD_MSG_CLI_SECRET_NEED_ADMIN, NULL, 0, 1); @@ -676,10 +677,10 @@ int cli_run_secret(const cli_args *a) return secret_fail(a, "usage", "the password is empty", SD_MSG_CLI_SECRET_EMPTY, 2); } } - if (!connect_service(&c)) { + e_connect = connect_service(&c); + if (e_connect != SD_OK) { SecureZeroMemory(pw, sizeof(pw)); - say(SD_MSG_CLI_SERVICE_UNAVAILABLE, NULL, 0, 1); - return 8; + return service_connect_fail(a, e_connect); } sd_wr_init_secret(&w, 1024); { @@ -721,6 +722,7 @@ int cli_run_service(const cli_args *a) { sd_ipc_conn *c = NULL; int rc; + sd_err ce; const wchar_t *verb = a->positional; if (verb == NULL) return 2; @@ -735,10 +737,9 @@ int cli_run_service(const cli_args *a) say(SD_MSG_CLI_SERVICE_NEED_ADMIN, NULL, 0, 1); return 9; } - if (!connect_service(&c)) { - say(SD_MSG_CLI_SERVICE_UNAVAILABLE, NULL, 0, 1); - return 8; - } + ce = connect_service(&c); + if (ce != SD_OK) + return service_connect_fail(a, ce); if (is_verb(verb, L"list")) rc = run_list(a, c); else if (is_verb(verb, L"mount")) diff --git a/l10n/cli/de.po b/l10n/cli/de.po index 66d10e9..957aaa7 100644 --- a/l10n/cli/de.po +++ b/l10n/cli/de.po @@ -830,6 +830,11 @@ msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "Der ShellDrive-Dienst läuft nicht oder ist nicht installiert." +#. Printed when the service pipe refuses this logon session, e.g. a batch logon of a service_access member (exit 9). +msgctxt "cli.service.access_denied" +msgid "Access to the ShellDrive service is denied." +msgstr "Der Zugriff auf den ShellDrive-Dienst wurde verweigert." + msgctxt "cli.service.none" msgid "No global drives are mounted." msgstr "Es sind keine globalen Laufwerke eingebunden." diff --git a/l10n/cli/en.po b/l10n/cli/en.po index a69ee1f..4547677 100644 --- a/l10n/cli/en.po +++ b/l10n/cli/en.po @@ -900,6 +900,11 @@ msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "" +#. Printed when the service pipe refuses this logon session, e.g. a batch logon of a service_access member (exit 9). +msgctxt "cli.service.access_denied" +msgid "Access to the ShellDrive service is denied." +msgstr "" + #. Printed by "shelldrive service list" when no global drive is mounted. msgctxt "cli.service.none" msgid "No global drives are mounted." diff --git a/l10n/cli/es.po b/l10n/cli/es.po index 4bf64d3..60b0ec3 100644 --- a/l10n/cli/es.po +++ b/l10n/cli/es.po @@ -839,6 +839,11 @@ msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "El servicio de ShellDrive no está en ejecución o no está instalado." +#. Printed when the service pipe refuses this logon session, e.g. a batch logon of a service_access member (exit 9). +msgctxt "cli.service.access_denied" +msgid "Access to the ShellDrive service is denied." +msgstr "El acceso al servicio de ShellDrive está denegado." + msgctxt "cli.service.none" msgid "No global drives are mounted." msgstr "No hay unidades globales montadas." diff --git a/l10n/cli/fr.po b/l10n/cli/fr.po index 6646cfa..691bfac 100644 --- a/l10n/cli/fr.po +++ b/l10n/cli/fr.po @@ -839,6 +839,11 @@ msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "Le service ShellDrive n'est pas démarré ou n'est pas installé." +#. Printed when the service pipe refuses this logon session, e.g. a batch logon of a service_access member (exit 9). +msgctxt "cli.service.access_denied" +msgid "Access to the ShellDrive service is denied." +msgstr "L'accès au service ShellDrive est refusé." + msgctxt "cli.service.none" msgid "No global drives are mounted." msgstr "Aucun lecteur global n'est monté." diff --git a/l10n/cli/it.po b/l10n/cli/it.po index 0b4cea2..2176894 100644 --- a/l10n/cli/it.po +++ b/l10n/cli/it.po @@ -839,6 +839,11 @@ msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "Il servizio ShellDrive non è in esecuzione o non è installato." +#. Printed when the service pipe refuses this logon session, e.g. a batch logon of a service_access member (exit 9). +msgctxt "cli.service.access_denied" +msgid "Access to the ShellDrive service is denied." +msgstr "L'accesso al servizio ShellDrive è negato." + msgctxt "cli.service.none" msgid "No global drives are mounted." msgstr "Nessuna unità globale è montata." diff --git a/l10n/cli/pt-BR.po b/l10n/cli/pt-BR.po index 936161f..ab9a48b 100644 --- a/l10n/cli/pt-BR.po +++ b/l10n/cli/pt-BR.po @@ -839,6 +839,11 @@ msgctxt "cli.service.unavailable" msgid "The ShellDrive service is not running or not installed." msgstr "O serviço do ShellDrive não está em execução ou não está instalado." +#. Printed when the service pipe refuses this logon session, e.g. a batch logon of a service_access member (exit 9). +msgctxt "cli.service.access_denied" +msgid "Access to the ShellDrive service is denied." +msgstr "O acesso ao serviço do ShellDrive está negado." + msgctxt "cli.service.none" msgid "No global drives are mounted." msgstr "Nenhuma unidade global está montada." diff --git a/src/sd_ipc/ipc_client.c b/src/sd_ipc/ipc_client.c index 0f65214..49fe151 100644 --- a/src/sd_ipc/ipc_client.c +++ b/src/sd_ipc/ipc_client.c @@ -505,7 +505,7 @@ static sd_err open_pipe(const wchar_t *name, sd_ipc_endpoint ep, int start_if_mi HANDLE *out) { uint64_t deadline = sd_tick_ms() + timeout_ms; - int spawned = 0, missing = 0; + int spawned = 0, missing = 0, denied = 0; HANDLE ready = NULL; *out = INVALID_HANDLE_VALUE; for (;;) { @@ -526,8 +526,10 @@ static sd_err open_pipe(const wchar_t *name, sd_ipc_endpoint ep, int start_if_mi WaitNamedPipeW(name, (DWORD)SD_MIN(deadline - now, 1000)); continue; } - if (gle != ERROR_FILE_NOT_FOUND) + if (gle != ERROR_FILE_NOT_FOUND) { + denied = gle == ERROR_ACCESS_DENIED; /* the pipe is there but its DACL refuses us (2.2) */ break; + } if (!start_if_missing || ep != SD_IPC_AGENT) { if (++missing < 3) { /* ride out an instance turnover on the server */ Sleep(20); @@ -552,6 +554,8 @@ static sd_err open_pipe(const wchar_t *name, sd_ipc_endpoint ep, int start_if_mi Sleep(50); } if (ready) CloseHandle(ready); + if (denied) + return SD_E_ACCESS_DENIED; /* report the refusal as access_denied (03 §6.2, exit 9), not as "not running" */ return spawned ? SD_E_TIMEOUT : SD_E_NOT_FOUND; } diff --git a/tests/apps/main.c b/tests/apps/main.c index a7d164e..b3cc0ee 100644 --- a/tests/apps/main.c +++ b/tests/apps/main.c @@ -9,6 +9,7 @@ SD_TEST_DECL(cli_parse_profile_hostkey_status); SD_TEST_DECL(cli_parse_hostkey_add_machine); SD_TEST_DECL(cli_parse_secret_proxy); SD_TEST_DECL(smoke_hostkey_add_machine); +SD_TEST_DECL(smoke_service_list_denied_vs_unavailable); SD_TEST_DECL(smoke_hostkey_add_offers); SD_TEST_DECL(smoke_version_help_usage); SD_TEST_DECL(smoke_mount_prompts_and_errors); @@ -71,6 +72,7 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(cli_parse_hostkey_add_machine), SD_TEST_ENTRY(cli_parse_secret_proxy), SD_TEST_ENTRY(smoke_hostkey_add_machine), + SD_TEST_ENTRY(smoke_service_list_denied_vs_unavailable), SD_TEST_ENTRY(smoke_version_help_usage), SD_TEST_ENTRY(smoke_mount_prompts_and_errors), SD_TEST_ENTRY(smoke_mount_m2_flags_and_prompts), diff --git a/tests/apps/test_smoke.c b/tests/apps/test_smoke.c index b5cee14..c85382c 100644 --- a/tests/apps/test_smoke.c +++ b/tests/apps/test_smoke.c @@ -660,6 +660,39 @@ SD_TEST(smoke_hostkey_add_machine) SetEnvironmentVariableW(L"SHELLDRIVE_TEST_ELEVATED", NULL); } +/* service list (03 §6.2, TM): without the service the CLI reports "not running or not installed" (exit 8); a service + pipe that exists but refuses this logon session (the DACL admits interactive sessions only, 03 §2.2) must come back + as access denied (exit 9), never as "not running" — a batch logon of a service_access member used to read exactly + that way. */ +SD_TEST(smoke_service_list_denied_vs_unavailable) +{ + static run_result r; + wchar_t sid[192], name[512]; + DWORD session = 0; + PSECURITY_DESCRIPTOR sd = NULL; + SECURITY_ATTRIBUTES sa; + HANDLE squat = INVALID_HANDLE_VALUE; + SD_REQUIRE(setup()); + EXPECT(RUN(L"service list", NULL) == 8 && strstr(r.err, "not running or not installed") != NULL, + "service list without the service is unavailable"); + SD_REQUIRE(sd_ipc_current_identity(sid, SD_COUNTOF(sid), &session) == SD_OK); + SD_REQUIRE(sd_ipc_pipe_name(SD_IPC_SERVICE, sid, session, name, SD_COUNTOF(name)) == SD_OK); + SD_REQUIRE(ConvertStringSecurityDescriptorToSecurityDescriptorW(L"D:P(A;;GA;;;SY)", SDDL_REVISION_1, &sd, NULL)); + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = sd; + sa.bInheritHandle = FALSE; + squat = CreateNamedPipeW(name, PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED, PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, + 4, 4096, 4096, 0, &sa); + LocalFree(sd); + if (squat == INVALID_HANDLE_VALUE) + return; /* the name could not be claimed (a leftover server): the unit suite covers the mapping */ + EXPECT(RUN(L"service list", NULL) == 9 && strstr(r.err, "denied") != NULL, + "a refused service connect is access denied"); + EXPECT(RUN(L"--json service list", NULL) == 9 && strstr(r.out, "\"code\":\"access_denied\"") != NULL && + strstr(r.out, "\"exit_code\":9") != NULL, "refused service connect json is access_denied/9"); + CloseHandle(squat); +} + /* diag bench (03 §6.1, 02 §11) through the real CLI: usage errors, a drive that is not there, and short runs into a folder of the test (SHELLDRIVE_TEST_BENCH_ROOT, test builds only). Timings are never asserted: the reports must carry the structure and the numbers, and the folder must be empty again afterwards. */ diff --git a/tests/unit/sd_ipc/main.c b/tests/unit/sd_ipc/main.c index ac80788..a0b6629 100644 --- a/tests/unit/sd_ipc/main.c +++ b/tests/unit/sd_ipc/main.c @@ -19,6 +19,8 @@ SD_TEST_DECL(ipc_instance_quota_and_rate_limit); SD_TEST_DECL(ipc_first_instance_detects_squatting); SD_TEST_DECL(ipc_client_rejects_foreign_owner); SD_TEST_DECL(ipc_client_rejects_squatted_pipe); +SD_TEST_DECL(ipc_connect_denied_is_access_denied); +SD_TEST_DECL(service_pipe_sd_matches_design); SD_TEST_DECL(ipc_connect_without_server); SD_TEST_DECL(ipc_instance_turnover_under_load); SD_TEST_DECL(m3_topic_validation); @@ -47,6 +49,8 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(ipc_first_instance_detects_squatting), SD_TEST_ENTRY(ipc_client_rejects_foreign_owner), SD_TEST_ENTRY(ipc_client_rejects_squatted_pipe), + SD_TEST_ENTRY(ipc_connect_denied_is_access_denied), + SD_TEST_ENTRY(service_pipe_sd_matches_design), SD_TEST_ENTRY(ipc_connect_without_server), SD_TEST_ENTRY(ipc_instance_turnover_under_load), SD_TEST_ENTRY(m3_topic_validation), diff --git a/tests/unit/sd_ipc/test_ipc.c b/tests/unit/sd_ipc/test_ipc.c index 9981ab7..7405229 100644 --- a/tests/unit/sd_ipc/test_ipc.c +++ b/tests/unit/sd_ipc/test_ipc.c @@ -2,6 +2,7 @@ * checks of 03 §2.2 (TM-R25/R26 basics). Every test uses its own instance suffix so it never * touches a running agent. */ #include +#include #include #include @@ -609,6 +610,124 @@ SD_TEST(ipc_client_rejects_squatted_pipe) CloseHandle(squat); } +/* A pipe that exists but whose DACL refuses us (2.2, here a squatter granting only SYSTEM): the connect fails with + access_denied, not not_found, so the CLI reports it as access denied instead of "not running or not installed" + (03 §6.2). */ +SD_TEST(ipc_connect_denied_is_access_denied) +{ + wchar_t sid[192], name[512]; + DWORD session = 0; + PSECURITY_DESCRIPTOR sd = NULL; + SECURITY_ATTRIBUTES sa; + HANDLE squat; + sd_ipc_conn *c = NULL; + unique_suffix(); + SD_REQUIRE(sd_ipc_current_identity(sid, SD_COUNTOF(sid), &session) == SD_OK); + SD_REQUIRE(sd_ipc_pipe_name(SD_IPC_AGENT, sid, session, name, SD_COUNTOF(name)) == SD_OK); + SD_REQUIRE(ConvertStringSecurityDescriptorToSecurityDescriptorW(L"D:P(A;;GA;;;SY)", SDDL_REVISION_1, &sd, NULL)); + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = sd; + sa.bInheritHandle = FALSE; + squat = CreateNamedPipeW(name, PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED | FILE_FLAG_FIRST_PIPE_INSTANCE, + PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE, 4, 4096, 4096, 0, &sa); + LocalFree(sd); + SD_REQUIRE(squat != INVALID_HANDLE_VALUE); + SD_CHECK(connect_client(0, 2000, &c) == SD_E_ACCESS_DENIED); + SD_CHECK(c == NULL); + CloseHandle(squat); +} + +/* The service pipe's security descriptor matches the design (03 §2.2, carried in 05): SYSTEM and Administrators + full control, interactive users read/write, network logons denied, protected DACL. The logon-type SIDs are the + boundary: a member of service_access in a batch (S-1-5-3) or service logon is refused at the pipe on purpose, + and the service then authorizes every method per profile. The DACL is checked structurally (ACE types, SIDs and + access masks in order); the SDDL text round trip does not keep the generic masks. */ +SD_TEST(service_pipe_sd_matches_design) +{ + /* Same layout as ACCESS_ALLOWED/DENIED_ACE; named locally because the SDK headers spell the + mask member differently (Mask on MinGW, AccessMask on MSVC). */ + typedef struct test_ace { + ACE_HEADER Header; + DWORD Mask; + DWORD SidStart; + } test_ace; + srv_ctx ctx; + sd_ipc_server *s = NULL; + wchar_t sid[192], name[512], *own = NULL; + DWORD session = 0, got = 0, i; + BYTE tbuf[64]; + HANDLE tok = NULL, h; + PSECURITY_DESCRIPTOR sd = NULL; + PACL dacl = NULL; + BOOL present = FALSE, defaulted = FALSE; + WORD ctl = 0; + DWORD rev = 0; + ACL_SIZE_INFORMATION sz; + PSID nu = NULL, sy = NULL, ba = NULL, iu = NULL; + SID_IDENTIFIER_AUTHORITY nt = SECURITY_NT_AUTHORITY; + unique_suffix(); + /* The owner is replaced with ours (test hook, tests/unit/sd_svc does the same): a client may then verify the + pipe, and this process can read the DACL back as its owner. The DACL is the design one either way. */ + SD_REQUIRE(OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &tok)); + SD_REQUIRE(GetTokenInformation(tok, TokenUser, tbuf, sizeof(tbuf), &got)); + CloseHandle(tok); + SD_REQUIRE(ConvertSidToStringSidW(((TOKEN_USER *)tbuf)->User.Sid, &own) != 0 && own != NULL); + sd_ipc_test_set_service_owner(own); + LocalFree(own); + SD_REQUIRE(sd_ipc_current_identity(sid, SD_COUNTOF(sid), &session) == SD_OK); + SD_REQUIRE(sd_ipc_pipe_name(SD_IPC_SERVICE, sid, session, name, SD_COUNTOF(name)) == SD_OK); + memset(&ctx, 0, sizeof(ctx)); + SD_REQUIRE(sd_ipc_server_start(SD_IPC_SERVICE, handler, &ctx, &s) == SD_OK); + h = CreateFileW(name, READ_CONTROL, 0, NULL, OPEN_EXISTING, + SECURITY_SQOS_PRESENT | SECURITY_IDENTIFICATION, NULL); + SD_REQUIRE(h != INVALID_HANDLE_VALUE); + SD_REQUIRE(GetSecurityInfo(h, SE_KERNEL_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &dacl, NULL, &sd) == + ERROR_SUCCESS); + SD_REQUIRE(GetSecurityDescriptorControl(sd, &ctl, &rev)); + SD_CHECK((ctl & SE_DACL_PROTECTED) != 0); /* protected DACL, no inherited ACEs */ + SD_REQUIRE(GetSecurityDescriptorDacl(sd, &present, &dacl, &defaulted) && present && dacl != NULL); + SD_REQUIRE(AllocateAndInitializeSid(&nt, 1, SECURITY_NETWORK_RID, 0, 0, 0, 0, 0, 0, 0, &nu)); + SD_REQUIRE(AllocateAndInitializeSid(&nt, 1, SECURITY_LOCAL_SYSTEM_RID, 0, 0, 0, 0, 0, 0, 0, &sy)); + SD_REQUIRE(AllocateAndInitializeSid(&nt, 2, SECURITY_BUILTIN_DOMAIN_RID, DOMAIN_ALIAS_RID_ADMINS, + 0, 0, 0, 0, 0, 0, &ba)); + SD_REQUIRE(AllocateAndInitializeSid(&nt, 1, SECURITY_INTERACTIVE_RID, 0, 0, 0, 0, 0, 0, 0, &iu)); + SD_REQUIRE(GetAclInformation(dacl, &sz, sizeof(sz), AclSizeInformation)); + SD_CHECK_EQ_INT((int)sz.AceCount, 4); + for (i = 0; i < sz.AceCount && i < 4; i++) { + void *ace = NULL; + test_ace *a; + BYTE want_type; + PSID want_sid; + DWORD want_mask; + const char *what; + wchar_t *got_sid = NULL; + SD_REQUIRE(GetAce(dacl, i, &ace)); + a = (test_ace *)ace; + /* design order: deny NU first, then the allows (03 §2.2 SDDL, verbatim); the SDDL parser maps the generic + masks through the file/pipe generic mapping, so GA is FILE_ALL_ACCESS and GRGW is read|write generic */ + if (i == 0) { want_type = ACCESS_DENIED_ACE_TYPE; want_sid = nu; want_mask = FILE_ALL_ACCESS; what = "deny NU GA"; } + else if (i == 1) { want_type = ACCESS_ALLOWED_ACE_TYPE; want_sid = sy; want_mask = FILE_ALL_ACCESS; what = "allow SY GA"; } + else if (i == 2) { want_type = ACCESS_ALLOWED_ACE_TYPE; want_sid = ba; want_mask = FILE_ALL_ACCESS; what = "allow BA GA"; } + else { want_type = ACCESS_ALLOWED_ACE_TYPE; want_sid = iu; + want_mask = FILE_GENERIC_READ | FILE_GENERIC_WRITE; what = "allow IU GRGW"; } + (void)ConvertSidToStringSidW((PSID)&a->SidStart, &got_sid); + if (!(a->Header.AceType == want_type && got_sid != NULL && EqualSid((PSID)&a->SidStart, want_sid) && + a->Mask == want_mask)) + sd_test_fail(sd_t_, __FILE__, __LINE__, "ace %lu, want %s (mask %08lx): got type=%u mask=%08lx sid=%ls", + (unsigned long)i, what, (unsigned long)want_mask, (unsigned)a->Header.AceType, + (unsigned long)a->Mask, got_sid ? got_sid : L"?"); + LocalFree(got_sid); + } + FreeSid(nu); + FreeSid(sy); + FreeSid(ba); + FreeSid(iu); + CloseHandle(h); + LocalFree(sd); + sd_ipc_test_set_service_owner(NULL); + sd_ipc_server_stop(s); +} + SD_TEST(ipc_connect_without_server) { sd_ipc_conn *c = NULL; From 3b9f032c89c6d5c71e1bb45570f2723cbcf1e346 Mon Sep 17 00:00:00 2001 From: FacuM Date: Sat, 31 Oct 2026 01:21:56 -0300 Subject: [PATCH 7/9] tests: keep the SCM denial smoke result off the stack run_result is 32 KiB; as a local it trips C6262 under /analyze (x64-analysis), like the other smoke tests it is static. --- tests/apps/test_smoke.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/apps/test_smoke.c b/tests/apps/test_smoke.c index c85382c..93a52c4 100644 --- a/tests/apps/test_smoke.c +++ b/tests/apps/test_smoke.c @@ -1225,7 +1225,7 @@ SD_TEST(smoke_location_verbs) whose SCM call is refused gets the SCM message with the Win32 code (exit 1). No real service is touched. */ SD_TEST(smoke_service_scm_denials) { - run_result r; + static run_result r; /* 32 KiB: off the stack */ SD_REQUIRE(setup()); SetEnvironmentVariableW(L"SHELLDRIVE_TEST_SCM", L"denied"); From 9b26bfc4259c9d407ca0d5046c621fea263b4116 Mon Sep 17 00:00:00 2001 From: FacuM Date: Sat, 31 Oct 2026 02:35:20 -0300 Subject: [PATCH 8/9] service: let a new mount replace a failed record of the same profile The live lab run found the boot autostart mount failing (its host key was added later) and then `service mount` of the same profile answering already_mounted (exit 10): the duplicate check matched the final failed record, so the drive could not come up without an unmount first. A final failed record holds no live drive (03 1.11: failed is terminal), so a new mount of its profile now detaches it under g.lock and queues its release to the control thread, the way the tick reaps an unmounted record; the new mount gets the same letter. A live record still answers already_mounted. Tests: a failed autostart mount is replaced by a new mount (new id, same target) and a live one is still refused. wait_state compared the state string after freeing the reply it pointed into (it only bit with a NULL `last` in Debug); it now compares first. The boot autostart tests register their own host-key trust so they also pass when run alone. --- apps/shelldrive-host/service.c | 55 +++++++++++++++++---- tests/unit/sd_svc/main.c | 2 + tests/unit/sd_svc/test_service.c | 82 ++++++++++++++++++++++++++++---- 3 files changed, 122 insertions(+), 17 deletions(-) diff --git a/apps/shelldrive-host/service.c b/apps/shelldrive-host/service.c index c5a7e44..02a620f 100644 --- a/apps/shelldrive-host/service.c +++ b/apps/shelldrive-host/service.c @@ -267,7 +267,9 @@ static void svc_log_info(const char *fmt, ...) /* ---- work queue (the control thread runs it) ---- */ -static void svc_enqueue(void (*fn)(void *), void *arg) +/* Appends a job for the control thread; g.lock held (svc_mount_queue detaches a failed record and queues its + release in one critical section, so a concurrent mount of the same profile cannot slip in between). */ +static void svc_enqueue_locked(void (*fn)(void *), void *arg) { svc_work *w = (svc_work *)sd_alloc(sizeof(*w)); if (w == NULL) @@ -275,13 +277,18 @@ static void svc_enqueue(void (*fn)(void *), void *arg) w->next = NULL; w->fn = fn; w->arg = arg; - sd_mutex_lock(&g.lock); if (g.qt) g.qt->next = w; else g.qh = w; g.qt = w; - sd_mutex_unlock(&g.lock); SetEvent(g.wake.h); } +static void svc_enqueue(void (*fn)(void *), void *arg) +{ + sd_mutex_lock(&g.lock); + svc_enqueue_locked(fn, arg); + sd_mutex_unlock(&g.lock); +} + static void svc_post(void *ctx, void (*fn)(void *), void *arg) { (void)ctx; @@ -2243,6 +2250,14 @@ static void reap_mount(svc_mount *r) sd_free(r); } +/* Control thread, from svc_mount_queue: releases a final failed record that a new mount of the same profile + detached from the table. The teardown is the tick's own reap after an unmount: the sd_mount already ended + (or was never created), and prompts of the record, if any, are dropped with it. */ +static void job_reap_failed(void *arg) +{ + reap_mount((svc_mount *)arg); +} + /* Control-thread timer: reaps finished mounts, expires open prompts (2.5). */ static void svc_tick(void) { @@ -2514,9 +2529,12 @@ static sd_err handle_status(sd_ipc_client *c, const sd_json *params, sd_json_wri } /* The one code path that brings a global mount up (2.4 "mount"; 1.8 and 3.4.1 boot autostart): finds the loaded - service profile `key`, refuses a duplicate, picks the drive letter and publishes the record, then queues the - start job for the control thread. Takes g.lock. Returns SD_OK and fills out_id and out_target; on failure - returns the sd_err of the reply, with *code set (NULL for the plain errors) and detail filled. */ + service profile `key`, refuses a duplicate of a live mount, picks the drive letter and publishes the record, + then queues the start job for the control thread. A final failed record of the profile is no duplicate: `failed` + is the terminal state of a mount (03 1.11), so it holds no live drive and the record is detached and released, + its letter going to the new mount (03 1.11 keeps a failed record only until it is unmounted or replaced). Takes + g.lock. Returns SD_OK and fills out_id and out_target; on failure returns the sd_err of the reply, with *code + set (NULL for the plain errors) and detail filled. */ static sd_err svc_mount_queue(const char *key, const char *tgt, char out_id[16], char out_target[4], const char **code, char detail[200]) { @@ -2541,13 +2559,34 @@ static sd_err svc_mount_queue(const char *key, const char *tgt, char out_id[16], _snprintf_s(detail, 200, _TRUNCATE, "%s: %s", bad ? bad : "profile", why ? why : "refused"); return SD_E_INVALID_PARAM; } - for (i = 0; i < SVC_MAX_MOUNTS; i++) - if (g.mounts[i] && strcmp(g.mounts[i]->prof->p.id, src->p.id) == 0) { + { + svc_mount *failed = NULL; /* a final failed record of this profile, detached for release */ + for (i = 0; i < SVC_MAX_MOUNTS; i++) { + svc_mount *old = g.mounts[i]; + if (old == NULL || strcmp(old->prof->p.id, src->p.id) != 0) + continue; + /* `already_mounted` protects a live mount (03 6.2 conflict, exit 10); a failed record of the profile never + blocks a new mount (03 1.11: failed is terminal, the offline state is the one that keeps the drive). The + record goes the way the tick releases an unmounted record, only now: detached here under g.lock, released + by the control thread. A live record, or a failed one whose sd_mount has not ended yet, still answers + already_mounted. */ + if (failed == NULL && old->final && old->st == SD_MOUNT_FAILED && !old->stop_req && + (old->m == NULL || sd_mount_is_final(old->m))) { + failed = old; + g.mounts[i] = NULL; + old->stop_req = 1; + continue; + } sd_mutex_unlock(&g.lock); *code = "already_mounted"; strcpy_s(detail, 200, "this profile is already mounted"); return SD_E_EXISTS; } + if (failed != NULL) { + SD_LOGI("svc", "mount %s replaced by a new mount of the same profile", failed->id); + svc_enqueue_locked(job_reap_failed, failed); /* control thread: destroy r->m, drop the record */ + } + } copy = (svc_prof *)sd_alloc(sizeof(*copy)); if (copy == NULL) { sd_mutex_unlock(&g.lock); diff --git a/tests/unit/sd_svc/main.c b/tests/unit/sd_svc/main.c index ae1dc04..69fcac7 100644 --- a/tests/unit/sd_svc/main.c +++ b/tests/unit/sd_svc/main.c @@ -47,6 +47,7 @@ SD_TEST_DECL(svc_status_names_a_refused_machine_folder_for_admins_only); SD_TEST_DECL(svc_repair_moves_the_folder_aside_and_the_service_recovers); SD_TEST_DECL(svc_autostart_boot_is_mounted_at_start_and_logon_is_not); SD_TEST_DECL(svc_autostart_reload_does_not_double_mount); +SD_TEST_DECL(svc_autostart_failed_mount_is_replaced_by_a_new_mount); SD_TEST_DECL(scm_install_registers_the_service_in_order); SD_TEST_DECL(scm_install_skips_the_sid_type_before_vista); SD_TEST_DECL(scm_install_refuses_without_elevation); @@ -148,6 +149,7 @@ static const sd_test_case k_tests[] = { SD_TEST_ENTRY(svc_repair_moves_the_folder_aside_and_the_service_recovers), SD_TEST_ENTRY(svc_autostart_boot_is_mounted_at_start_and_logon_is_not), SD_TEST_ENTRY(svc_autostart_reload_does_not_double_mount), + SD_TEST_ENTRY(svc_autostart_failed_mount_is_replaced_by_a_new_mount), SD_TEST_ENTRY(scm_install_registers_the_service_in_order), SD_TEST_ENTRY(scm_install_skips_the_sid_type_before_vista), SD_TEST_ENTRY(scm_install_refuses_without_elevation), diff --git a/tests/unit/sd_svc/test_service.c b/tests/unit/sd_svc/test_service.c index 5d8d22c..b3792de 100644 --- a/tests/unit/sd_svc/test_service.c +++ b/tests/unit/sd_svc/test_service.c @@ -488,13 +488,14 @@ static int wait_state(sd_ipc_conn *c, const char *mount_id, const char *want, ui sd_json *res = call(c, "status", params, &err, &e); if (res) { const char *st = sd_json_get_str(res, "state"); + int hit = st != NULL && strcmp(st, want) == 0; /* compared before res is freed: st points into it */ if (last) { if (*last) sd_json_free(*last); *last = res; } else { sd_json_free(res); } - if (st && strcmp(st, want) == 0) + if (hit) return 1; } else if (strcmp(err.code, "not_mounted") == 0 && strcmp(want, "gone") == 0) { return 1; @@ -2098,7 +2099,6 @@ static int start_boot_core(svc_opts *o, const wchar_t *root) memset(o, 0, sizeof(*o)); o->backend = fake_backend_hostkeys(); o->machine_root = root; - o->open_log = 0; o->owner_trusted = owner_trust_all; o->test_all_admin = 1; o->dacl_extra_ace = L"(A;OICI;FA;;;OW)"; @@ -2210,12 +2210,12 @@ static int make_boot_root(const wchar_t *root, wchar_t *key, size_t key_cch) return write_file(key, "fixture-key", 11); } -/* A service profile of the boot fixture: the shared base, the identity under root's own service folder and the - [mount] autostart value. */ +/* A service profile of the boot fixture: the shared base on `host`, the identity under root's own service folder + and the [mount] autostart value. */ static int write_boot_profile(const wchar_t *root, const wchar_t *file, const char *id, const char *name, - const char *autostart) + const char *host, const char *autostart) { - sd_profile p = base_profile(id, name, "trusted.example"); + sd_profile p = base_profile(id, name, host); char utf8[MAX_PATH * 3]; wchar_t key[MAX_PATH]; _snwprintf_s(key, MAX_PATH, _TRUNCATE, L"%s\\service\\keys\\key1.ppk", root); @@ -2235,8 +2235,9 @@ SD_TEST(svc_autostart_boot_is_mounted_at_start_and_logon_is_not) int n_boot = -1, n_logon = -1; char mid[16] = ""; SD_REQUIRE(make_boot_root(root, key, SD_COUNTOF(key))); - SD_REQUIRE(write_boot_profile(root, L"a-bootauto.ini", T_ID_BOOTAUTO, "bootauto", "boot")); - SD_REQUIRE(write_boot_profile(root, L"b-bootlogon.ini", T_ID_BOOTLOGON, "bootlogon", "logon")); + SD_REQUIRE(write_boot_profile(root, L"a-bootauto.ini", T_ID_BOOTAUTO, "bootauto", "trusted.example", "boot")); + SD_REQUIRE(write_boot_profile(root, L"b-bootlogon.ini", T_ID_BOOTLOGON, "bootlogon", "trusted.example", "logon")); + fake_backend_pre_trust("trusted.example"); /* own trust: the test also runs alone (--filter) */ SD_REQUIRE(start_boot_core(&o, root)); SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); /* the service mounts the boot profile by itself; autostart = logon is the agent's business (3.4.1) */ @@ -2264,7 +2265,8 @@ SD_TEST(svc_autostart_reload_does_not_double_mount) int n_boot = -1, n_logon = -1; char mid[16] = "", mid2[16] = ""; SD_REQUIRE(make_boot_root(root, key, SD_COUNTOF(key))); - SD_REQUIRE(write_boot_profile(root, L"a-bootauto.ini", T_ID_BOOTAUTO, "bootauto", "boot")); + SD_REQUIRE(write_boot_profile(root, L"a-bootauto.ini", T_ID_BOOTAUTO, "bootauto", "trusted.example", "boot")); + fake_backend_pre_trust("trusted.example"); /* own trust: the test also runs alone (--filter) */ SD_REQUIRE(start_boot_core(&o, root)); SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); SD_REQUIRE(wait_boot_list(c, &n_boot, &n_logon, mid, sizeof(mid))); @@ -2287,3 +2289,65 @@ SD_TEST(svc_autostart_reload_does_not_double_mount) SD_CHECK_EQ_INT(g_boot_rc, 0); RemoveDirectoryW(root); } + +/* The live lab defect of the boot autostart: the start mount runs before the machine host key exists and ends + failed; its record stays for list and status (03 1.11). When the key is added, a mount of the same profile must + replace the failed record (a new mount id, the same letter) instead of answering already_mounted, while a live + mount of the profile still answers already_mounted. */ +SD_TEST(svc_autostart_failed_mount_is_replaced_by_a_new_mount) +{ + static svc_opts o; + wchar_t root[MAX_PATH], key[MAX_PATH]; + sd_ipc_conn *c = NULL; + sd_ipc_error err; + sd_err e; + sd_json *res, *st = NULL; + int n_boot = -1, n_logon = -1; + uint32_t waited = 0; + char mid[16] = "", mid2[16] = "", tgt1[4] = "", tgt2[4] = ""; + SD_REQUIRE(make_boot_root(root, key, SD_COUNTOF(key))); + /* no key for late.example in the machine store yet: the autostart mount fails (TOFU refused, 2.6 rule 2) */ + SD_REQUIRE(write_boot_profile(root, L"a-bootauto.ini", T_ID_BOOTAUTO, "bootauto", "late.example", "boot")); + SD_REQUIRE(start_boot_core(&o, root)); + SD_REQUIRE(connect_as("testadmin", 0, &c) == SD_OK); + for (;;) { + res = call(c, "list", NULL, &err, &e); + if (res != NULL) { + boot_list_scan(res, &n_boot, &n_logon, mid, sizeof(mid)); + sd_json_free(res); + if (mid[0] != 0) + break; + } + SD_REQUIRE(waited < 10000); + Sleep(100); + waited += 100; + } + SD_CHECK(wait_state(c, mid, "failed", 10000, &st)); + if (st) { + copy_str(tgt1, sizeof(tgt1), sd_json_get_str(st, "target")); + json_drop(&st); + } + /* the admin adds the host key and mounts again: the failed record does not answer already_mounted */ + fake_backend_pre_trust("late.example"); + res = mount_profile(c, T_ID_BOOTAUTO, &err, &e); + SD_CHECK(res != NULL); + if (res != NULL) { + copy_str(mid2, sizeof(mid2), sd_json_get_str(res, "mount_id")); + copy_str(tgt2, sizeof(tgt2), sd_json_get_str(res, "target")); + sd_json_free(res); + } + SD_CHECK(mid2[0] != 0 && strcmp(mid2, mid) != 0); /* a new mount id */ + SD_CHECK_STR(tgt2, tgt1); /* the same letter comes back */ + SD_CHECK(wait_state(c, mid, "gone", 10000, NULL)); /* the failed record was released */ + SD_CHECK(wait_state(c, mid2, "mounted", 20000, NULL)); /* the drive is up without an unmount */ + /* a live mount of the profile is still refused */ + res = mount_profile(c, T_ID_BOOTAUTO, &err, &e); + SD_CHECK(res == NULL); + SD_CHECK_STR(err.code, "already_mounted"); + SD_CHECK(unmount_id(c, mid2)); + SD_CHECK(wait_state(c, mid2, "gone", 10000, NULL)); + sd_ipc_close(c); + stop_boot_core(); + SD_CHECK_EQ_INT(g_boot_rc, 0); + RemoveDirectoryW(root); +} From 0ec4afa1c38231f88ddd20ecd3e1216cbcc51ae5 Mon Sep 17 00:00:00 2001 From: FacuM Date: Sat, 31 Oct 2026 03:13:03 -0300 Subject: [PATCH 9/9] docs: record the M5 gap fixes and the second live run The four gaps of the first live run (boot autostart, the batch-session service list report, the unnamed profile, the SCM access-denied message) are fixed and checked live on 2026-10-31, as is the failed-record replacement that run found. --- docs/dev/m5-notes.md | 43 +++++++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/docs/dev/m5-notes.md b/docs/dev/m5-notes.md index b24f56f..0052983 100644 --- a/docs/dev/m5-notes.md +++ b/docs/dev/m5-notes.md @@ -1334,26 +1334,33 @@ And in `unit.webdav`, against a real loopback server with real SSPI (the test us install/start as SYSTEM (after `58d6b68`), machine profile load and host key, the global mount (Event 1100), G1 batch and B1/B2 console logons (two live sessions), G5 (1 GiB write 74.6 s, read 17.6 s, SHA-256 match), G7 including the drop-member mutation, G3 stop/start and VM save/resume, and uninstall. Per-item results with - the quoted log lines are in the 2026-10-30 section of `docs/dev/lab-results.md`. The run left these product - gaps, to triage: - - `autostart = boot` is not applied by the service. After every reboot `shelldrive service list` was empty - until a manual `service mount` (03 §3.4.1 says the service applies it); both phase-B reboots log - `pre-logon-state OK … list=[No global drives are mounted.]` and `remount OK remounted by hand (boot - autostart absent)`. Product gap; it blocks the pre-logon (G6) claim for Dokany. - - A standard user's `shelldrive service list` exits 8, "The ShellDrive service is not running or not - installed.", while the service runs: the same user gets rc=0 and `Z: GlobalDokan mounted` from its console - session but rc=8 from a batch session. Pipe access for non-admin members, or the CLI mis-mapping access - denied — triage. - - `service mount ` for an unknown profile prints "There is no profile named ." — the name placeholder - is empty. - - `scm_error` (`apps/shelldrive/cli_service.c:280`) maps every ERROR_ACCESS_DENIED to "needs an elevated - console" — misleading when the caller is elevated; that message is what the pre-`58d6b68` install attempts - reported as they rolled back (exit 9). + the quoted log lines are in the 2026-10-30 section of `docs/dev/lab-results.md`. The run left four product + gaps; all four are fixed on this branch and the fixes were checked by a second live run (2026-10-31, log + `D:\sdlab\logs\m5-20261031-025507.log`): + - Fixed (`5780848`): `autostart = boot` was not applied by the service (03 §3.4.1). The service now mounts + those profiles after loading them (start, config reload, data-folder repair) through the same queue as a + `mount` request. Live: both phase-B reboots log `pre-logon-state OK … list=[Z: GlobalDokan mounted]` and + `boot-autostart OK mounted by the service at boot, 33 s after boot` (35 s on the second), so the pre-logon + (G6) claim now holds for Dokany; a service stop unmounts Z: cleanly and the restart brings it back. + - Fixed (`b478b45`): a standard user's `shelldrive service list` exited 8 "not running or not installed" + from a batch session. 03 §2.2 grants the service pipe to interactive users only + (`O:SYD:P(D;;GA;;;NU)(A;;GA;;;SY)(A;;GA;;;BA)(A;;GRGW;;;IU)`), so a batch logon is refused by design; the + defect was the report. The CLI now says "Access to the ShellDrive service is denied." with exit 9, and + exit 8 only when the service is really absent. Live: `service_list_rc=9` from the batch session, + `list-after-uninstall rc=8`. + - Fixed (`7f4538c`): `service mount|unmount|status ` errors now name the profile ("There is no profile + named nosuch."). + - Fixed (`7f4538c`): `scm_error` tells a caller that is not elevated (exit 9, "Managing the ShellDrive service + needs an elevated console") from an elevated console whose SCM call was refused (exit 1, with the Win32 + code). + - Found by the second run and fixed (`9b26bfc`): the first autostart mount after install fails until the + machine host key is added, and its final `failed` record then made `service mount` answer already_mounted + (exit 10). A new mount of the profile now replaces a final failed record (03 §1.11: failed is terminal); + a live mount is still refused. Live: phase A `service-mount` brings Z: up after the key is added. - Note only: Get-Volume (MSFT_Volume) does not list the Dokany Z: ("No MSFT_Volume objects found with property 'DriveLetter' equal to 'Z'."); `dir` and Explorer work. - - Not covered by the run: RDP/FUS second interactive session (G2 proper), Server 2022 + Secure Boot (the Gen2 - media problem of lab-results' 2026-10-29 night section), pre-logon mount at boot (blocked by the - `autostart` gap above). + - Not covered by the runs: RDP/FUS second interactive session (G2 proper), Server 2022 + Secure Boot (the + Gen2 media problem of lab-results' 2026-10-29 night section). - Everything in 05 §12.8 that has no code: release roots, crash dumps, Event 3100, the FUS/RDP session list, `--elevated-view`, `settings.ini` checks, discovered-proxy detection. - The per-user identity test (two users, each mapping the other's UNC) and the rest of G1-G7.