access security

[mrkraimer]

Sinisa has asked that pvDatabaseCPP support access security.
In pull request #58 he proposed changes to PRecord and ChannelProviderLocal

I have reread the epics-base access security in
https://epics.anl.gov/base/R3-16/2-docs/AppDevGuide/AccessSecurity.html#x9-3040008
But the implementation is in the epics-base code for accessing DBRecords.
This it is not usable by pvDatabaseCPP.

Here are my initial thoughts.

Access security should be implemented by ChannelProviderLocal with NO changes in PVRecord.
A configuration file would be something like the following:

{ 
  "rules: : [
       {   { "USERS" : "user1 user2 ..."},
           {"HOSTS" : "host1 host2..."},
           { "RECORDS" : "record1,..."},
           { "ACCESS": accessdef}
      },
     ...
  ]
}

Consider the following record

 PVRdouble
        double value
        alarm_t alarm
            int severity
            int status
            string message
        time_t timeStamp
            long secondsPastEpoch
            int nanoseconds
            int userTag

Some example accessdefs are

{ "value" : "WRITE","alarm:"READ","timeStamp":{"userTag":"WRITE"}}

"READ"        // this makes all fields in the records read only
"NONE"  
"WRITE"   this is what is available with no access.

If access security is turned on then when ChannelProviderLocal is started it creates a record that provides support for access security. It would have a method:

 byte accss(PVRecord record.string user, string host)

byte would have one of the values:
0 read access
1 write access
2 no access

Just my initial thoughts.

[sveseli]

Implementation in PR #58 is actually fully functional. When the records are created, appropriate AS structures are initialized, using provided AS group and AS level, and those are used when record is accessed to verify whether record is write-able or not for a given client.

I have verified that it works correctly by implementing corresponding python interfaces, and running instance of PvaServer in python (see https://github.com/epics-base/pvaPy/tree/access-security/documentation). Here is how things work there.

1. Start server and create record with AS, put it into DEFAULT group with level 1:

$ python
Python 3.8.5 (default, Sep  4 2020, 07:30:14) 
[GCC 7.3.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pvaccess import *
>>> s = PvaServer()
pvAccess Server v7.1.2
Active configuration (w/ defaults)
EPICS_PVAS_INTF_ADDR_LIST = 0.0.0.0:5075
...
EPICS_PVAS_PROVIDER_NAMES = local
>>> print(s.isAsActive())
False
>>> pv = PvObject({'x' : INT}, {'x':0})
>>> s.addRecordWithAs('x', pv, 1, 'DEFAULT', None)

2. At this point record is fully write-able. From the second terminal, one can do this:

>>> c = Channel('x')
>>> print(c.get())
structure 
    int x 0

>>> pv = PvObject({'x' : INT}, {'x':1})
>>> c.put(pv)
>>> print(c.get())
structure 
    int x 1

3. Enable access security with a file like this:

$ cat /local/sveseli/EPICS/acl.conf
UAG(uag) {user1,user2}
HAG(hag) {host1,host2}
ASG(DEFAULT) {
    RULE(0,READ)
    RULE(0,WRITE) 
    RULE(1,READ)
    RULE(1,WRITE) {
        UAG(uag)
        HAG(hag)
    }
}

In python:

>>> s.initAs('/local/sveseli/EPICS/acl.conf', '')

4. At this point, the record we created does not allow writes any longer:

>>> c = Channel('x')
>>> print(c.get())
structure 
    int x 1

>>> pv = PvObject({'x' : INT}, {'x':2})
>>> c.put(pv)
Traceback (most recent call last):
  File "", line 1, in 
pvaccess.pvaccess.PvaException: channel x PvaClientPut::put Channel put is not allowed
>>> print(c.get())
structure 
    int x 1
>>>

If I modify the above file to include client's machine and username, re-run initAs() method, the record can be written again using that same client.

[mrkraimer]

I need time to think about this but let me ask:

1. sounds like this makes entire record either having read access or read/write access. Not by field.
2. is there any way for client to see access rules.

I will be gone until tomorrow so no more from me today.

[sveseli]

This is true, there is no access by field in this approach, and there is no way for client to see access rules. I imagine this would cover most use cases, and I think one should certainly have the ability to enable access rule for the entire record without having to worry about individual fields.

If we wanted to add access by field, we would have to add interface similar to the one you suggested, but I am not sure whether preventing read access for a field would be consistent with current practices.

[mrkraimer]

Note that for epics-base records (DBRecords) access security has provisions for access on a field by field basis.

8.6 Database Access Security
8.6.1 Access Level definition
The definition of access level means that a level is defined for each field of each record type.

struct dbFldDes in dbBase.h contains a field as_level. In addition definitions are provided for the symbols ASL0 and ASL1.
Each field description in a record description contains a field with the value ASLx.
The meanings of the Access Security Level definitions are as follows:

ASL0 Assigned to fields used during normal operation
ASL1 Assigned to fields that may be sensitive to change. Permission to access this level implies permission for ASL0.
Most record types assign ASL as follows: The fields VAL, RES (Reset), and CMD use the value ASL0. All other fields use ASL1.

8.6.2 Access Security Group definition
struct dbCommon contains the fields ASG and ASP. ASG (Access Security Group) is a character string. The value can be assigned via a database configuration tool or else a utility could be provided to assign values during ioc initialization. ASP is an access security private field. It contains the address of an ASGMEMBER.

The following are a few examples:

dbCommon.dbd describes the fields that are common to every DBRecord.
The first few are:

field(NAME,DBF_STRING) {
	prompt("Record Name")
	special(SPC_NOMOD)
	size(61)
}
field(DESC,DBF_STRING) {
	prompt("Descriptor")
	promptgroup("10 - Common")
	size(41)
}
field(ASG,DBF_STRING) {
	prompt("Access Security Group")
	promptgroup("10 - Common")
	special(SPC_AS)
	size(29)
}

special(SPC_NOMOD) in NAME states that a put can never change this field.

Field ASG is used to assign a DBRecord to an access security group.

aiRecord.dbd has the definitions for a aiRecord.
Some of the fields are:

field(VAL,DBF_DOUBLE) {
    prompt("Current EGU Value")
    promptgroup("40 - Input")
    asl(ASL0)
    pp(TRUE)
}
field(INP,DBF_INLINK) {
    prompt("Input Specification")
    promptgroup("40 - Input")
    interest(1)
}

asl(ASL0) in VAL states that the default access security for VAL is to allow write access.
If a field does not define asl then the default is only to allow read access.

The ASG field in dbCommon makes me think harder about my original suggestion at the beginning of this issue.
ASG does allows the access security rules to configure ASGs rather then individual records.
For databases with thousands of records, this is important for performance.

But I still think that access security can be implemented by ChannelProviderLocal with NO changes in PVRecord.
ChannelProviderLocal can support the equivalent of ASG.

[anjohnson]

asl(ASL0) in VAL states that the default access security for VAL is to allow write access.
If a field does not define asl then the default is only to allow read access.

That's not quite true, it means that the author of the AS-config file can set different rules for ASL0 fields than for ASL1 fields. The field-level ASL0/ASL1 configuration for IOC records is per record-type, so effectively at class level, not an instance level. The older record types mostly have only their VAL field set as ASL0, but it looks like the authors of the calcout and aSub records didn't really understand the levels properly as they have weird collections of fields marked as ASL0.

I don't think they are really necessary (at least to begin with) but if you do add field-level permissions for the PVRecord I would recommend that each instance be allowed to define whether a particular field should be ASL0 or ASL1, in addition to letting the server-layer know which ASG this record belongs to.

I haven't tried but believe the PVA implementation will actually ignore any attempt to prevent read access to a field, it only supports preventing of write access – please don't spend any time trying to prevent reads, most clients have not been coded to expect this and will probably behave badly when presented with a PV that they can connect to but not access at all.

Then there's also the new question of security for an RPC operation. Can something that accepts RPCs find out who the client is that's making the request?

[mrkraimer]

Maybe it is best, at least for now, is allow:

1. all clients read access.
2. Only some clients located at some hosts have write access.

It does sound like allowing access on a field basis for PVRecords may be a major task.
I want to try to create an example in C++ that does what the Python example above does.

[mrkraimer]

I merged the pull request.
I am then going to create a new branch because I see a way to be able to start access security from the IOC shell.

[mrkraimer]

Note that I did push one change to branch access-security.

I have been thinking a lot about how access security would be used with pvDatabase.

I do have one question.
Should pvDatabase use DEFAULT as the default asGroup or should it use something like PVDATABASE.
That is make the following change to pvDatabase.h (two places)

-        int asLevel = 0, const std::string& asGroup = "DEFAULT");
+        int asLevel = 0, const std::string& asGroup = "PVDATABASE");

The reason I ask is I wonder if there might be conflicts with how access security would be used with DBRecords vs how it would be used with PVRecords.

I have created exampleLink in https://github.com/mrkraimer/testDatabaseCPP.
Don't bother looking at it yet because it is still a work in progress.

I have found that if I start an ioc with a st.cmd like

< envPaths
cd ${TOP}
## Register all support components
dbLoadDatabase("dbd/exampleLink.dbd")
exampleLink_registerRecordDeviceDriver(pdbbase)
## Load record instance
dbLoadRecords("db/ai.db","name=exampleLinkAI");
cd ${TOP}/iocBoot/${IOC}
## START ACCESS SECURITY 
asSetFilename(asYES)
iocInit()
##  Create PVRecords
doubleArrayCreateRecord
exampleMonitorLinkCreateRecord
exampleGetLinkCreateRecord
examplePutLinkCreateRecord

Then access security DOES apply to PVRecords!!

I am now think about how to use this in exampleCPP/database.

[anjohnson]

I don't really see why a site might want to use a different default group for PVRecords than for IOC records, I suspect that might be unusual, but if you wanted to allow for that possibility maybe instead of using a const std::string& asGroup you could provide an API to allow sites that do want it to set the default to be used for subsequently created PVRecords?

[mrkraimer]

I just pushed changes.
See commit messages for details.
Note that the main changes are to the special records.

I have been thinking a lot about how access security could be used in an actual database application.
My main thought is that an application can use PVDatabase to access a PVRecord rather than pvaClient in order to access records which do not allow write access.

In
https://github.com/mrkraimer/testDatabaseCPP

testSpecial is new.

It has two main features:

it tests pvdbcrScalar and pvdbcrScalarArray

After you clone and build testDatabaseCPP just do the following:

In one window:

mrk> pwd
/home/epics7/modules/testDatabaseCPP/testSpecial/iocBoot/testSpecial
mrk> ../../bin/linux-x86_64/testSpecial st.cmd 

In another window:

mrk> pwd
/home/epics7/modules/testDatabaseCPP/testSpecial
mrk> ./testScalar &> temp
mrk> ./testScalarArray &> temp1
mrk> diff temp1 testScalarArrayResult
mrk> diff temp testScalarResult

You should see no differences

It has pvdbcrTestAddRecord

This is used to test pvdbcrAddRecord

This allows the ability to use either pvaClient to access the pvdbcrAddRecord or it can just access it via the PVDatabase,

I want to change exampleCPP/exampleClient to do the same.

[mrkraimer]

I have just pushed changes to exampleClientCPP on branch access-security.
exampleLink is completely rewritten.
Read exampleCPP/documentation/exampleLink.html for a description.
This is in anticipation of access security being implemented for pvDatabase.
If anyone gets time, please clone it and try the new exampleLink.

[mrkraimer]

While working on exampleLink I thought of more changes to pvDatabaseCPP.
Mainly in the special code.
The way it handles #include needs a lot of cleanup.
Also the *Register.cpp code should allow the asGroup to be overridden.

A change should also be made so that if a client is connected when the asGroup is changed then that client will no longer be able to make changes if the rules do not allow it.

[mrkraimer]

I just pushed more changes to branch access-security.
One change I made is to add canRead support for access security.

I updated
https://github.com/mrkraimer/testDatabaseCPP

Start

mrk> pwd
/home/epics7/modules/testDatabaseCPP/testSpecial/iocBoot/testSpecial
mrk> ../../bin/linux-x86_64/testSpecial st.testcanRead 

Then

mrk> pvget doubleNOCLIENT
doubleNOCLIENT Error ChannelGet::get is not allowed

I hope someone gets a chance to try the latest.