Page MenuHomeFreeBSD

Add deprecation warnings for IPsec algorithms deprecated in RFC 8221.
ClosedPublic

Authored by jhb on May 21 2019, 9:40 PM.
Tags
None
Referenced Files
F80226363: D20340.id57807.diff
Fri, Mar 29, 10:08 AM
Unknown Object (File)
Feb 13 2024, 7:02 PM
Unknown Object (File)
Jan 31 2024, 9:53 PM
Unknown Object (File)
Jan 28 2024, 12:12 PM
Unknown Object (File)
Jan 20 2024, 3:32 AM
Unknown Object (File)
Jan 16 2024, 12:13 PM
Unknown Object (File)
Jan 10 2024, 3:42 AM
Unknown Object (File)
Dec 28 2023, 11:55 AM

Details

Summary

All of these algorithms are either explicitly marked MUST NOT, or they
are implicitly MUST NOTs by virtue of not being included in IETF's
list of protocols at all despite having assignments from IANA.

Specifically, this adds warnings for the following ciphers:

  • des-cbc
  • blowfish-cbc
  • cast128-cbc
  • des-deriv
  • des-32iv
  • camellia-cbc

Warnings for the following authentication algorithms are also added:

  • hmac-md5
  • keyed-md5
  • keyed-sha1
  • hmac-ripemd160
Test Plan
  • used setkey with configurations using the deprecated warnings and verified I got warnings. Also used setkey with some other configurations such as GCM and confirmed I did not get any deprecation warnings.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/netipsec/key.c
8598 ↗(On Diff #57652)

Bah, I will drop this.

sys/netipsec/xform_ah.c
195 ↗(On Diff #57652)

RFC 8221 doesn't explicitly mention ripemd160 either way (I think's it just a bit obscure). It's use was documented in RFC 2857, but other hits in google suggest it was popular before SHA2.

sys/netipsec/xform_esp.c
176 ↗(On Diff #57652)

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@.

  • Drop extra blank line.
sys/netipsec/xform_ah.c
111 ↗(On Diff #57656)

BTW, I used rate limiting for the warnings because the set key configurations I tested would spam the console twice for each configuration as there were separate configurations for each direction. The rate limit reduced this to one warning per setkey invocation.

Did you also want something for ipcomp?

I wasn't planning on doing ipcomp yet, but might propose a diff for that next.

cem added inline comments.
sys/netipsec/xform_ah.c
111 ↗(On Diff #57656)

I think this should be built into gone_in(), honestly. Possibly even a limit of "once" (although I can understand an argument that there is some value in printing at least once more after the deluge of boot messages has scrolled by).

195 ↗(On Diff #57652)

There are Reasons not to use shorter hashes (<256 bit) in general; I don't know if they apply here. But I don't feel too bad about removing a hash that isn't even mentioned in the update RFC.

sys/netipsec/xform_esp.c
97–98 ↗(On Diff #57656)

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.

176 ↗(On Diff #57652)

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.)

This revision is now accepted and ready to land.May 21 2019, 11:51 PM
gnn added a subscriber: gnn.

Thanks.

sys/netipsec/xform_ah.c
111 ↗(On Diff #57656)

I think it's a bit hard to build into gone_in though I appreciate the sentiment. (For example if a user tries one bad algo and then switches to another bad one, you want them to keep getting messages.) I don't know how gone_in could really cope with that. It could maybe track the last string passed in, but that's kind of crappy as well and in the case of IPsec that would result in spam if you are using a bad hash + bad cipher.

195 ↗(On Diff #57652)

Yes, I'm probably inclined to still remove this one whereas with Camellia there may be a reason to keep it.

sys/netipsec/xform_esp.c
97–98 ↗(On Diff #57656)

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.

176 ↗(On Diff #57652)

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.

sys/netipsec/xform_esp.c
97–98 ↗(On Diff #57656)

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.

176 ↗(On Diff #57652)

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.)

I think it would be good to have this committed into 11.3 release. So you will be able to see how many users will complain that they need this support, if any.

  • Add individual warn variables.
This revision now requires review to proceed.May 23 2019, 8:41 PM
jhb marked 2 inline comments as done.May 23 2019, 8:42 PM
This revision was not accepted when it landed; it landed in state Needs Review.May 23 2019, 10:07 PM
This revision was automatically updated to reflect the committed changes.