Page MenuHomeFreeBSD
Differential D20340 Diff 57656 sys/netipsec/xform_esp.c

Changeset View

Standalone View

View Options

sys/netipsec/xform_esp.c

Show First 20 Lines • Show All 88 Lines • ▼ Show 20 Lines
SYSCTL_DECL(_net_inet_esp); SYSCTL_DECL(_net_inet_esp);
SYSCTL_INT(_net_inet_esp, OID_AUTO, esp_enable, SYSCTL_INT(_net_inet_esp, OID_AUTO, esp_enable,
CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(esp_enable), 0, ""); CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(esp_enable), 0, "");
SYSCTL_VNET_PCPUSTAT(_net_inet_esp, IPSECCTL_STATS, stats, SYSCTL_VNET_PCPUSTAT(_net_inet_esp, IPSECCTL_STATS, stats,
struct espstat, espstat, struct espstat, espstat,
"ESP statistics (struct espstat, netipsec/esp_var.h"); "ESP statistics (struct espstat, netipsec/esp_var.h");
static struct timeval lastwarn;
static struct timeval warninterval = { .tv_sec = 1, .tv_usec = 0 };
cemUnsubmitted
Done
Inline Actions

It's a little odd that there are per file rather than per algorithm (maybe an argument for building rate-limiting in to gone_in instead). For example, someone who has two deprecated algorithms configured may get only one warning. This is a bit of a tortured hypothetical, though.

cem: It's a little odd that there are per file rather than per algorithm (maybe an argument for…
jhbAuthorUnsubmitted
Done
Inline Actions

You can't combine two ciphers or two auth algorithms in a single SA is part of why I did it, but I guess I could split out to N different 'lastwarn' variables.

jhb: You can't combine two ciphers or two auth algorithms in a single SA is part of why I did it…
cemUnsubmitted
Done
Inline Actions

I think it's vaguely plausible N different SAs could be initialized within 1s as part of an rc.d start script or similar, but I don't feel strongly about it.

cem: I think it's vaguely plausible `N` different SAs could be initialized within 1s as part of an…
static int esp_input_cb(struct cryptop *op); static int esp_input_cb(struct cryptop *op);
static int esp_output_cb(struct cryptop *crp); static int esp_output_cb(struct cryptop *crp);
size_t size_t
esp_hdrsiz(struct secasvar *sav) esp_hdrsiz(struct secasvar *sav)
{ {
size_t size; size_t size;
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Lines if (sav->key_enc == NULL) {
return EINVAL; return EINVAL;
} }
if ((sav->flags & (SADB_X_EXT_OLD | SADB_X_EXT_IV4B)) == if ((sav->flags & (SADB_X_EXT_OLD | SADB_X_EXT_IV4B)) ==
SADB_X_EXT_IV4B) { SADB_X_EXT_IV4B) {
DPRINTF(("%s: 4-byte IV not supported with protocol\n", DPRINTF(("%s: 4-byte IV not supported with protocol\n",
__func__)); __func__));
return EINVAL; return EINVAL;
} }
switch (sav->alg_enc) {
case SADB_EALG_DESCBC:
if (ratecheck(&lastwarn, &warninterval))
gone_in(13, "DES cipher for IPsec");
break;
case SADB_X_EALG_BLOWFISHCBC:
if (ratecheck(&lastwarn, &warninterval))
gone_in(13, "Blowfish cipher for IPsec");
break;
case SADB_X_EALG_CAST128CBC:
if (ratecheck(&lastwarn, &warninterval))
gone_in(13, "CAST cipher for IPsec");
break;
case SADB_X_EALG_CAMELLIACBC:
jhbAuthorUnsubmitted
Done
Inline Actions

RFC 8221 does not explicitly name Camellia either. Note that RFC 4429 lists CTR and CCM modes for Camellia in addition to CBC, but we only implement CBC. @bjk had maybe suggested keeping Camellia in the thread on arch@.

jhb: RFC 8221 does not explicitly name Camellia either. Note that RFC 4429 lists CTR and CCM modes…
cemUnsubmitted
Not Done
Inline Actions

I am happy to remove an obscure-ish cipher. I think we could be open to keeping it if enough users really must have it for some reason (like FCP 101). (Only for Camellia. DES, Blowfish, and Cast128 should go.)

cem: I am happy to remove an obscure-ish cipher. I think we could be open to keeping it if enough…
jhbAuthorUnsubmitted
Done
Inline Actions

The main reason I asked about Camellia is that bjk@ raised it during the thread on arch@, so I want to give Ben a chance to chime in. Camellia roughly speaking just seems to be AES with different S-boxes? Ben did say it might be popular in .jp. When digging through some other RFCs for IKE (IIRC), there are still mentions of modern versions of Camellia (there were new IKE cipher numbers for implicit IV variants of AES-GCM and Camellia-GCM in a recent RFC I saw). Of course, we only have a CBC mode for Camellia. I do not plan on adding any other modes for Camellia myself. RFC 8221 does list chacha2 + poly1035 as a "SHOULD" btw, so if there's any effort into adding new modes to IPsec it should be that IMO.

jhb: The main reason I asked about Camellia is that bjk@ raised it during the thread on arch@, so I…
cemUnsubmitted
Not Done
Inline Actions

Yeah, I am giving some charity to Camellia for those reasons as well. Agree Chacha-poly is the popular/well-vetted AES-alternative du jour and that's probably a better future direction, though I don't have any interest in IPsec myself. (My understanding is that S-box algorithms are difficult to implement in constant time and that one major advantage of Chacha over software AES and perhaps Camellia is ease of constant-time implementation.)

cem: Yeah, I am giving some charity to Camellia for those reasons as well. Agree Chacha-poly is the…
if (ratecheck(&lastwarn, &warninterval))
gone_in(13, "Camellia cipher for IPsec");
break;
}
/* subtract off the salt, RFC4106, 8.1 and RFC3686, 5.1 */ /* subtract off the salt, RFC4106, 8.1 and RFC3686, 5.1 */
keylen = _KEYLEN(sav->key_enc) - SAV_ISCTRORGCM(sav) * 4; keylen = _KEYLEN(sav->key_enc) - SAV_ISCTRORGCM(sav) * 4;
if (txform->minkey > keylen || keylen > txform->maxkey) { if (txform->minkey > keylen || keylen > txform->maxkey) {
DPRINTF(("%s: invalid key length %u, must be in the range " DPRINTF(("%s: invalid key length %u, must be in the range "
"[%u..%u] for algorithm %s\n", __func__, "[%u..%u] for algorithm %s\n", __func__,
keylen, txform->minkey, txform->maxkey, keylen, txform->minkey, txform->maxkey,
txform->name)); txform->name));
return EINVAL; return EINVAL;
▲ Show 20 Lines • Show All 809 Lines • Show Last 20 Lines