Re: [PATCH v5 17/18] pesto, conf: Send updated rules from pesto back to passt/pasta

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3352 bytes --]

On Sat, Apr 25, 2026 at 11:36:05AM +0200, Stefano Brivio wrote:
> On Fri, 24 Apr 2026 18:38:57 -0400
> Jon Maloy <jmaloy@redhat.com> wrote:
> 
> > On 2026-04-21 02:25, David Gibson wrote:
> > > Extend pesto to send the updated rule configuration back to passt/pasta.
> > > Extend passt/pasta to read the new configuration and store the new rules in
> > > a "pending" table.   We don't yet attempt to activate them.
> > > 
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > > Message-ID: <20260322141843.4095972-3-sbrivio@redhat.com>
> > > [dwg: Based on an early draft from Stefano]\
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>  
> > 
> > [...]
> > >   
> > > +/**
> > > + * conf_recv_rules() - Receive forwarding rules from configuration client
> > > + * @c:		Execution context
> > > + * @fd:		Socket to the client
> > > + *
> > > + * Return: 0 on success, -1 on failure
> > > + */
> > > +static int conf_recv_rules(const struct ctx *c, int fd)
> > > +{
> > > +	while (1) {
> > > +		struct fwd_table *fwd;
> > > +		struct fwd_rule r;
> > > +		uint32_t count;
> > > +		uint8_t pif;
> > > +		unsigned i;
> > > +
> > > +		if (read_u8(fd, &pif))
> > > +			return -1;
> > > +
> > > +		if (pif == PIF_NONE)
> > > +			break;
> > > +
> > > +		if (pif >= ARRAY_SIZE(c->fwd_pending) ||
> > > +		    !(fwd = c->fwd_pending[pif])) {
> > > +			err("Received rules for non-existent table");
> > > +			return -1;
> > > +		}
> > > +
> > > +		if (read_u32(fd, &count))
> > > +			return -1;
> > > +
> > > +		if (count > MAX_FWD_RULES) {
> > > +			err("Received %"PRIu32" rules (maximum %u)",
> > > +			    count, MAX_FWD_RULES);
> > > +			return -1;
> > > +		}
> > > +
> > > +		for (i = 0; i < count; i++) {
> > > +			fwd_rule_read(fd, &r);  
> > 
> > Since we don't check the return value I think we risk passing an only 
> > partially initialized fwd_rule to fwd_rule_add() if the read fails.
> > Maybe:
> >    if (fwd_rule_read(fd, &r))
> >        return -1;
> 
> Right, yes, that makes sense in general, even though I think this will
> need a small rework (I didn't get to that yet) to implement this point
> of the to-do list (see cover letter):
> 
> > - Don't allow a client which sends a partial configuration then
> >   blocks also block passt

Right.  In retrospect this requirement makes the way I structured the
helpers in serialise.c not so helpful after all.

> ...because at that point we'll want to permit partial reads and keep a
> buffer with a counter of received bytes (perhaps rules / PIFs too).
> 
> But actually it doesn't even need to be in this series or in a first
> implementation. It could simply be a limitation (in that case, I'll add
> the return -1 you suggest).
> 
> A user who can connect to passt could anyway configure it to be useless
> so I don't see any particular security concern with it.

That's a good point.  At the moment the limitations of the protocol
(specifically the lack of TAP rules) limits the amount of damage a
client can do, but we do hope to extend that, so I think the argument
makes sense anyway.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]