lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-Id: <20100924.205747.35037043.davem@davemloft.net> Date: Fri, 24 Sep 2010 20:57:47 -0700 (PDT) From: David Miller <davem@...emloft.net> To: christophe.gouault@...nd.com Cc: netdev@...r.kernel.org Subject: Re: IPsec: Why do pfkey_getspi and xfrm_alloc_userspi call From: Christophe Gouault <christophe.gouault@...nd.com> Date: Mon, 13 Sep 2010 14:31:28 +0200 > I guess the larval state found by xfrm_get_acq_byseq must have the > same parameters as those provided in the message (mode, reqid, proto, > daddr, saddr, family). Contrary to what one might think, the call to > xfrm_get_acq_byseq is more costly than the call to xfrm_find_acq, > because the later uses a hash table. Whether any such requirement exists, we certainly have never enforced something like that. I tried to look for guidance in some other PFKEYV2 implementations, but those I looked at (OpenBSD for example) are even more permissive than we are. For example, OpenBSD doesn't validate the incoming sequence number at all as far as making sure it matches the one emitted for the acquire message. In fact it's approach to handling this sequence number is completely different from ours. It treats it truly as a unique 32-bit cookie which is used entirely by the user. When an acquire is emitted, it does not create an ipsec database entry, instead it waits until the GETSPI happens (see reserve_spi()). This lack of consistency for enforcement and validation amongst implementations makes it really difficult to say what might or might not be safe to do here. > Shouldn't we extend the xfrm_get_acq function so that it accepts an > optional seq parameter? We would replace the first call to > xfrm_find_acq_byseq by: > > x = xfrm_find_acq(mode, reqid, proto, xdaddr, xsaddr, 0, family, seq); > > and the if no entry is found, we would call: > > x = xfrm_find_acq(mode, reqid, proto, xdaddr, xsaddr, 1, family, 0); > > We would take benefit of the hash table, instead of looking up through > the whole SAD as does xfrm_get_acq_byseq. Even with all of the above, I do think this suggestion of your's is reasonable. But please, this function has too many arguments already, find a way to do it without adding new ones. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists