diff --git a/CHANGELOG.md b/CHANGELOG.md index b37ca61..e3fa923 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/open_pgp/encrypt/public_key_encrypted_session_key_packet_impl.ex b/lib/open_pgp/encrypt/public_key_encrypted_session_key_packet_impl.ex index 5c84c7c..7f74a95 100644 --- a/lib/open_pgp/encrypt/public_key_encrypted_session_key_packet_impl.ex +++ b/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 <>, reduce: 0 do - acc -> rem(acc + byte, 65536) - end + chsum = Util.checksum(session_key) - value_m = Util.PKCS1.encode(:eme_pkcs1_v1_5, <>, sender_pub_key) + value_m = Util.PKCS1.encode(:eme_pkcs1_v1_5, <>, sender_pub_key) m = :binary.decode_unsigned(value_m) g_k_mod_p = :crypto.mod_pow(g, k, p) diff --git a/lib/open_pgp/public_key_encrypted_session_key_packet.ex b/lib/open_pgp/public_key_encrypted_session_key_packet.ex index 9776f9a..0961c38 100644 --- a/lib/open_pgp/public_key_encrypted_session_key_packet.ex +++ b/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 - <> = payload - octets = for <>, do: b - actual_checksum = octets |> Enum.sum() |> rem(65536) + <> = payload + + actual_checksum = Util.checksum(session_key) if expected_checksum == actual_checksum do %{ diff --git a/lib/open_pgp/secret_key_packet.ex b/lib/open_pgp/secret_key_packet.ex index 4b88c45..d4b154e 100644 --- a/lib/open_pgp/secret_key_packet.ex +++ b/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, ""} + + <> 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,14 +217,48 @@ 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 + + <> = 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 - <> = plaintext + <> = plaintext - actual_checksum = :crypto.hash(:sha, data) + actual_checksum = Util.checksum(data) if actual_checksum == expected_checksum do {data, actual_checksum} @@ -215,13 +266,13 @@ defmodule OpenPGP.SecretKeyPacket do 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 diff --git a/lib/open_pgp/util.ex b/lib/open_pgp/util.ex index 77279c3..c849a96 100644 --- a/lib/open_pgp/util.ex +++ b/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 <>, reduce: <<0::16>> do + <> -> <> + end + end end diff --git a/mix.exs b/mix.exs index e786348..4320bab 100644 --- a/mix.exs +++ b/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 diff --git a/test/fixtures/rsa2048-priv.no-s2k.pgp b/test/fixtures/rsa2048-priv.no-s2k.pgp new file mode 100644 index 0000000..8d7ace2 Binary files /dev/null and b/test/fixtures/rsa2048-priv.no-s2k.pgp differ diff --git a/test/open_pgp/secret_key_packet_test.exs b/test/open_pgp/secret_key_packet_test.exs index d705c4e..09078fd 100644 --- a/test/open_pgp/secret_key_packet_test.exs +++ b/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 + <> = input + + {data <> new_checksum, original_checksum} end end