Tests fail on macos due to hostname case

[simon-ess]

It seems that the hostname that is passed to the json parser is lowercase, while the hostname that it is compared to is not. This causes tests to fail, see e.g. https://github.com/simon-ess/caPutLog/actions/runs/8685849063/job/23816129382:

/Users/runner/work/caPutLog/caPutLog/test/O.darwin-x86
  caPutJsonLogTest.tap .. 
  not ok  6 - DBF_STRING test - Hostname check
  not ok 15 - DBF_STRING test - Hostname check
  not ok 25 - DBF_LONG test - Hostname check
  not ok 34 - DBF_LONG test - Hostname check
  not ok 44 - DBF_SHORT test - Hostname check
  not ok 53 - DBF_SHORT test - Hostname check
  not ok 63 - DBF_FLOAT test - Hostname check
  not ok 72 - DBF_FLOAT test - Hostname check
  not ok 82 - DBF_FLOAT test - Hostname check
  not ok 91 - DBF_FLOAT test - Hostname check
  not ok 101 - DBF_MENU test - Hostname check
  not ok 110 - DBF_MENU test - Hostname check
  not ok 120 - DBF_ENUM test - Hostname check
  not ok 129 - DBF_ENUM test - Hostname check
  not ok 139 - DBF_DEVICE test - Hostname check
  not ok 148 - DBF_DEVICE test - Hostname check
  not ok 158 - DBF_LINK test - Hostname check
  not ok 167 - DBF_LINK test - Hostname check
  not ok 177 - DBF_DOUBLE array test - Hostname check
  not ok [23](https://github.com/simon-ess/caPutLog/actions/runs/8685849063/job/23816129382#step:7:24)3 - DBF_DOUBLE array test - Hostname check
  not ok 339 - DBF_STRING array test - Hostname check
  not ok 357 - DBF_STRING array test - Hostname check
  not ok 385 - DBF_CHAR array (lso) test - Hostname check
  not ok 394 - DBF_CHAR array (lso) test - Hostname check
  not ok 404 - Metadata test - Hostname check
  not ok 413 - Metadata test - Hostname check
  not ok 4[24](https://github.com/simon-ess/caPutLog/actions/runs/8685849063/job/23816129382#step:7:25) - Metadata test - Hostname check
  not ok 433 - Metadata test - Hostname check
  not ok 444 - Metadata test - Hostname check
  not ok 453 - Metadata test - Hostname check
  not ok 464 - Metadata test - Hostname check
  not ok 473 - Metadata test - Hostname check
  not ok 484 - Metadata test - Hostname check
  not ok 493 - Metadata test - Hostname check
  Failed 34/499 subtests

[simon-ess]

This seems to be due to hostname case: the hostname as passed by the asLib is in lower case, see this commit from 16 years ago from @anjohnson : epics-base/epics-base@e47fc8b

However, gethostname on macos seems to return an upper-case hostname.

What is the reason that we lower-case the host name in the access security code?

[mdavidsaver]

What is the reason that we lower-case the host name in the access security code?

Maybe related to RFC4343.

Personally, I have found this behavior inconvenient as it complicates sharing a single copy of the host name string between multiple ASCLIENTPVT.

[anjohnson]

commit from 16 years ago from @anjohnson : epics-base/epics-base@e47fc8b

That particular commit was to fix an omission from Marty's 2003 commit 8e3b9a0 which did the same thing in 2 other nearby routines. This 2002 tech-talk message may have had something to do with the change.

For whatever reason asLib has ignored the case of hostnames for a long time, so the bug here may be that caPutJsonLogTest.cpp should be doing a case-independent string comparison; epicsString.h does now provide both epicsStrCaseCmp() and epicsStrnCaseCmp() which could be used for that both here and as an update to asLib if @mdavidsaver were so inclined – maybe create a Codethon issue?

[ralphlange]

Back then, it used to be that for Linux machines, the case was not relevant. For Windows machines, however, IIRC even depending on the version of Windows, the case was relevant.

Could have been a CA client issue, where Linux would always send a lowercase hostname and Windows was sending the original configuration value: lower, upper or CamelCase. I remember that for Windows clients, we always had all three spellings in the Access Security host definitions.

[simon-ess]

Since it seems that mac hostnames are (or at least can be) uppercase, it sounds like the solution is to modify this test to ensure that it performs a lower-case comparison as well.

[ralphlange]

Well, if I were nitpicking...
If MacOS treats "HOST" and "host" as two different hostnames, shouldn't we do that, too?
In URLs, hostname case is ignored, by definition. Is our hostname following a standard?

[simon-ess]

From what I can tell, hostnames are case insensitive on a mac, see https://support.apple.com/guide/mac-help/change-computers-local-hostname-mac-mchlp2322/mac

The local hostname is your computer’s name with .local added, and any spaces are replaced with hyphens. For example, if your computer’s name is My Computer, your local hostname is My-Computer.local. Local hostnames aren’t case sensitive, so my-computer.local is the same as My-Computer.local.

[ralphlange]

And even in Apple's own net-world, "My Computer" and "my computer" are the same?

The question is: If two clients are accessing an EPICS server and claim their host names are "My Computer" and "my computer", is it legitimate for AS to treat them as the same machine?

[anjohnson]

I had second thoughts about my suggestion above to change asLib to use epicsStrCaseCmp(), the implementation uses a hash table for fast lookups by name, so the host (and user) names in the ASC file do have to hash to the same value as those provided by the client to match.

To answer @ralphlange's question, to protect against that kind of issue you would need to set the asCheckClientIP flag so the IOC uses the IP address (or DNS name) of the client-provided hostname instead for comparisons.

[simon-ess]

@ralphlange From what I can understand, "my computer" and "My CoMpUtEr" are the same according to a mac. This is probably somewhat akin to how filesystems on os x are "CaSe SeNsItIvE"; the filesystem keeps whatever case you provide for it, but it regards two filenames that only differ by case as being the same filename. This can be a problem sometimes, ask me how I know...

Anyhow, by my read of the asLib code, it seems that the access security code itself would not recognise that difference as the host name is converted to lowercase in any case.

[ralphlange]

So - at least for the file system, Mac and Windows are doing things in the same fashion. Yay.

Converting everything to lowercase makes sense, in that case (bad pun intended). Should apply to both sides of the checking expression, right?

[prjemian]

Right. See https://stackoverflow.com/a/940480, for example.

…

On Mon, Apr 22, 2024, 1:52 AM Ralph Lange ***@***.***> wrote: So - at least for the file system, Mac and Windows are doing things in the same fashion. Yay. Converting everything to lowercase makes sense, in that case (bad pun intended). Should apply to both sides of the checking expression, right? — Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AARMUMGTJKM3LBJQUCAMPLTY6SXRJAVCNFSM6AAAAABGHGGQK2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYGYYTOOJUGE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[simon-ess]

@ralphlange : I agree. Changes have been done here: ad671df as a part of #20 .

[anjohnson]

I have a simpler fix for the hostname comparisons:

diff --git a/test/caPutJsonLogTest.cpp b/test/caPutJsonLogTest.cpp
index b6411d2..89ba586 100644
--- a/test/caPutJsonLogTest.cpp
+++ b/test/caPutJsonLogTest.cpp
@@ -23,6 +23,7 @@
 #include <osiSock.h>
 #include <envDefs.h>
 #include <errlog.h>
+#include <epicsString.h>
 #include <db_access.h>
 #include <db_access_routines.h>
 #include <dbUnitTest.h>
@@ -460,7 +461,7 @@ void commonTests(JsonParser &jsonParser, const char* pvname, const char * testPr
     // Test hostname
     char hostname[1024];
     gethostname(hostname, 1024);
-    testOk(!jsonParser.host.compare(hostname),
+    testOk(!epicsStrCaseCmp(hostname, jsonParser.host.c_str()),
             "%s - %s", testPrefix, "Hostname check");
 
     // Test username

@simon-ess There are other fixes in PR #5 that I want to merge, and I just hit this problem running the tests on my laptop after merging that locally. The above patch fixes the failure of the hostname tests, but other changes in that PR seem to have broken the later self-tests, so I've been investigating them. I plan to do #15, #16 and #17 after that.

[simon-ess]

@anjohnson - that is a nicer fix, I have updated the PR.