Skip to content

fix: agent config not picked up from parameters #397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lenkan wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: agent config not picked up from parameters #397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a1df90
fix: agent config not picked up from parameters
lenkan Nov 3, 2025
e656d30
Merge branch 'main' into fix-config-dt
lenkan Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/keria/app/agenting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ def _loadConfigForAgent(self, caid):
config[habName] = {}

config[habName]["curls"] = config[habName].get("curls", [])
config[habName]["dt"] = config[habName].get("dt", timestamp)
Copy link
Collaborator

@kentbull kentbull Nov 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@lenkan lenkan Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@lenkan lenkan Nov 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@kentbull kentbull Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@kentbull kentbull Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@lenkan lenkan Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

@kentbull kentbull Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

config["iurls"] = config.get("iurls", [])
config["durls"] = config.get("durls", [])
if self.curls is not None and isinstance(self.curls, list):
Expand Down
3 changes: 2 additions & 1 deletion tests/app/test_agenting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,8 @@ def test_agency_with_urls_from_arguments():
assert agent.pre == "EK-DLhw2jJ__SNNaSmSwfkA0gHtsAMAFnx21xOKncX23"

assert agent.hby.cf is not None
assert agent.hby.cf.get()[f"agent-{caid}"]["curls"] == curls
assert agent.hby.cf.get()[f"agent-{caid}"].get("curls") == curls
assert agent.hby.cf.get()[f"agent-{caid}"].get("dt") is not None
assert agent.hby.cf.get()["iurls"] == iurls
assert agent.hby.cf.get()["durls"] == durls

Expand Down
Loading