Skip to content

SymmetricKeyAlgorithmTag comments label Camellia128/192/256 (11/12/13) as "AES with N-bit key" #730

Open

Description

@hidingmyname

Summary

In SymmetricKeyAlgorithmTag, the three Camellia members carry the comments
from the three AES members above them. Camellia128 = 11 says "Reserved for
AES with 128-bit key", and the same goes for 12 and 13. IDs 11/12/13 are
Camellia (RFC 5581, and now RFC 9580). They have nothing to do with AES.

Where

crypto/src/bcpg/SymmetricKeyAlgorithmTags.cs:13-19:

        Aes128 = 7,       // Reserved for AES with 128-bit key
        Aes192 = 8,       // Reserved for AES with 192-bit key
        Aes256 = 9,       // Reserved for AES with 256-bit key
        Twofish = 10,     // Reserved for Twofish
        Camellia128 = 11, // Reserved for AES with 128-bit key
        Camellia192 = 12, // Reserved for AES with 192-bit key
        Camellia256 = 13  // Reserved for AES with 256-bit key

Lines 17-19 are the same text as lines 13-15. They were added that way in
bcgit/bc-csharp@0c945d556ab ("added Camellia tags", 2014). That commit
appended the three Camellia members and copied the AES comments along with
them:

-        Twofish = 10     // Reserved for Twofish
+        Twofish = 10,     // Reserved for Twofish
+        Camellia128 = 11, // Reserved for AES with 128-bit key
+        Camellia192 = 12, // Reserved for AES with 192-bit key
+        Camellia256 = 13  // Reserved for AES with 256-bit key

The library itself treats these as Camellia everywhere. In
crypto/src/openpgp/PgpUtilities.cs, GetSymmetricCipherName maps all three
to "Camellia", and CreateWrapper maps them to "CAMELLIAWRAP", not
"AESWRAP".

RFC 5581 §3 gives the numbers:

     | Camellia Key Length | OpenPGP Symmetric-Key Algorithm Number |
     |         128         |                   11                   |
     |         192         |                   12                   |
     |         256         |                   13                   |

RFC 9580 §9.3 lists them as "Camellia with 128-bit key [RFC3713]" and so on.

Why it matters

These are // comments, not XML docs, so they don't appear in IntelliSense.
This is a source-readability fix only, and I wouldn't call it urgent. But this
file is where you look up what a tag number means, for example when you see
11 in a packet dump. Right now it says AES-128 for that number. AES-128 is 7,
and getting the cipher wrong is the kind of mistake that makes an interop bug
much harder to track down.

The "Reserved for" wording on these lines (and on 7-10) is a separate
question, which I've left out so this one stays a plain copy-paste fix.

Suggested fix

         Twofish = 10,     // Reserved for Twofish
-        Camellia128 = 11, // Reserved for AES with 128-bit key
-        Camellia192 = 12, // Reserved for AES with 192-bit key
-        Camellia256 = 13  // Reserved for AES with 256-bit key
+        Camellia128 = 11, // Camellia with 128-bit key
+        Camellia192 = 12, // Camellia with 192-bit key
+        Camellia256 = 13  // Camellia with 256-bit key

Upstream reference

This slip only exists in bc-csharp. The bc-java file always said "Camellia"
on these three lines. Before bc-java reworded the block they read "Reserved
for Camellia with 128-bit key" and so on. The current wording comes from
bcgit/bc-java@69ab17af222 ("Document SymmetricKeyAlgorithmTags", 2024-05-07),
which turned the trailing comments into javadoc:

--- a/pg/src/main/java/org/bouncycastle/bcpg/SymmetricKeyAlgorithmTags.java
+++ b/pg/src/main/java/org/bouncycastle/bcpg/SymmetricKeyAlgorithmTags.java
-    int CAMELLIA_128 = 11;    // Reserved for Camellia with 128-bit key
-    int CAMELLIA_192 = 12;    // Reserved for Camellia with 192-bit key
-    int CAMELLIA_256 = 13;    // Reserved for Camellia with 256-bit key
...
+    /**
+     * Camellia with 128-bit key.
+     */
+    int CAMELLIA_128 = 11;
+    /**
+     * Camellia with 192-bit key.
+     */
+    int CAMELLIA_192 = 12;
+    /**
+     * Camellia with 256-bit key.
+     */
+    int CAMELLIA_256 = 13;

Current bc-java master is unchanged from that:

    /**
     * Camellia with 128-bit key.
     */
    int CAMELLIA_128 = 11;
    /**
     * Camellia with 192-bit key.
     */
    int CAMELLIA_192 = 12;
    /**
     * Camellia with 256-bit key.
     */
    int CAMELLIA_256 = 13;

The suggested fix above uses that wording but keeps bc-csharp's trailing
// comment style.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions