SecretKey without S2K specifier

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## v0.6.3
+
+### Enhancements
+
+* Updated `OpenPGP.SecretKeyPacket` to support unencrypted keys (no S2K specifier given, S2K usage byte of 0).
+* Introduced `OpenPGP.Util.checksum/1` and refactored related codebase.
+
 ## v0.6.2
 
 ### Enhancements

lib/open_pgp/encrypt/public_key_encrypted_session_key_packet_impl.ex

@@ -61,12 +61,9 @@ defimpl OpenPGP.Encrypt, for: OpenPGP.PublicKeyEncryptedSessionKeyPacket do
     g = :binary.decode_unsigned(group_g)
     y = :binary.decode_unsigned(value_y)
 
-    checksum =
-      for <<byte::8 <- session_key>>, reduce: 0 do
-        acc -> rem(acc + byte, 65536)
-      end
+    chsum = Util.checksum(session_key)
 
-    value_m = Util.PKCS1.encode(:eme_pkcs1_v1_5, <<sym_algo_id::8, session_key::binary, checksum::16>>, sender_pub_key)
+    value_m = Util.PKCS1.encode(:eme_pkcs1_v1_5, <<sym_algo_id::8, session_key::binary, chsum::binary>>, sender_pub_key)
     m = :binary.decode_unsigned(value_m)
 
     g_k_mod_p = :crypto.mod_pow(g, k, p)

lib/open_pgp/public_key_encrypted_session_key_packet.ex

@@ -160,9 +160,9 @@ defmodule OpenPGP.PublicKeyEncryptedSessionKeyPacket do
     payload = :crypto.private_decrypt(:rsa, encrypted_session_key, priv_key, [])
 
     bsize = byte_size(payload) - 2 - 1
-    <<sym_key_algo::8, session_key::bytes-size(bsize), expected_checksum::16>> = payload
-    octets = for <<b::8 <- session_key>>, do: b
-    actual_checksum = octets |> Enum.sum() |> rem(65536)
+    <<sym_key_algo::8, session_key::bytes-size(bsize), expected_checksum::bytes-size(2)>> = payload
+
+    actual_checksum = Util.checksum(session_key)
 
     if expected_checksum == actual_checksum do
       %{

lib/open_pgp/secret_key_packet.ex

@@ -1,17 +1,17 @@
 defmodule OpenPGP.SecretKeyPacket do
-  @v05x_note """
-  As of 0.5.x Secret-Key Packet supports only:
+  @v06x_note """
+  As of 0.6.x Secret-Key Packet supports only:
 
     1. V4 packets
     1. Iterated and Salted String-to-Key (S2K) specifier (ID: 3)
-    1. S2K usage convention octet of 254 only
+    1. S2K usage convention octet of 254 or 0 (no S2K given)
     1. S2K hashing algo SHA1
     1. AES128 symmetric encryption of secret key material
   """
   @moduledoc """
   Represents structured data for Secret-Key Packet.
 
-  > NOTE: #{@v05x_note}
+  > NOTE: #{@v06x_note}
   ---
 
   ## [RFC4880](https://www.ietf.org/rfc/rfc4880.txt)
@@ -128,10 +128,10 @@ defmodule OpenPGP.SecretKeyPacket do
   @type t :: %__MODULE__{
           public_key: PublicKeyPacket.t(),
           s2k_usage: {0..255, binary()},
-          s2k_specifier: S2KSpecifier.t(),
+          s2k_specifier: S2KSpecifier.t() | nil,
           sym_key_algo: OpenPGP.Util.sym_algo_tuple(),
-          sym_key_initial_vector: binary(),
-          sym_key_size: non_neg_integer(),
+          sym_key_initial_vector: binary() | nil,
+          sym_key_size: non_neg_integer() | nil,
           secret_key_material: tuple() | nil,
           ciphertext: binary()
         }
@@ -161,23 +161,40 @@ defmodule OpenPGP.SecretKeyPacket do
           ciphertext: ciphertext
         }
 
