fix: agent config not picked up from parameters

[lenkan]

When agent URLs where only provided through parameters (from env variables), the "dt" field was not set, which caused keripy to ignore the configuration. This PR ensure that "dt" field is set.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.63%. Comparing base (1f22907) to head (e656d30).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #397      +/-   ##
==========================================
+ Coverage   86.60%   86.63%   +0.03%     
==========================================
  Files          25       25              
  Lines        5286     5299      +13     
==========================================
+ Hits         4578     4591      +13     
  Misses        708      708              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kentbull]

I believe the security protocol for reading config is that KERIA, if it wants to be most secure, should reject the config if it doesn't have a "dt" field like how habbing's Habery.reconfigure only loads config if dt is in conf.

It is more convenient to just add the 'dt' in here if it's missing, though at a bare minimum we should check to see if "dt" is specified and if not either add it in or throw a config error.

[lenkan]

Thank you for feedback. Can you elaborate on how it is more secure to only load the config if it has a "dt" field?

[lenkan]

I had a look at the link you provided and followed the oobirecords path after resolving.

It looks like there are two uses of conf["dt"]. One is for oobi resolving, the link you provided. But there it is only used to keep track of when to retry failed oobis:

    def processRetries(self):
        """ Process Client responses by parsing the messages and removing the client/doer

        """
        for (url,), obr in self.hby.db.eoobi.getItemIter():
            last = helping.fromIso8601(obr.date)
            now = helping.nowUTC()
            if (now - last) > datetime.timedelta(seconds=self.RetryDelay):
                obr.date = helping.toIso8601(now)
                self.hby.db.eoobi.rem(keys=(url,))
                self.hby.db.oobis.pin(keys=(url,), val=obr)

Then it is used to create for the /end/role/add and /loc/scheme events to add the controller url. But this is only done once on first load (I might be reading the code wrong, but that's what it looks like). In my opinion, this should use the timestamp of when the event was created, not some stale timestamp from a configuration?

        conf = self.cf.get()
        if self.name not in conf:
            return

        conf = conf[self.name]
        if "dt" in conf:  # datetime of config file
            dt = help.fromIso8601(conf["dt"])  # raises error if not convert
            msgs = bytearray()
            msgs.extend(self.makeEndRole(eid=self.pre,
                                         role=kering.Roles.controller,
                                         stamp=help.toIso8601(dt=dt)))
            if "curls" in conf:
                curls = conf["curls"]
                for url in curls:
                    splits = urlsplit(url)
                    scheme = (splits.scheme if splits.scheme in kering.Schemes
                              else kering.Schemes.http)
                    msgs.extend(self.makeLocScheme(url=url,
                                                   scheme=scheme,
                                                   stamp=help.toIso8601(dt=dt)))
            self.psr.parse(ims=msgs)

Please correct me if I am wrong.

[kentbull]

Can you elaborate on how it is more secure to only load the config if it has a "dt" field?

Yeah, it has to do something with BADA-RUN or KRAM, I just haven't had the time to go re-read the BADA-RUN and KRAM rules to think this through.

[kentbull]

not some stale timestamp from a configuration

From a convo I had with Sam the reason the "dt" is necessary in the config is so that BADA-RUN rules can be applied.

Here's a quote from the KERI spec BADA-RUN language on the purpose of BADA-RUN:

The primary purpose of BADA policy is to enforce monotonicity of the updates with respect to the key state of that associated AID.

A date timestamp field is intended to be used in conjunction with a signature. In this case it's a local config file so what seems most to apply here is that a timestamp should be used with the config data so that the code can, in theory, check the timestamp of config already in the system to determine whether the config being read in is newer or older than the existing config. To my understanding this would protect against local replay attacks.

[lenkan]

So, just to clarify the flows so we can determine what the difference is:

Config file

• Create a config file with curls and a timestamp dt.
• Start the keria instance
• Some time later, an agent is started, creates and signs rpy events /add/end/role and /loc/scheme with timestamp from config.

Env variable (with this change)

• Start the keria instance
• Some time later, an agent is started, creates and signs rpy events /add/end/role and /loc/scheme with current system timestamp.

Do you agree with this flow? Is the second flow insecure according to the rules that you mentioned?

[kentbull]

It is important to emphasize the risk profile and threat model whenever we discuss security. While maximal security is a good discipline to use mentally it can also be unnecessarily constraining. This is one case where I see it as unnecessarily constraining. I believe it is okay for the Agency to take the configured KERIA config and add the current system timestamp to the Agent as the initial config for that agent on startup. We can look at this adding of initial timestamp as setting the initial monotonic timeline starting point. This would protect the agent from future config replays, yet leave the Agency exposed to config replays per-Agent prior to Agent initialization, which may be an acceptable risk since we already have a supported way for the "Dr" field to be set for BADA-RUN to be applied.

To get down into the BADA-RUN details, the difference between the two scenarios you mentioned is that pulling the current system timestamp means that the Agency cannot protect itself from replay attacks of config that is missing a timestamp. The risk here is that per-agent a malicious attacker could feed bad, backdated config to the Agency which could mess up both the Agency's internal config and any new Agent configs.

Yet, it is reasonable to assume that if an attacker has access to a KERIA server that they could just use a more recent timestamp and inject bad config anyway, so using BADA-RUN to protect from local replay attacks doesn't really buy us anything. And the configs we're taking about are all only read once on each Agency startup and then again once during Agent initialization. There is no dynamic config reload during runtime which means the attack window is small, limited to when the Agency is restarted.