From f2ebb6e4544fe58aa66b75b089543203ea7f72fd Mon Sep 17 00:00:00 2001 From: jrconlin Date: Thu, 17 Mar 2022 17:36:44 -0700 Subject: [PATCH 1/5] feat: return more explicit VAPID error message Previously, we would accept invalidly formatted VAPID values (e.g. a string numeric value for `exp`.) With rust, we can no longer do that. Return an error that's more descriptive of what the problem is so that folk can fix it. Closes #1707 --- Cargo.lock | 226 +++++++++++--------- autoendpoint/Cargo.toml | 2 +- autoendpoint/src/extractors/subscription.rs | 66 +++++- autoendpoint/src/headers/vapid.rs | 2 + 4 files changed, 188 insertions(+), 108 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b71dbc1a9..04be41e3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -122,8 +122,8 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4ca8ce00b267af8ccebbd647de0d61e0674b6e61185cc7a592ff88772bed655" dependencies = [ - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -291,8 +291,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad26f77093333e0e7c6ffe54ebe3582d908a104e448723eec6d43d08b07143fb" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -379,8 +379,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "061a7acccaa286c011ddc30970520b98fa40e00c9d644633fb26b5fc63a265e3" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -943,12 +943,12 @@ dependencies = [ [[package]] name = "crossbeam-channel" -version = "0.5.2" +version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e54ea8bc3fb1ee042f5aace6e3c6e025d3874866da222930f70ce62aceba0bfa" +checksum = "fdbfe11fe19ff083c48923cf179540e8cd0535903dc35e178a1fdeeb59aef51f" dependencies = [ "cfg-if 1.0.0", - "crossbeam-utils 0.8.7", + "crossbeam-utils 0.8.8", ] [[package]] @@ -1020,9 +1020,9 @@ dependencies = [ [[package]] name = "crossbeam-utils" -version = "0.8.7" +version = "0.8.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5e5bed1f1c269533fa816a0a5492b3545209a205ca1a54842be180eb63a16a6" +checksum = "0bf124c720b7686e3c2663cf54062ab0f68a88af2fb6a030e87e30bf721fcb38" dependencies = [ "cfg-if 1.0.0", "lazy_static", @@ -1109,9 +1109,9 @@ checksum = "4fb810d30a7c1953f91334de7244731fc3f3c10d7fe163338a35b9f640960321" dependencies = [ "convert_case", "proc-macro2 1.0.36", - "quote 1.0.15", + "quote 1.0.16", "rustc_version 0.4.0", - "syn 1.0.86", + "syn 1.0.89", ] [[package]] @@ -1121,7 +1121,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1220ad071cb8996454c20adf547a34ba3ac793759dab793d9dc04996a373ac83" dependencies = [ "darling", - "heck", + "heck 0.3.3", "petgraph", "proc-macro2 0.4.30", "quote 0.6.13", @@ -1185,12 +1185,12 @@ dependencies = [ [[package]] name = "dirs-sys" -version = "0.3.6" +version = "0.3.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03d86534ed367a67548dc68113a0f5db55432fdfbb6e6f9d77704397d95d5780" +checksum = "1b1d1d91c932ef41c0f2663aa8b0ca0342d444d842c06914aa0a7e352d0bada6" dependencies = [ "libc", - "redox_users 0.4.0", + "redox_users 0.4.2", "winapi 0.3.9", ] @@ -1201,7 +1201,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4ebda144c4fe02d1f7ea1a7d9641b6fc6b580adcfa024ae48797ecdeb6825b4d" dependencies = [ "libc", - "redox_users 0.4.0", + "redox_users 0.4.2", "winapi 0.3.9", ] @@ -1252,14 +1252,14 @@ dependencies = [ [[package]] name = "enum-as-inner" -version = "0.3.3" +version = "0.3.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c5f0096a91d210159eceb2ff5e1c4da18388a170e1e3ce948aac9c8fdbbf595" +checksum = "570d109b813e904becc80d8d5da38376818a143348413f7149f1340fe04754d4" dependencies = [ - "heck", + "heck 0.4.0", "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -1277,9 +1277,9 @@ dependencies = [ [[package]] name = "erased-serde" -version = "0.3.18" +version = "0.3.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "56047058e1ab118075ca22f9ecd737bcc961aa3566a3019cb71388afa280bd8a" +checksum = "ad132dd8d0d0b546348d7d86cb3191aad14b34e5f979781fc005c80d4ac67ffd" dependencies = [ "serde 1.0.136", ] @@ -1311,8 +1311,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aa4da3c766cd7a0db8242e326e9e4e081edd567072893ed320008189715366a4" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", "synstructure", ] @@ -1523,8 +1523,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "33c1e13800337f4d4d7a316bf45a567dbcb6ffe087f16424852d97e97a91f512" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -1675,6 +1675,12 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "heck" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2540771e65fc8cb83cd6e8a237f70c319bd5c29f78ed1084ba5d50eeac86f7f9" + [[package]] name = "hermit-abi" version = "0.1.19" @@ -2006,9 +2012,9 @@ dependencies = [ [[package]] name = "ipnet" -version = "2.3.1" +version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68f2d64f2edebec4ce84ad108148e67e1064789bee435edc5b60ad398714a3a9" +checksum = "35e70ee094dc02fd9c13fdad4940090f22dbd6ac7c9e7094a46cf0232a50bc7c" [[package]] name = "itoa" @@ -2033,11 +2039,11 @@ dependencies = [ [[package]] name = "jsonwebtoken" -version = "7.2.0" +version = "8.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "afabcc15e437a6484fc4f12d0fd63068fe457bf93f1c148d3d9649c60b103f32" +checksum = "012bb02250fdd38faa5feee63235f7a459974440b9b57593822414c31f92839e" dependencies = [ - "base64 0.12.3", + "base64 0.13.0", "pem", "ring", "serde 1.0.136", @@ -2082,9 +2088,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.119" +version = "0.2.120" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bf2e165bb3457c8e098ea76f3e3bc9db55f87aa90d52d0e6be741470916aaa4" +checksum = "ad5c14e80759d0939d013e6ca49930e59fc53dd8e5009132f76240c179380c09" [[package]] name = "linked-hash-map" @@ -2279,8 +2285,8 @@ checksum = "7c461918bf7f59eefb1459252756bf2351a995d6bd510d0b2061bd86bcdabfa6" dependencies = [ "cfg-if 0.1.10", "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -2360,9 +2366,9 @@ checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" [[package]] name = "num-bigint" -version = "0.2.6" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "090c7f9998ee0ff65aa5b723e4009f7b217707f1fb5ea551329cc4d6231fb304" +checksum = "f93ab6289c7b344a8a9f60f88d80aa20032336fe78da341afc91c8a2341fc75f" dependencies = [ "autocfg 1.1.0", "num-integer", @@ -2409,9 +2415,9 @@ dependencies = [ [[package]] name = "num_threads" -version = "0.1.3" +version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97ba99ba6393e2c3734791401b66902d981cb03bf190af674ca69949b6d5fb15" +checksum = "aba1801fb138d8e85e11d0fc70baf4fe1cdfffda7c6cd34a854905df588e5ed0" dependencies = [ "libc", ] @@ -2427,9 +2433,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.9.0" +version = "1.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da32515d9f6e6e489d7bc9d84c71b060db7247dc035bbe44eac88cf87486d8d5" +checksum = "87f3e037eac156d1775da914196f0f37741a274155e34a0b7e427c35d2a2ecb9" [[package]] name = "opaque-debug" @@ -2528,20 +2534,18 @@ dependencies = [ "cfg-if 1.0.0", "instant", "libc", - "redox_syscall 0.2.10", + "redox_syscall 0.2.11", "smallvec 1.8.0", "winapi 0.3.9", ] [[package]] name = "pem" -version = "0.8.3" +version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd56cbd21fea48d0c440b41cd69c589faacade08c992d9a54e471b79d0fd13eb" +checksum = "e9a3b09a20e374558580a4914d3b7d89bd61b954a5a5e1dcbea98753addb1947" dependencies = [ "base64 0.13.0", - "once_cell", - "regex", ] [[package]] @@ -2591,8 +2595,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "044964427019eed9d49d9d5bbce6047ef18f37100ea400912a9fa4a3523ab12a" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -2602,8 +2606,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "744b6f092ba29c3650faf274db506afd39944f48420f6c86b17cfe0ee1cb36bb" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -2673,8 +2677,8 @@ checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" dependencies = [ "proc-macro-error-attr", "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", "version_check", ] @@ -2685,7 +2689,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", + "quote 1.0.16", "version_check", ] @@ -2729,6 +2733,15 @@ version = "1.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" +[[package]] +name = "quickcheck" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6" +dependencies = [ + "rand 0.8.5", +] + [[package]] name = "quote" version = "0.6.13" @@ -2740,9 +2753,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.15" +version = "1.0.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "864d3e96a899863136fc6e99f3d7cae289dafe43bf2c5ac19b70df7210c0a145" +checksum = "b4af2ec4714533fcdf07e886f17025ace8b997b9ce51204ee69b6da831c3da57" dependencies = [ "proc-macro2 1.0.36", ] @@ -2963,9 +2976,9 @@ checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce" [[package]] name = "redox_syscall" -version = "0.2.10" +version = "0.2.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8383f39639269cde97d255a32bdb68c047337295414940c68bdd30c2e13203ff" +checksum = "8380fe0152551244f0747b1bf41737e0f8a74f97a14ccefd1148187271634f3c" dependencies = [ "bitflags", ] @@ -2983,19 +2996,20 @@ dependencies = [ [[package]] name = "redox_users" -version = "0.4.0" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "528532f3d801c87aec9def2add9ca802fe569e44a544afe633765267840abe64" +checksum = "7776223e2696f1aa4c6b0170e83212f47296a00424305117d013dfe86fb0fe55" dependencies = [ "getrandom 0.2.5", - "redox_syscall 0.2.10", + "redox_syscall 0.2.11", + "thiserror", ] [[package]] name = "regex" -version = "1.5.4" +version = "1.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d07a8629359eb56f1e2fb1652bb04212c072a87ba68546a04065d525673ac461" +checksum = "1a11647b6b25ff05a515cb92c365cec08801e83423a235b51e231e1808747286" dependencies = [ "aho-corasick", "memchr", @@ -3298,7 +3312,7 @@ dependencies = [ "base64 0.13.0", "blake2b_simd", "constant_time_eq", - "crossbeam-utils 0.8.7", + "crossbeam-utils 0.8.8", ] [[package]] @@ -3662,8 +3676,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08597e7152fcd306f41838ed3e37be9eaeed2b61c42e2117266a554fab4662f9" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -3814,13 +3828,14 @@ dependencies = [ [[package]] name = "simple_asn1" -version = "0.4.1" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "692ca13de57ce0613a363c8c2f1de925adebc81b04c923ac60c5488bb44abe4b" +checksum = "4a762b1c38b9b990c694b9c2f8abe3372ce6a9ceaae6bca39cfc46e054f45745" dependencies = [ - "chrono", "num-bigint", "num-traits 0.2.14", + "thiserror", + "time 0.3.7", ] [[package]] @@ -4001,10 +4016,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c87a60a40fccc84bef0652345bbbbbe20a605bf5d0ce81719fc476f5c03b50ef" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", + "quote 1.0.16", "serde 1.0.136", "serde_derive", - "syn 1.0.86", + "syn 1.0.89", ] [[package]] @@ -4015,12 +4030,12 @@ checksum = "58fa5ff6ad0d98d1ffa8cb115892b6e69d67799f6763e162a1c9db421dc22e11" dependencies = [ "base-x", "proc-macro2 1.0.36", - "quote 1.0.15", + "quote 1.0.16", "serde 1.0.136", "serde_derive", "serde_json", "sha1", - "syn 1.0.86", + "syn 1.0.89", ] [[package]] @@ -4069,12 +4084,12 @@ dependencies = [ [[package]] name = "syn" -version = "1.0.86" +version = "1.0.89" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a65b3f4ffa0092e9887669db0eae07941f023991ab58ea44da8fe8e2d511c6b" +checksum = "ea297be220d52398dcc07ce15a209fce436d361735ac1db700cab3b6cdfb9f54" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", + "quote 1.0.16", "unicode-xid 0.2.2", ] @@ -4085,8 +4100,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f36bdaa60a83aca3921b5259d5400cbf5e90fc51931376a9bd4a0eb79aa7210f" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", "unicode-xid 0.2.2", ] @@ -4105,7 +4120,7 @@ dependencies = [ "cfg-if 1.0.0", "fastrand", "libc", - "redox_syscall 0.2.10", + "redox_syscall 0.2.11", "remove_dir_all", "winapi 0.3.9", ] @@ -4123,9 +4138,9 @@ dependencies = [ [[package]] name = "termcolor" -version = "1.1.2" +version = "1.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dfed899f0eb03f32ee8c6a0aabdb8a7949659e3466561fc0adf54e26d88c5f4" +checksum = "bab24d30b911b2376f3a13cc2cd443142f0c81dda04c118693e35b3835757755" dependencies = [ "winapi-util", ] @@ -4152,8 +4167,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aa32fd3f627f367fe16f893e2597ae3c05020f8bba2666a4e6ea73d377e5714b" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -4209,6 +4224,7 @@ dependencies = [ "itoa 1.0.1", "libc", "num_threads", + "quickcheck", "time-macros 0.2.3", ] @@ -4236,9 +4252,9 @@ checksum = "fd3c141a1b43194f3f56a1411225df8646c55781d5f26db825b3d98507eb482f" dependencies = [ "proc-macro-hack", "proc-macro2 1.0.36", - "quote 1.0.15", + "quote 1.0.16", "standback", - "syn 1.0.86", + "syn 1.0.89", ] [[package]] @@ -4394,8 +4410,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e44da00bfc73a25f814cd8d7e57a68a5c31b74b3152a0a1d1f590c97ed06265a" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", ] [[package]] @@ -4635,9 +4651,9 @@ checksum = "360dfd1d6d30e05fda32ace2c8c70e9c0a9da713275777f5a4dbb8a1893930c6" [[package]] name = "tracing" -version = "0.1.31" +version = "0.1.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6c650a8ef0cd2dd93736f033d21cbd1224c5a967aa0c258d00fcf7dafef9b9f" +checksum = "4a1bdf54a7c28a2bbf701e1d2233f6c77f473486b94bee4f9678da5a148dca7f" dependencies = [ "cfg-if 1.0.0", "log", @@ -4647,9 +4663,9 @@ dependencies = [ [[package]] name = "tracing-core" -version = "0.1.22" +version = "0.1.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "03cfcb51380632a72d3111cb8d3447a8d908e577d31beeac006f836383d29a23" +checksum = "aa31669fa42c09c34d94d8165dd2012e8ff3c66aca50f3bb226b68f216f2706c" dependencies = [ "lazy_static", ] @@ -4875,9 +4891,9 @@ dependencies = [ "lazy_static", "proc-macro-error", "proc-macro2 1.0.36", - "quote 1.0.15", + "quote 1.0.16", "regex", - "syn 1.0.86", + "syn 1.0.89", "validator_types", ] @@ -4888,7 +4904,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ded9d97e1d42327632f5f3bae6403c04886e2de3036261ef42deebd931a6a291" dependencies = [ "proc-macro2 1.0.36", - "syn 1.0.86", + "syn 1.0.89", ] [[package]] @@ -4958,8 +4974,8 @@ dependencies = [ "lazy_static", "log", "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", "wasm-bindgen-shared", ] @@ -4981,7 +4997,7 @@ version = "0.2.79" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2f4203d69e40a52ee523b2529a773d5ffc1dc0071801c87b3d270b471b80ed01" dependencies = [ - "quote 1.0.15", + "quote 1.0.16", "wasm-bindgen-macro-support", ] @@ -4992,8 +5008,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bfa8a30d46208db204854cadbb5d4baf5fcf8071ba5bf48190c3e59937962ebc" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -5183,9 +5199,9 @@ dependencies = [ [[package]] name = "zeroize" -version = "1.5.2" +version = "1.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c88870063c39ee00ec285a2f8d6a966e5b6fb2becc4e8dac77ed0d370ed6006" +checksum = "7eb5728b8afd3f280a869ce1d4c554ffaed35f45c231fc41bfbd0381bef50317" dependencies = [ "zeroize_derive", ] @@ -5197,7 +5213,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f8f187641dad4f680d25c4bfc4225b418165984179f26ca76ec4fb6441d3a17" dependencies = [ "proc-macro2 1.0.36", - "quote 1.0.15", - "syn 1.0.86", + "quote 1.0.16", + "syn 1.0.89", "synstructure", ] diff --git a/autoendpoint/Cargo.toml b/autoendpoint/Cargo.toml index 5d474a752..84392d0e4 100644 --- a/autoendpoint/Cargo.toml +++ b/autoendpoint/Cargo.toml @@ -28,7 +28,7 @@ docopt = "1.1.0" fernet = "0.1.3" futures = "0.3" hex = "0.4.2" -jsonwebtoken = "7.2" +jsonwebtoken = "8.0" lazy_static = "1.4.0" log = "0.4" openssl = "0.10" diff --git a/autoendpoint/src/extractors/subscription.rs b/autoendpoint/src/extractors/subscription.rs index a674746e4..e2829cdee 100644 --- a/autoendpoint/src/extractors/subscription.rs +++ b/autoendpoint/src/extractors/subscription.rs @@ -221,11 +221,29 @@ fn validate_vapid_jwt(vapid: &VapidHeaderWithKey, domain: &Url) -> ApiResult<()> let public_key = base64::decode_config(public_key, base64::URL_SAFE_NO_PAD) .map_err(|_| VapidError::InvalidKey)?; // NOTE: This will fail if `exp` is specified as a string instead of a numeric. - let token_data = jsonwebtoken::decode::( + let token_data = match jsonwebtoken::decode::( &vapid.token, &DecodingKey::from_ec_der(&public_key), &Validation::new(Algorithm::ES256), - )?; + ) { + Ok(v) => v, + Err(e) => match e.clone().into_kind() { + jsonwebtoken::errors::ErrorKind::Json(e) => { + if e.is_data() { + // Data returns normally for string as numeric. Vapid only has + // one of those right now, so it's safe to presume. + return Err(VapidError::InvalidVapid( + "'exp' may be a string. Remove quotes and try again.".to_owned(), + ) + .into()); + } + // Other errors are always possible. Try to be helpful by returning + // the Json parse error. + return Err(VapidError::InvalidVapid(e.to_string()).into()); + } + _ => return Err(e.into()), + }, + }; // Make sure the expiration isn't too far into the future if token_data.claims.exp > (sec_since_epoch() + ONE_DAY_IN_SECONDS) { @@ -256,6 +274,7 @@ mod tests { use crate::extractors::subscription::repad_base64; use crate::headers::vapid::{VapidError, VapidHeader, VapidHeaderWithKey, VapidVersionData}; use autopush_common::util::sec_since_epoch; + use serde::{Deserialize, Serialize}; use std::str::FromStr; use url::Url; @@ -334,4 +353,47 @@ mod tests { ApiErrorKind::VapidError(VapidError::InvalidAudience) )); } + + #[test] + fn vapid_exp_is_string() { + #[derive(Debug, Deserialize, Serialize)] + struct StrExpVapidClaims { + exp: String, + aud: String, + sub: String, + } + + let priv_key = base64::decode_config( + "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgZImOgpRszunnU3j1\ + oX5UQiX8KU4X2OdbENuvc/t8wpmhRANCAATN21Y1v8LmQueGpSG6o022gTbbYa4l\ + bXWZXITsjknW1WHmELtouYpyXX7e41FiAMuDvcRwW2Nfehn/taHW/IXb", + base64::STANDARD, + ) + .unwrap(); + let public_key = "BM3bVjW_wuZC54alIbqjTbaBNtthriVtdZlchOyOSdbVYeYQu2i5inJdft7jUWIAy4O9xHBbY196Gf-1odb8hds".to_owned(); + let domain = "https://push.services.mozilla.org"; + let jwk_header = jsonwebtoken::Header::new(jsonwebtoken::Algorithm::ES256); + let enc_key = jsonwebtoken::EncodingKey::from_ec_der(&priv_key); + let claims = StrExpVapidClaims { + exp: (sec_since_epoch() + super::ONE_DAY_IN_SECONDS - 100).to_string(), + aud: domain.to_owned(), + sub: "mailto:admin@example.com".to_owned(), + }; + let token = jsonwebtoken::encode(&jwk_header, &claims, &enc_key).unwrap(); + let header = VapidHeaderWithKey { + public_key, + vapid: VapidHeader { + scheme: "vapid".to_string(), + token, + version_data: VapidVersionData::Version1, + }, + }; + let vv = validate_vapid_jwt(&header, &Url::from_str("http://example.org").unwrap()) + .unwrap_err() + .kind; + assert!(matches![ + vv, + ApiErrorKind::VapidError(VapidError::InvalidVapid(_)) + ]) + } } diff --git a/autoendpoint/src/headers/vapid.rs b/autoendpoint/src/headers/vapid.rs index 09c7b4db0..844f3c4e2 100644 --- a/autoendpoint/src/headers/vapid.rs +++ b/autoendpoint/src/headers/vapid.rs @@ -81,6 +81,8 @@ impl VapidHeader { pub enum VapidError { #[error("Missing VAPID token")] MissingToken, + #[error("Invalid VAPID token: {0}")] + InvalidVapid(String), #[error("Missing VAPID public key")] MissingKey, #[error("Invalid VAPID public key")] From deb7f507946e752bd1521ad3a88daa060180dbfa Mon Sep 17 00:00:00 2001 From: jrconlin Date: Thu, 17 Mar 2022 18:01:16 -0700 Subject: [PATCH 2/5] f handle `"sub":None` as well --- autoendpoint/src/extractors/subscription.rs | 49 +++++++++++++++++++-- 1 file changed, 45 insertions(+), 4 deletions(-) diff --git a/autoendpoint/src/extractors/subscription.rs b/autoendpoint/src/extractors/subscription.rs index e2829cdee..b931e2362 100644 --- a/autoendpoint/src/extractors/subscription.rs +++ b/autoendpoint/src/extractors/subscription.rs @@ -230,15 +230,13 @@ fn validate_vapid_jwt(vapid: &VapidHeaderWithKey, domain: &Url) -> ApiResult<()> Err(e) => match e.clone().into_kind() { jsonwebtoken::errors::ErrorKind::Json(e) => { if e.is_data() { - // Data returns normally for string as numeric. Vapid only has - // one of those right now, so it's safe to presume. return Err(VapidError::InvalidVapid( - "'exp' may be a string. Remove quotes and try again.".to_owned(), + "A value in the vapid claims is either missing or incorrectly specified (e.g. \"exp\":\"12345\" or \"sub\":None). Please correct and retry.".to_owned(), ) .into()); } // Other errors are always possible. Try to be helpful by returning - // the Json parse error. + // the Json parse error. return Err(VapidError::InvalidVapid(e.to_string()).into()); } _ => return Err(e.into()), @@ -396,4 +394,47 @@ mod tests { ApiErrorKind::VapidError(VapidError::InvalidVapid(_)) ]) } + + #[test] + fn vapid_missing_sub() { + #[derive(Debug, Deserialize, Serialize)] + struct NoSubVapidClaims { + exp: u64, + aud: String, + sub: Option, + } + + let priv_key = base64::decode_config( + "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgZImOgpRszunnU3j1\ + oX5UQiX8KU4X2OdbENuvc/t8wpmhRANCAATN21Y1v8LmQueGpSG6o022gTbbYa4l\ + bXWZXITsjknW1WHmELtouYpyXX7e41FiAMuDvcRwW2Nfehn/taHW/IXb", + base64::STANDARD, + ) + .unwrap(); + let public_key = "BM3bVjW_wuZC54alIbqjTbaBNtthriVtdZlchOyOSdbVYeYQu2i5inJdft7jUWIAy4O9xHBbY196Gf-1odb8hds".to_owned(); + let domain = "https://push.services.mozilla.org"; + let jwk_header = jsonwebtoken::Header::new(jsonwebtoken::Algorithm::ES256); + let enc_key = jsonwebtoken::EncodingKey::from_ec_der(&priv_key); + let claims = NoSubVapidClaims { + exp: sec_since_epoch() + super::ONE_DAY_IN_SECONDS - 100, + aud: domain.to_owned(), + sub: None, + }; + let token = jsonwebtoken::encode(&jwk_header, &claims, &enc_key).unwrap(); + let header = VapidHeaderWithKey { + public_key, + vapid: VapidHeader { + scheme: "vapid".to_string(), + token, + version_data: VapidVersionData::Version1, + }, + }; + let vv = validate_vapid_jwt(&header, &Url::from_str("http://example.org").unwrap()) + .unwrap_err() + .kind; + assert!(matches![ + vv, + ApiErrorKind::VapidError(VapidError::InvalidVapid(_)) + ]) + } } From 36cb892c85d930c1d2671263d055408c239777e3 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Fri, 18 Mar 2022 10:17:53 -0700 Subject: [PATCH 3/5] f Convert some errors to metric only Closes: #CONSRV-1708 --- autoendpoint/src/error.rs | 52 +++++++++++++++++++++ autoendpoint/src/extractors/subscription.rs | 1 + autoendpoint/src/middleware/sentry.rs | 35 ++++++++++++-- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/autoendpoint/src/error.rs b/autoendpoint/src/error.rs index 8496bc148..36f2c4aa7 100644 --- a/autoendpoint/src/error.rs +++ b/autoendpoint/src/error.rs @@ -152,6 +152,58 @@ impl ApiErrorKind { } } + /// Specify the label to use for metrics reporting. + pub fn metric_label<'a>(&self) -> &'a str { + match self { + ApiErrorKind::PayloadError(_) => "payload_error", + ApiErrorKind::Router(_) => "router", + + ApiErrorKind::Validation(_) => "validation", + ApiErrorKind::InvalidEncryption(_) => "invalid_encryption", + ApiErrorKind::NoTTL => "no_ttl", + ApiErrorKind::InvalidRouterType => "invalid_router_type", + ApiErrorKind::InvalidRouterToken => "invalid_router_token", + ApiErrorKind::InvalidMessageId => "invalid_message_id", + + ApiErrorKind::VapidError(_) => "vapid_error", + ApiErrorKind::Jwt(_) => "jwt", + ApiErrorKind::TokenHashValidation(_) => "token_hash_validation", + ApiErrorKind::InvalidAuthentication => "invalid_authentication", + ApiErrorKind::InvalidLocalAuth(_) => "invalid_local_auth", + + ApiErrorKind::InvalidToken => "invalid_token", + ApiErrorKind::InvalidApiVersion => "invalid_api_version", + + ApiErrorKind::NoUser => "no_user", + ApiErrorKind::NoSubscription => "no_subscription", + + ApiErrorKind::LogCheck => "log_check", + + ApiErrorKind::Io(_) => "io", + ApiErrorKind::Metrics(_) => "metrics", + ApiErrorKind::Database(_) => "database", + ApiErrorKind::EndpointUrl(_) => "endpoint_url", + ApiErrorKind::RegistrationSecretHash(_) => "registration_secret_hash", + } + } + + /// Don't report all errors to sentry + pub fn is_reportable(&self) -> bool { + !matches!( + self, + // Ignore common webpush errors + ApiErrorKind::NoTTL | ApiErrorKind::InvalidEncryption(_) | + // Ignore common VAPID erros + ApiErrorKind::VapidError(_) + | ApiErrorKind::Jwt(_) + | ApiErrorKind::TokenHashValidation(_) + | ApiErrorKind::InvalidAuthentication + | ApiErrorKind::InvalidLocalAuth(_) | + // Ignore missing or invalid user errors + ApiErrorKind::NoUser | ApiErrorKind::NoSubscription, + ) + } + /// Get the associated error number pub fn errno(&self) -> Option { match self { diff --git a/autoendpoint/src/extractors/subscription.rs b/autoendpoint/src/extractors/subscription.rs index b931e2362..f3aceccd8 100644 --- a/autoendpoint/src/extractors/subscription.rs +++ b/autoendpoint/src/extractors/subscription.rs @@ -432,6 +432,7 @@ mod tests { let vv = validate_vapid_jwt(&header, &Url::from_str("http://example.org").unwrap()) .unwrap_err() .kind; + dbg!(vv.to_string()); assert!(matches![ vv, ApiErrorKind::VapidError(VapidError::InvalidVapid(_)) diff --git a/autoendpoint/src/middleware/sentry.rs b/autoendpoint/src/middleware/sentry.rs index a06f8192a..fc8b7d631 100644 --- a/autoendpoint/src/middleware/sentry.rs +++ b/autoendpoint/src/middleware/sentry.rs @@ -1,5 +1,6 @@ use crate::error::ApiError; use actix_web::dev::{Service, ServiceRequest, ServiceResponse}; +use cadence::CountedExt; use sentry::protocol::Event; use sentry::Hub; use std::future::Future; @@ -22,22 +23,46 @@ pub fn sentry_middleware( scope.add_event_processor(Box::new(move |event| process_event(event, &sentry_request))) }); + let state = request + .app_data::>() + .cloned(); let response = service.call(request); - async move { // Wait for the response and check for errors let response: ServiceResponse = match response.await { Ok(response) => response, - Err(e) => { - debug!("Reporting error to Sentry (service error): {}", e); - let event_id = hub.capture_event(event_from_actix_error(&e)); + Err(error) => { + if let Some(api_err) = error.as_error::() { + // if it's not reportable, and we have access to the metrics, record it as a metric. + if !api_err.kind.is_reportable() { + if let Some(state) = state { + match state + .metrics + .incr(&format!("api_error.{}", api_err.kind.metric_label())) + { + Ok(_) | Err(_) => {} + }; + } + debug!("Not reporting error (service error): {:?}", error); + return Err(error); + } + } + debug!("Reporting error to Sentry (service error): {}", error); + let event_id = hub.capture_event(event_from_actix_error(&error)); trace!("event_id = {}", event_id); - return Err(e); + return Err(error); } }; // Check for errors inside the response if let Some(error) = response.response().error() { + if let Some(api_err) = error.as_error::() { + if !api_err.kind.is_reportable() { + debug!("Not reporting error (service error): {:?}", error); + drop(_scope_guard); + return Ok(response); + } + } debug!("Reporting error to Sentry (response error): {}", error); let event_id = hub.capture_event(event_from_actix_error(error)); trace!("event_id = {}", event_id); From bd3a2cc82c0068297a7560c87f98a5dea8e7c720 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Fri, 18 Mar 2022 10:45:26 -0700 Subject: [PATCH 4/5] f kill dbg --- autoendpoint/src/extractors/subscription.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/autoendpoint/src/extractors/subscription.rs b/autoendpoint/src/extractors/subscription.rs index f3aceccd8..b931e2362 100644 --- a/autoendpoint/src/extractors/subscription.rs +++ b/autoendpoint/src/extractors/subscription.rs @@ -432,7 +432,6 @@ mod tests { let vv = validate_vapid_jwt(&header, &Url::from_str("http://example.org").unwrap()) .unwrap_err() .kind; - dbg!(vv.to_string()); assert!(matches![ vv, ApiErrorKind::VapidError(VapidError::InvalidVapid(_)) From 5312c1489723d751d61e8a7247524c9ea1a82f18 Mon Sep 17 00:00:00 2001 From: jrconlin Date: Fri, 18 Mar 2022 14:36:49 -0700 Subject: [PATCH 5/5] f r's --- autoendpoint/src/error.rs | 4 ++-- autoendpoint/src/extractors/subscription.rs | 4 ++-- autoendpoint/src/middleware/sentry.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/autoendpoint/src/error.rs b/autoendpoint/src/error.rs index 36f2c4aa7..36c80a890 100644 --- a/autoendpoint/src/error.rs +++ b/autoendpoint/src/error.rs @@ -153,7 +153,7 @@ impl ApiErrorKind { } /// Specify the label to use for metrics reporting. - pub fn metric_label<'a>(&self) -> &'a str { + pub fn metric_label(&self) -> &'static str { match self { ApiErrorKind::PayloadError(_) => "payload_error", ApiErrorKind::Router(_) => "router", @@ -188,7 +188,7 @@ impl ApiErrorKind { } /// Don't report all errors to sentry - pub fn is_reportable(&self) -> bool { + pub fn is_sentry_event(&self) -> bool { !matches!( self, // Ignore common webpush errors diff --git a/autoendpoint/src/extractors/subscription.rs b/autoendpoint/src/extractors/subscription.rs index b931e2362..69fca9165 100644 --- a/autoendpoint/src/extractors/subscription.rs +++ b/autoendpoint/src/extractors/subscription.rs @@ -227,11 +227,11 @@ fn validate_vapid_jwt(vapid: &VapidHeaderWithKey, domain: &Url) -> ApiResult<()> &Validation::new(Algorithm::ES256), ) { Ok(v) => v, - Err(e) => match e.clone().into_kind() { + Err(e) => match e.kind() { jsonwebtoken::errors::ErrorKind::Json(e) => { if e.is_data() { return Err(VapidError::InvalidVapid( - "A value in the vapid claims is either missing or incorrectly specified (e.g. \"exp\":\"12345\" or \"sub\":None). Please correct and retry.".to_owned(), + "A value in the vapid claims is either missing or incorrectly specified (e.g. \"exp\":\"12345\" or \"sub\":null). Please correct and retry.".to_owned(), ) .into()); } diff --git a/autoendpoint/src/middleware/sentry.rs b/autoendpoint/src/middleware/sentry.rs index fc8b7d631..702139ea8 100644 --- a/autoendpoint/src/middleware/sentry.rs +++ b/autoendpoint/src/middleware/sentry.rs @@ -34,7 +34,7 @@ pub fn sentry_middleware( Err(error) => { if let Some(api_err) = error.as_error::() { // if it's not reportable, and we have access to the metrics, record it as a metric. - if !api_err.kind.is_reportable() { + if !api_err.kind.is_sentry_event() { if let Some(state) = state { match state .metrics @@ -57,7 +57,7 @@ pub fn sentry_middleware( // Check for errors inside the response if let Some(error) = response.response().error() { if let Some(api_err) = error.as_error::() { - if !api_err.kind.is_reportable() { + if !api_err.kind.is_sentry_event() { debug!("Not reporting error (service error): {:?}", error); drop(_scope_guard); return Ok(response);