+        {packet, ""}
+
+      <<s2k_usage::8, next::binary>> when s2k_usage == 0 ->
+        packet = %__MODULE__{
+          public_key: public_key,
+          s2k_usage: s2k_usage_tuple(s2k_usage),
+          sym_key_algo: Util.sym_algo_tuple(0),
+          ciphertext: next
+        }
+
         {packet, ""}
     end
   end
 
   @doc """
   Decrypt Secret-Key Packet given decoded Secret-Key Packet and a
-  passphrase.
+  passphrase (optional).
   Return Secret-Key Packet with `:secret_key_material` attr assigned.
-  Raises an error if checksum does not match.
+  Raises an error if checksum/hash does not match.
   """
-  @spec decrypt(t(), passphrase :: binary()) :: t()
-  def decrypt(%__MODULE__{} = packet, "" <> _ = passphrase) do
+  @spec decrypt(t(), passphrase :: binary() | nil) :: t()
+  def decrypt(%__MODULE__{public_key: %{version: 4}} = packet, passphrase) do
     case packet do
-      %__MODULE__{public_key: %{version: 4}, s2k_usage: {254, _}, sym_key_algo: {7, _}} -> :ok
-      %__MODULE__{} -> raise(@v05x_note <> "\n Got: #{inspect(packet)}")
+      %__MODULE__{s2k_usage: {254, _}, sym_key_algo: {7, _}} -> do_decrypt({254, 7}, packet, passphrase)
+      %__MODULE__{s2k_usage: {0, _}, sym_key_algo: {0, _}} -> do_decrypt({0, 0}, packet, nil)
+      %__MODULE__{} -> raise(@v06x_note <> "\n Got: #{inspect(packet)}")
     end
+  end
 
