# # # patch "NEWS" # from [76511d0b3a4b098028881ed70e290f90f0928584] # to [f6e5ed470c0c9685fa4eae8d7ea683261bdafcbd] # # patch "packet.cc" # from [7a0c4e9c49cba8d651e2ba8a69c8eee80769bc47] # to [c7972e5821369ca4d8279fb2e7d62196fb06645f] # # patch "unit-tests/packet.cc" # from [ed9cbd4154e6740014f5adde65c273f4f1f20d74] # to [4ad32f58c05111c29c6f633e90da93109d229785] # ============================================================ --- NEWS 76511d0b3a4b098028881ed70e290f90f0928584 +++ NEWS f6e5ed470c0c9685fa4eae8d7ea683261bdafcbd @@ -148,6 +148,10 @@ Xxx Xxx 99 99:99:99 UTC 2010 but instead validates if the given revision really exists in the current database. + - `mtn read` (and also `mtn automate read_packets`) now tests public + and private key data more thoroughly and aborts if it encounters + invalid data. + - `mtn conflicts store` now gives a nice error message when run outside a workspace (fixes bug #30473) ============================================================ --- packet.cc 7a0c4e9c49cba8d651e2ba8a69c8eee80769bc47 +++ packet.cc c7972e5821369ca4d8279fb2e7d62196fb06645f @@ -9,6 +9,8 @@ #include "base.hh" #include +#include +#include #include "cset.hh" #include "constants.hh" @@ -20,6 +22,7 @@ #include "cert.hh" #include "key_store.hh" // for keypair #include "char_classifiers.hh" +#include "lazy_rng.hh" using std::istream; using std::istringstream; @@ -150,6 +153,44 @@ namespace made_from, F("malformed packet: invalid key name")); } + void validate_public_key_data(string const & name, string const & keydata) const + { + string decoded = decode_base64_as(keydata, origin::user); + Botan::SecureVector key_block; + key_block.set(reinterpret_cast(decoded.c_str()), decoded.size()); + try + { + Botan::X509::load_key(key_block); + } + catch (Botan::Decoding_Error const & e) + { + E(false, origin::user, + F("malformed packet: invalid public key data for '%s': %s") + % name % e.what()); + } + } + void validate_private_key_data(string const & name, string const & keydata) const + { + string decoded = decode_base64_as(keydata, origin::user); + Botan::DataSource_Memory ds(decoded); + try + { +#if BOTAN_VERSION_CODE >= BOTAN_VERSION_CODE_FOR(1,7,7) + Botan::PKCS8::load_key(ds, lazy_rng::get(), string()); +#else + Botan::PKCS8::load_key(ds, string()); +#endif + } + catch (Botan::Decoding_Error const & e) + { + E(false, origin::user, + F("malformed packet: invalid private key data for '%s': %s") + % name % e.what()); + } + // since we do not want to prompt for a password to decode it finally, + // we ignore all other exceptions + catch (Botan::Invalid_Argument) {} + } void validate_certname(string const & cn) const { E(!cn.empty() @@ -238,6 +279,7 @@ namespace L(FL("read pubkey packet")); validate_key(args); validate_base64(body); + validate_public_key_data(args, body); cons.consume_public_key(key_name(args, made_from), decode_base64_as(body, made_from)); @@ -252,7 +294,10 @@ namespace validate_key(args); validate_base64(pub); + validate_public_key_data(args, pub); validate_base64(priv); + validate_private_key_data(args, priv); + cons.consume_key_pair(key_name(args, made_from), keypair(decode_base64_as(pub, made_from), decode_base64_as(priv, made_from))); @@ -371,11 +416,11 @@ extract_packets(string const & s, packet // this is same as rfind, but search area is haystack[start:] (from start to end of string) // haystack is searched, needle is pattern -static size_t +static size_t rfind_in_substr(std::string const& haystack, size_t start, std::string const& needle) { I(start <= haystack.size()); - const std::string::const_iterator result = + const std::string::const_iterator result = std::find_end(haystack.begin() + start, haystack.end(), needle.begin(), needle.end()); @@ -395,7 +440,7 @@ read_packets(istream & in, packet_consum static string const end("[end]"); while(in) { - size_t const next_search_pos = (accum.size() >= end.size()) + size_t const next_search_pos = (accum.size() >= end.size()) ? accum.size() - end.size() : 0; in.read(buf, bufsz); accum.append(buf, in.gcount()); ============================================================ --- unit-tests/packet.cc ed9cbd4154e6740014f5adde65c273f4f1f20d74 +++ unit-tests/packet.cc 4ad32f58c05111c29c6f633e90da93109d229785 @@ -9,11 +9,12 @@ #include "base.hh" #include "unit_tests.hh" +#include "transforms.hh" #include "vocab_cast.hh" #include "xdelta.hh" #include "../packet.cc" - +#include using std::ostringstream; UNIT_TEST(validators) @@ -55,6 +56,95 @@ UNIT_TEST(validators) Y_THROW(f.validate_key("")); Y_THROW(f.validate_key("graydon at venge dot net")); + // validate_public_key_data + N_THROW(f.validate_public_key_data( + "address@hidden", + "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDS8J8cI0a" + "Ab1Pd55UE0vlxHHBS9ZyDKGQXTf3dA+ywGeXfKYjBCAYgcZ" + "obRxVSziKZ17SfYFSOa0HvMAXykpHc+Uy3SHHnFSJb+wFYp" + "JdUrxecZMpzhySCR49lw8aFoGmpsZZmNiherpuP2CzLDCax" + "IK1dbTgilMd0dfoy277M9QIDAQAB" + )); + // this is a private key + Y_THROW(f.validate_public_key_data( + "invalid0", + "LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUNkUUl" + "CQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQWw4d2dnSmJBZ0" + "VBQW9HQkFOTHdueHdqUm9CdlU5M24KbFFUUytYRWNjRkwxb" + "klNb1pCZE4vZDBEN0xBWjVkOHBpTUVJQmlCeG1odEhGVkxP" + "SXBuWHRKOWdWSTVyUWU4dwpCZktTa2R6NVRMZEljZWNWSWx" + "2N0FWaWtsMVN2RjV4a3luT0hKSUpIajJYRHhvV2dhYW14bG" + "1ZMktGNnVtNC9ZCkxNc01KckVnclYxdE9DS1V4M1IxK2pMY" + "nZzejFBZ01CQUFFQ2dZQUFsTlZyYm91SU15bm9IMTZURW43" + "NUlzeVkKd0U3K0tVRDN2VURpRGNRQytuYi9uak81bGZUYWc" + "3M3Yva1d1Tjc3YmpxZCtQQkpLUWNFTlV0ejMyaE45elBWSQ" + "p5SzFRa1E4MmRlNHRCYlY4dFlDbmdXSFB3VWwxOHRrcFpzU" + "HJpd3E1MUpWOC9SdTdUanpRZDNHLzExQVdxcnFpCm9mMGtI" + "bC9PODBKbDNRZWJ3UUpCQU9pcEc1RlkzY1hOY0QwTjRiWjl" + "YMjZ6WWpNQWlBTG5WbktGcGpGblFqTUkKcVhCRitraWI2SU" + "11ZnZaRm1nT09LWG9vdzlyY3EyY2RwRlJ3bFVWQXdoRUNRU" + "URvR2JZNXhDNFoxMEVuQjErVAp4dGx5SEZzQW9LMXY3eGtG" + "c3RZV3hacXJUZ1hNemVkdkxiU2dHZ1lzMFNrZnlyQVFtREQ" + "yNGpjL25SOW0yNG0zCnJqaWxBa0JFZDI5cmFIRnJBamZqWD" + "dCcW1aNTUzMFFvcWlGY2FXT2hNLzlpVG5iR3VlZlM2R1RzO" + "VNTSlppZHEKcGJUYkV2elZ2Q1ZXeE5XVDlMOGxNalJiT3VG" + "aEFrQUZJcHgvaHJHbWJMYktVRVZ6RlpFMkR4Nk1Vd0hEV2p" + "6cApmVjF6UDRmK2hrbG1rSit3UEFpbENpNWN5M3ZuY2lxWE" + "UyYng3MnRkZ3ZKdzZpYVA0OURwQWtCTFlWZ3NaNHErCkxkL" + "0VYWFJibTJGOEd6MjVCaTFNV0p5OWxQOXBoY2FPaDdpZlBh" + "bVZDeTRlUGx4aTU3Wi9aTFByaC8wL2pzb3YKbExSTFdGVE8" + "2aldLCi0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K" + )); + Y_THROW(f.validate_public_key_data("invalid1", "invalid")); + // the following are both valid base64 + Y_THROW(f.validate_public_key_data("invalid2", "YmwK")); + Y_THROW(f.validate_public_key_data("invalid3", "Y m x h a A o = ")); + + // validate_private_key_data + N_THROW(f.validate_private_key_data( + "address@hidden", + "LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUNkUUl" + "CQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQWw4d2dnSmJBZ0" + "VBQW9HQkFOTHdueHdqUm9CdlU5M24KbFFUUytYRWNjRkwxb" + "klNb1pCZE4vZDBEN0xBWjVkOHBpTUVJQmlCeG1odEhGVkxP" + "SXBuWHRKOWdWSTVyUWU4dwpCZktTa2R6NVRMZEljZWNWSWx" + "2N0FWaWtsMVN2RjV4a3luT0hKSUpIajJYRHhvV2dhYW14bG" + "1ZMktGNnVtNC9ZCkxNc01KckVnclYxdE9DS1V4M1IxK2pMY" + "nZzejFBZ01CQUFFQ2dZQUFsTlZyYm91SU15bm9IMTZURW43" + "NUlzeVkKd0U3K0tVRDN2VURpRGNRQytuYi9uak81bGZUYWc" + "3M3Yva1d1Tjc3YmpxZCtQQkpLUWNFTlV0ejMyaE45elBWSQ" + "p5SzFRa1E4MmRlNHRCYlY4dFlDbmdXSFB3VWwxOHRrcFpzU" + "HJpd3E1MUpWOC9SdTdUanpRZDNHLzExQVdxcnFpCm9mMGtI" + "bC9PODBKbDNRZWJ3UUpCQU9pcEc1RlkzY1hOY0QwTjRiWjl" + "YMjZ6WWpNQWlBTG5WbktGcGpGblFqTUkKcVhCRitraWI2SU" + "11ZnZaRm1nT09LWG9vdzlyY3EyY2RwRlJ3bFVWQXdoRUNRU" + "URvR2JZNXhDNFoxMEVuQjErVAp4dGx5SEZzQW9LMXY3eGtG" + "c3RZV3hacXJUZ1hNemVkdkxiU2dHZ1lzMFNrZnlyQVFtREQ" + "yNGpjL25SOW0yNG0zCnJqaWxBa0JFZDI5cmFIRnJBamZqWD" + "dCcW1aNTUzMFFvcWlGY2FXT2hNLzlpVG5iR3VlZlM2R1RzO" + "VNTSlppZHEKcGJUYkV2elZ2Q1ZXeE5XVDlMOGxNalJiT3VG" + "aEFrQUZJcHgvaHJHbWJMYktVRVZ6RlpFMkR4Nk1Vd0hEV2p" + "6cApmVjF6UDRmK2hrbG1rSit3UEFpbENpNWN5M3ZuY2lxWE" + "UyYng3MnRkZ3ZKdzZpYVA0OURwQWtCTFlWZ3NaNHErCkxkL" + "0VYWFJibTJGOEd6MjVCaTFNV0p5OWxQOXBoY2FPaDdpZlBh" + "bVZDeTRlUGx4aTU3Wi9aTFByaC8wL2pzb3YKbExSTFdGVE8" + "2aldLCi0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K" + )); + // this is a public key + Y_THROW(f.validate_private_key_data( + "invalid0", + "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDS8J8cI0a" + "Ab1Pd55UE0vlxHHBS9ZyDKGQXTf3dA+ywGeXfKYjBCAYgcZ" + "obRxVSziKZ17SfYFSOa0HvMAXykpHc+Uy3SHHnFSJb+wFYp" + "JdUrxecZMpzhySCR49lw8aFoGmpsZZmNiherpuP2CzLDCax" + "IK1dbTgilMd0dfoy277M9QIDAQAB" + )); + Y_THROW(f.validate_private_key_data("invalid1", "invalid")); + // the following are both valid base64 + Y_THROW(f.validate_private_key_data("invalid2", "YmwK")); + Y_THROW(f.validate_private_key_data("invalid3", "Y m x h a A o = ")); + + // validate_certname N_THROW(f.validate_certname("graydon-at-venge-dot-net")); N_THROW(f.validate_certname("ABCDEFGHIJKLMNOPQRSTUVWXYZ" @@ -134,16 +224,49 @@ UNIT_TEST(roundabout) keypair kp; // a public key packet - kp.pub = rsa_pub_key("this is not a real rsa key"); - pw.consume_public_key(key_name("address@hidden"), kp.pub); + kp.pub = rsa_pub_key(decode_base64_as( + "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDS8J8cI0a" + "Ab1Pd55UE0vlxHHBS9ZyDKGQXTf3dA+ywGeXfKYjBCAYgcZ" + "obRxVSziKZ17SfYFSOa0HvMAXykpHc+Uy3SHHnFSJb+wFYp" + "JdUrxecZMpzhySCR49lw8aFoGmpsZZmNiherpuP2CzLDCax" + "IK1dbTgilMd0dfoy277M9QIDAQAB", origin::internal + ), origin::internal); + pw.consume_public_key(key_name("address@hidden"), kp.pub); // a keypair packet - kp.priv = rsa_priv_key("this is not a real rsa key either!"); - pw.consume_key_pair(key_name("address@hidden"), kp); + kp.priv = rsa_priv_key(decode_base64_as( + "LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUNkUUl" + "CQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQWw4d2dnSmJBZ0" + "VBQW9HQkFOTHdueHdqUm9CdlU5M24KbFFUUytYRWNjRkwxb" + "klNb1pCZE4vZDBEN0xBWjVkOHBpTUVJQmlCeG1odEhGVkxP" + "SXBuWHRKOWdWSTVyUWU4dwpCZktTa2R6NVRMZEljZWNWSWx" + "2N0FWaWtsMVN2RjV4a3luT0hKSUpIajJYRHhvV2dhYW14bG" + "1ZMktGNnVtNC9ZCkxNc01KckVnclYxdE9DS1V4M1IxK2pMY" + "nZzejFBZ01CQUFFQ2dZQUFsTlZyYm91SU15bm9IMTZURW43" + "NUlzeVkKd0U3K0tVRDN2VURpRGNRQytuYi9uak81bGZUYWc" + "3M3Yva1d1Tjc3YmpxZCtQQkpLUWNFTlV0ejMyaE45elBWSQ" + "p5SzFRa1E4MmRlNHRCYlY4dFlDbmdXSFB3VWwxOHRrcFpzU" + "HJpd3E1MUpWOC9SdTdUanpRZDNHLzExQVdxcnFpCm9mMGtI" + "bC9PODBKbDNRZWJ3UUpCQU9pcEc1RlkzY1hOY0QwTjRiWjl" + "YMjZ6WWpNQWlBTG5WbktGcGpGblFqTUkKcVhCRitraWI2SU" + "11ZnZaRm1nT09LWG9vdzlyY3EyY2RwRlJ3bFVWQXdoRUNRU" + "URvR2JZNXhDNFoxMEVuQjErVAp4dGx5SEZzQW9LMXY3eGtG" + "c3RZV3hacXJUZ1hNemVkdkxiU2dHZ1lzMFNrZnlyQVFtREQ" + "yNGpjL25SOW0yNG0zCnJqaWxBa0JFZDI5cmFIRnJBamZqWD" + "dCcW1aNTUzMFFvcWlGY2FXT2hNLzlpVG5iR3VlZlM2R1RzO" + "VNTSlppZHEKcGJUYkV2elZ2Q1ZXeE5XVDlMOGxNalJiT3VG" + "aEFrQUZJcHgvaHJHbWJMYktVRVZ6RlpFMkR4Nk1Vd0hEV2p" + "6cApmVjF6UDRmK2hrbG1rSit3UEFpbENpNWN5M3ZuY2lxWE" + "UyYng3MnRkZ3ZKdzZpYVA0OURwQWtCTFlWZ3NaNHErCkxkL" + "0VYWFJibTJGOEd6MjVCaTFNV0p5OWxQOXBoY2FPaDdpZlBh" + "bVZDeTRlUGx4aTU3Wi9aTFByaC8wL2pzb3YKbExSTFdGVE8" + "2aldLCi0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K", origin::internal + ), origin::internal); + pw.consume_key_pair(key_name("address@hidden"), kp); // an old privkey packet old_arc4_rsa_priv_key oldpriv("and neither is this!"); - pw.consume_old_private_key(key_name("address@hidden"), oldpriv); + pw.consume_old_private_key(key_name("address@hidden"), oldpriv); tmp = oss.str(); }