bacm address config file no longer generated by service

bacm address config file no longer generated by service

[Mark Haywood]

I see that the ibacm address configuration file,
/etc/rdma/ibacm_addr.cfg, is no longer generated by the ibacm service.
This change in behavior occurred as a result of service hardening
implemented by patch 
https://github.com/linux-rdma/rdma-core/commit/c719619aaa0ec2651edc4e5dee9f5ff81208b185.

The patch hardened the ibacm service by adding the following options to
ibacm.service:

 > ProtectSystem=full
 > ProtectHome=true
 > ProtectHostname=true
 > ProtectKernelLogs=true

ProtectSystem=full setting makes /etc read-only for processes invoked by
the ibacm service.

As a result, the code that generates the address configuration file (if
it does not exist) fails:

static FILE *acm_open_addr_file(void)
{
         FILE *f;

         if ((f = fopen(addr_file, "r")))
                  return f;

         acm_log(0, "notice - generating %s file\n", addr_file);
         if (!(f = popen(acme, "r"))) {
                 acm_log(0, "ERROR - cannot generate %s\n", addr_file);
                 return NULL;
         }

         pclose(f);
         return fopen(addr_file, "r");
}

The popen() code above is supposed to generate the file if it does not
exist (i.e., fails the first fopen()). The popen() now fails as a result
of the ProtectSystem option setting.

ibacm(8) does say "If the address file cannot be found, the ibacm
service will attempt to create one using default values."

I guess my question is simply was this change in behavior expected? Are
admins expected to run ib_acme to generate the address configuration
file prior to starting the ibacm service?

Is the popen() code in acm_open_addr_file() being left in place in case
an admin decides to remove the ProtectSystem option from the
ibacm.service file?

Sorry if there was discussion around this previously that I missed.

Thanks.
Mark

Re: ibacm address config file is no longer generated by service

[Mark Haywood]

Fixed the subject line.

On 1/18/24 4:54 PM, Mark Haywood wrote:
> I see that the ibacm address configuration file,
> /etc/rdma/ibacm_addr.cfg, is no longer generated by the ibacm service.
> This change in behavior occurred as a result of service hardening
> implemented by patch 
> https://github.com/linux-rdma/rdma-core/commit/c719619aaa0ec2651edc4e5dee9f5ff81208b185.
> 
> The patch hardened the ibacm service by adding the following options to
> ibacm.service:
> 
>  > ProtectSystem=full
>  > ProtectHome=true
>  > ProtectHostname=true
>  > ProtectKernelLogs=true
> 
> ProtectSystem=full setting makes /etc read-only for processes invoked by
> the ibacm service.
> 
> As a result, the code that generates the address configuration file (if
> it does not exist) fails:
> 
> static FILE *acm_open_addr_file(void)
> {
>          FILE *f;
> 
>          if ((f = fopen(addr_file, "r")))
>                   return f;
> 
>          acm_log(0, "notice - generating %s file\n", addr_file);
>          if (!(f = popen(acme, "r"))) {
>                  acm_log(0, "ERROR - cannot generate %s\n", addr_file);
>                  return NULL;
>          }
> 
>          pclose(f);
>          return fopen(addr_file, "r");
> }
> 
> The popen() code above is supposed to generate the file if it does not
> exist (i.e., fails the first fopen()). The popen() now fails as a result
> of the ProtectSystem option setting.
> 
> ibacm(8) does say "If the address file cannot be found, the ibacm
> service will attempt to create one using default values."
> 
> I guess my question is simply was this change in behavior expected? Are
> admins expected to run ib_acme to generate the address configuration
> file prior to starting the ibacm service?
> 
> Is the popen() code in acm_open_addr_file() being left in place in case
> an admin decides to remove the ProtectSystem option from the
> ibacm.service file?
> 
> Sorry if there was discussion around this previously that I missed.
> 
> Thanks.
> Mark
> 

Re: bacm address config file no longer generated by service

[Jason Gunthorpe]

On Thu, Jan 18, 2024 at 04:54:16PM -0500, Mark Haywood wrote:
> I see that the ibacm address configuration file,
> /etc/rdma/ibacm_addr.cfg, is no longer generated by the ibacm service.
> This change in behavior occurred as a result of service hardening
> implemented by patch https://github.com/linux-rdma/rdma-core/commit/c719619aaa0ec2651edc4e5dee9f5ff81208b185.
> 
> The patch hardened the ibacm service by adding the following options to
> ibacm.service:
> 
> > ProtectSystem=full
> > ProtectHome=true
> > ProtectHostname=true
> > ProtectKernelLogs=true
> 
> ProtectSystem=full setting makes /etc read-only for processes invoked by
> the ibacm service.
> 
> As a result, the code that generates the address configuration file (if
> it does not exist) fails:
> 
> static FILE *acm_open_addr_file(void)
> {
>         FILE *f;
> 
>         if ((f = fopen(addr_file, "r")))
>                  return f;
> 
>         acm_log(0, "notice - generating %s file\n", addr_file);
>         if (!(f = popen(acme, "r"))) {
>                 acm_log(0, "ERROR - cannot generate %s\n", addr_file);
>                 return NULL;
>         }
> 
>         pclose(f);
>         return fopen(addr_file, "r");
> }
> 
> The popen() code above is supposed to generate the file if it does not
> exist (i.e., fails the first fopen()). The popen() now fails as a result
> of the ProtectSystem option setting.
> 
> ibacm(8) does say "If the address file cannot be found, the ibacm
> service will attempt to create one using default values."
> 
> I guess my question is simply was this change in behavior expected? Are
> admins expected to run ib_acme to generate the address configuration
> file prior to starting the ibacm service?

I don't think it is intentional, but it seems like the right course of
action to me.

daemons should not write to /etc/. There are many good reasons for
that.

> Is the popen() code in acm_open_addr_file() being left in place in case
> an admin decides to remove the ProtectSystem option from the
> ibacm.service file?

No, I think it was just missed

Jason