+  def decrypt(%__MODULE__{public_key: %{version: _}} = packet, _passphrase) do
+    raise(@v06x_note <> "\n Got: #{inspect(packet)}")
+  end
+
+  defp do_decrypt({254 = _s2k_usage, 7 = _sym_key_algo}, %__MODULE__{} = packet, passphrase) do
     %__MODULE__{
       sym_key_size: session_key_size,
       sym_key_initial_vector: iv,
@@ -188,7 +205,7 @@ defmodule OpenPGP.SecretKeyPacket do
     session_key = S2KSpecifier.build_session_key(s2k_specifier, session_key_size, passphrase)
 
     plaintext = :crypto.crypto_one_time(:aes_128_cfb128, session_key, iv, ciphertext, false)
-    {data, _checksum} = validate_checksum!(plaintext)
+    {data, _hash} = validate_hash!(plaintext)
 
     {secret_exp_d, next} = Util.decode_mpi(data)
     {prime_val_p, next} = Util.decode_mpi(next)
@@ -200,28 +217,62 @@ defmodule OpenPGP.SecretKeyPacket do
     %{packet | secret_key_material: material}
   end
 
-  @checksum_byte_size 20
+  defp do_decrypt({0 = _s2k_usage, 0 = _sym_key_algo}, %__MODULE__{} = packet, _passphrase) do
+    {data, _checksum} = validate_checksum!(packet.ciphertext)
+
+    {secret_exp_d, next} = Util.decode_mpi(data)
+    {prime_val_p, next} = Util.decode_mpi(next)
+    {prime_val_q, next} = Util.decode_mpi(next)
+    {secret_u, ""} = Util.decode_mpi(next)
+
+    material = {secret_exp_d, prime_val_p, prime_val_q, secret_u}
+
+    %{packet | secret_key_material: material}
+  end
+
+  @hash_byte_size 20
+  @spec validate_hash!(binary()) :: {data :: binary(), hash :: binary()}
+  defp validate_hash!("" <> _ = plaintext) do
+    data_byte_size = byte_size(plaintext) - @hash_byte_size
+
+    <<data::bytes-size(data_byte_size), expected_hash::bytes-size(@hash_byte_size)>> = plaintext
+
+    actual_hash = :crypto.hash(:sha, data)
+
+    if actual_hash == expected_hash do
+      {data, actual_hash}
+    else
+      expected_hex = expected_hash |> Base.encode16() |> inspect()
+      actual_hex = actual_hash |> Base.encode16() |> inspect()
+
+      msg = "Expected SecretKeyPacket SHA1 hash to be #{expected_hex}, got #{actual_hex}. Maybe incorrect passphrase?"
+
+      raise(msg)
+    end
+  end
+
+  @checksum_byte_size 2
   @spec validate_checksum!(binary()) :: {data :: binary(), checksum :: binary()}
   defp validate_checksum!("" <> _ = plaintext) do
-    plaintext_byte_size = byte_size(plaintext) - @checksum_byte_size
+    data_byte_size = byte_size(plaintext) - @checksum_byte_size
 
-    <<data::bytes-size(plaintext_byte_size), expected_checksum::bytes-size(@checksum_byte_size)>> = plaintext
+    <<data::bytes-size(data_byte_size), expected_checksum::bytes-size(@checksum_byte_size)>> = plaintext
 
-    actual_checksum = :crypto.hash(:sha, data)
+    actual_checksum = Util.checksum(data)
 
     if actual_checksum == expected_checksum do
       {data, actual_checksum}
     else
       expected_hex = expected_checksum |> Base.encode16() |> inspect()
       actual_hex = actual_checksum |> Base.encode16() |> inspect()
 
-      msg = "Expected SecretKeyPacket checksum to be #{expected_hex}, got #{actual_hex}. Maybe incorrect passphrase?"
+      msg = "Expected SecretKeyPacket checksum to be #{expected_hex}, got #{actual_hex}. Maybe malformed ciphertext?"
 
       raise(msg)
     end
   end
 
-  @s2k_spec_given_text "String-to-key specifier is being given"
-  defp s2k_usage_tuple(octet) when octet in 254..255, do: {octet, @s2k_spec_given_text}
+  defp s2k_usage_tuple(octet) when octet in 254..255, do: {octet, "String-to-key specifier is being given"}
+  defp s2k_usage_tuple(octet) when octet == 0, do: {octet, "String-to-key specifier is not given"}
   defp s2k_usage_tuple(octet), do: Util.sym_algo_tuple(octet)
 end

lib/open_pgp/util.ex

@@ -292,4 +292,20 @@ defmodule OpenPGP.Util do
   def sym_algo_to_crypto_cipher(8), do: :aes_192_cfb128
   def sym_algo_to_crypto_cipher(9), do: :aes_256_cfb128
   def sym_algo_to_crypto_cipher(algo), do: raise(@v06x_note <> "\n Got: #{inspect(algo)}")
+
+  @doc """
+  Calculate a two-octet checksum, which is equal to the sum of the
+  input binary octets modulo 65536.
+
+  ### Example:
+
+      iex> OpenPGP.Util.checksum(<<1::8, 2::8, 3::8>>)
+      <<6::16>>
+  """
+  @spec checksum(binary()) :: <<_::16>>
+  def checksum("" <> _ = binary) do
+    for <<byte::8 <- binary>>, reduce: <<0::16>> do
+      <<acc::16>> -> <<rem(acc + byte, 65536)::16>>
+    end
+  end
 end

mix.exs

@@ -2,7 +2,7 @@ defmodule OpenPGP.MixProject do
   use Mix.Project
 
   @source_url "https://github.com/DivvyPayHQ/open_pgp"
-  @version "0.6.2"
+  @version "0.6.3"
   @description "OpenPGP Message Format in Elixir - RFC4880"
 
   def project() do

test/open_pgp/secret_key_packet_test.exs

@@ -8,9 +8,10 @@ defmodule OpenPGP.SecretKeyPacketTest do
   alias OpenPGP.Util
 
   @rsa2048_priv File.read!("test/fixtures/rsa2048-priv.pgp")
+  @rsa2048_priv_no_s2k File.read!("test/fixtures/rsa2048-priv.no-s2k.pgp")
 
   describe ".decode/1" do
-    test "decodes Secret-Key Packet and assignes ciphertext" do
+    test "decodes Secret-Key Packet and assignes ciphertext (S2K given, RSA2048)" do
       assert [
                %Packet{tag: %PacketTag{tag: {5, "Secret-Key Packet"}}} = sk_packet
                | _
@@ -43,6 +44,32 @@ defmodule OpenPGP.SecretKeyPacketTest do
       assert "F831CDDF1B42A66D" = Base.encode16(s2k_salt)
       assert "36EB1D1342D9F9E9498F458E0A122A6A" = Base.encode16(sym_key_iv)
     end
+
+    test "decodes Secret-Key Packet and assignes ciphertext (S2K not given, RSA2048)" do
+      assert [
+               %Packet{tag: %PacketTag{tag: {5, "Secret-Key Packet"}}} = sk_packet
+               | _
+             ] = OpenPGP.list_packets(@rsa2048_priv_no_s2k)
+
+      assert {%SecretKeyPacket{} = sk_packet_decoded, <<>>} =
+               sk_packet
+               |> Util.concat_body()
+               |> SecretKeyPacket.decode()
+
+      assert %SecretKeyPacket{
+               public_key: %PublicKeyPacket{
+                 algo: {1, "RSA (Encrypt or Sign) [HAC]"},
+                 version: 4
+               },
+               s2k_usage: {0, "String-to-key specifier is not given"},
+               s2k_specifier: nil,
+               sym_key_algo: {0, "Plaintext or unencrypted data"},
+               sym_key_initial_vector: nil,
+               sym_key_size: nil,
+               ciphertext: <<7, 250, 2, _::binary>>,
+               secret_key_material: nil
+             } = sk_packet_decoded
+    end
   end
 
   describe ".decrypt/2" do
@@ -87,7 +114,47 @@ defmodule OpenPGP.SecretKeyPacketTest do
                "EAF22450D96B244CA2873569C407AE3F9D2252428" = Base.encode16(secret_u)
     end
 
-    test "raises error if passphrase invalid" do
+    test "decrypts Secret-Key Packet when S2K not given (no passphrase)" do
+      assert [
+               %Packet{tag: %PacketTag{tag: {5, "Secret-Key Packet"}}} = sk_packet
+               | _
+             ] = OpenPGP.list_packets(@rsa2048_priv_no_s2k)
+
+      assert {%SecretKeyPacket{} = sk_packet_decoded, <<>>} =
+               sk_packet
+               |> Util.concat_body()
+               |> SecretKeyPacket.decode()
+
+      assert %SecretKeyPacket{secret_key_material: material} = SecretKeyPacket.decrypt(sk_packet_decoded, nil)
+
+      assert {secret_exp_d, prime_val_p, prime_val_q, secret_u} = material
+
+      assert "0206152887DF8678CAC235EC5FD1ED537B2275D01242C306451EF7BFB005E2242F021BF46" <>
+               "EA996A74683766DC792C2D01A8253098CDE7C9A2F64017C8814DE4A69E276D93581AC77" <>
+               "CB81C672D442FEA242A03DC7C609FAC0F46B3C0755DE97D408BC7F41D4BE01D7252AD63" <>
+               "7F901C3AF34FA13E44E12CDF5C63C46CED14A4C73B8A88D9B6278A995DB0F49778169E2" <>
+               "AD4A774D7C33657617D7469594A02E5A54336766A804339AD4B5B27AC16660BA4584D4B" <>
+               "7F7E1FE4246C2D1B204AF90E53D1EA60A20AF9CBEF476867B61F979D523773F15147D7E" <>
+               "4E8A520F4FAA4FFFFC6CC5EAE8581DFF75BB05988A9B12D361893E4F754E964F3B6CA1E" <>
+               "237AE0D479807" = Base.encode16(secret_exp_d)
+
+      assert "E18852EA986485496F76F75A2EDA0891AC6E6D5A7859B59F5B1042D07C61B2CC526041D4C" <>
+               "4E653EB78213504EE62412B98F45E254FC346FAE03E45F624EEFAD58B3034F678B845B0" <>
+               "C84E9DEEA3D6E32B0D724620FC4DA3507107559432A61245EAA90B23C9730005FAD35D1" <>
+               "CAE634F6E02BA74F630FFC4344A4CC59A72C6E8A3" = Base.encode16(prime_val_p)
+
+      assert "FBA4A4B85E505D761A99078E4EA26B3673D8E6750447C90E324812155DE53EBDD9F6DF129" <>
+               "E53DA03DCD2FDE97EA6FA3E34F0EF2338EB50383361E4B832D51BFF0BF28DBD71F95144" <>
+               "F823D3711E1D8C6983D04FACD842AA7D706E69A2BB4827FAEE2319CB72C5DAA248FAFD0" <>
+               "AEE4A05B39237ABAAE0C02AE2B66E7EE3988F58B3" = Base.encode16(prime_val_q)
+
+      assert "49548DD97FC278ABDC1D67EEF641B84AD5225C34C19EB72C70FE548FD9E7CAAEF7B43013A" <>
+               "BBF97C5FBE281A5C602CA7055641C7D3169B459CBB9DDE37164B17A3E7EAE7BAC3986C5" <>
+               "239ED8B4E963E3C69DDDF8608DF11FBE1D2E97AE26D62B7882C045708E2BE8B684AF5F6" <>
+               "EAF22450D96B244CA2873569C407AE3F9D2252428" = Base.encode16(secret_u)
+    end
+
+    test "raises error if passphrase invalid (S2K given)" do
       assert [
                %Packet{tag: %PacketTag{tag: {5, "Secret-Key Packet"}}} = sk_packet
                | _
@@ -99,11 +166,40 @@ defmodule OpenPGP.SecretKeyPacketTest do
                |> SecretKeyPacket.decode()
 
       expected_error =
-        "Expected SecretKeyPacket checksum to be \"D243B4448D3EC2116BC163ED0CC4A5BD42C853EE\", got \"CF1B516FCC942C8796C20EAF2F045056835BF0CE\". Maybe incorrect passphrase?"
+        "Expected SecretKeyPacket SHA1 hash to be \"D243B4448D3EC2116BC163ED0CC4A5BD42C853EE\", got \"CF1B516FCC942C8796C20EAF2F045056835BF0CE\". Maybe incorrect passphrase?"
 
       assert_raise RuntimeError, expected_error, fn ->
         SecretKeyPacket.decrypt(sk_packet_decoded, "invalid")
       end
     end
+
+    @expected_error "Expected SecretKeyPacket checksum to be \"0000\", got \"48E3\". Maybe malformed ciphertext?"
+    test "raises error if ciphertext malformed (no S2K given)" do
+      assert [
+               %Packet{tag: %PacketTag{tag: {5, "Secret-Key Packet"}}} = sk_packet
+               | _
+             ] = OpenPGP.list_packets(@rsa2048_priv_no_s2k)
+
+      assert {%SecretKeyPacket{} = sk_packet_decoded, <<>>} =
+               sk_packet
+               |> Util.concat_body()
+               |> SecretKeyPacket.decode()
+
+      {malformed_ciphertext, _} = replace_checksum(sk_packet_decoded.ciphertext, <<0::16>>)
+
+      assert_raise RuntimeError, @expected_error, fn ->
+        sk_packet_decoded
+        |> Map.put(:ciphertext, malformed_ciphertext)
+        |> SecretKeyPacket.decrypt(nil)
+      end
+    end
+  end
+
+  @checksum_byte_size 2
+  defp replace_checksum("" <> _ = input, <<_::16>> = new_checksum) do
+    data_byte_size = byte_size(input) - @checksum_byte_size
+    <<data::bytes-size(data_byte_size), original_checksum::bytes-size(@checksum_byte_size)>> = input
+
+    {data <> new_checksum, original_checksum}
   end
 end