cert validation: use Android cert validation APIs #2525

danzh2010 · 2022-09-06T18:17:30Z

Description: add engine API to allow user config to use Android cert validation APIs.
Risk Level: high
Testing: added tests in Http2TestServerTest.java
Docs Changes:
Release Notes:
Fixes #1575
Part of #2144

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2022-09-06T18:20:48Z

/retest

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2022-09-07T02:51:40Z

/retest

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2022-09-07T14:59:11Z

/retest

RyanTheOptimist

Wow, this looks awesome. A number of mostly small comments. We'll definitely need to get some other folks on the review since it's touching a lot of EM stuff that I'm not an expert in, but this looks great!

RyanTheOptimist · 2022-09-06T20:34:38Z

envoy_build_config/extensions_build_config.bzl

@@ -24,5 +24,7 @@ EXTENSIONS = {
    "envoy.transport_sockets.raw_buffer":                  "//source/extensions/transport_sockets/raw_buffer:config",
    "envoy.transport_sockets.tls":                         "//source/extensions/transport_sockets/tls:config",
    "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:config",
+


nit: Remove newline?

RyanTheOptimist · 2022-09-07T14:00:15Z

library/common/config/templates.h

@@ -85,3 +85,7 @@ extern const char* socket_tag_config_insert;
 * direct responses to mutate headers which are then later used for routing.
 */
 extern const char* route_cache_reset_filter_insert;
+
+extern const char* default_cert_validation_context_template;


nit: comments, please

RyanTheOptimist · 2022-09-07T14:01:02Z

library/common/extensions/cert_validator/platform_bridge/c_types.h

+
+// NOLINT(namespace-envoy)
+
+typedef struct {


Comments on all the structs/functions, please

RyanTheOptimist · 2022-09-07T14:03:42Z

library/common/extensions/cert_validator/platform_bridge/config.cc

+CertValidatorPtr PlatformBridgeCertValidatorFactory::createCertValidator(
+    const Envoy::Ssl::CertificateValidationContextConfig* config, SslStats& stats,
+    TimeSource& /*time_source*/) {
+#if defined(__APPLE__)


Does the platform verifier work on non-Android platforms? If so, perhaps we should make this check "if not android"

Though we don't have non-Android platform supporting this, the current cronet tests do mock up the API calls so at least those cronet tests worked on non-Android platform. So I'm not going to enforce platform checking here.

Non-cronet enginer builders don't provide the interface to choose platform based cert validator after all.

RyanTheOptimist · 2022-09-07T14:04:39Z

library/common/extensions/cert_validator/platform_bridge/config.h

+  }
+
+private:
+  const envoy_cert_validator* platform_bridge_api_;


nit: since this member is actually the validator, I'd be inclined to name the member validator_, but your call.

library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.h

RyanTheOptimist · 2022-09-07T19:56:49Z

library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.h

+  // Return empty string
+  std::string getCaFileName() const override { return ""; }
+
+  void verifyCertChainByPlatform(


nit: comment, please.

RyanTheOptimist · 2022-09-07T19:57:09Z

library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.h

+private:
+  const Envoy::Ssl::CertificateValidationContextConfig* config_;
+  SslStats& stats_;
+  bool allow_untrusted_certificate_{false};


nit: does = false work here? If so, let's use it.

RyanTheOptimist · 2022-09-07T19:59:27Z

library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java

@@ -315,6 +315,8 @@ protected static native int recordHistogramValue(long engine, String elements, b
   */
  public static native String brotliConfigInsert();

+  public static native String certValidationTemplate(boolean use_platform);


nit: Comments here, please.

Since it's java I think that the convention is to use the camel case naming (see logLevel argument in one of the methods in this file). So let's rename to usePlatform (and update the corresponding doc string)

RyanTheOptimist · 2022-09-07T21:52:23Z

test/java/org/chromium/net/testing/Http2TestServerTest.java

@@ -78,6 +103,96 @@ public void getSchemeIsHttps() throws Exception {
    assertThat(response.getEnvoyError()).isNull();
  }

+  @Test
+  public void testGetRequest() throws Exception {
+    System.out.println("TEST_testGetRequest");


nit: presumably we should remove these printlns?

done. Though I really feel like it will always be needed. Hopefully there will be a better way to distinguish tests.

danzh2010 · 2022-09-07T23:09:32Z

/retest

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2022-10-05T21:30:09Z

/retest

Augustyniak

Thank you very much for simplifying the config template logic. It looks great now!

Left a few comments - mostly related to the registerAPI part and the logic responsible for the interpolation of strings in builders.

Augustyniak · 2022-10-06T14:55:16Z

library/cc/engine_builder.cc

+  const std::string& cert_validation_template =
+      (this->platform_certificates_validation_on_ ? platform_cert_validation_context_template
+                                                  : default_cert_validation_context_template);
+  config_builder << "- &validation_context" << cert_validation_template << std::endl;


Nice! I personally think that it is much simpler now - thank you for working on this!

Augustyniak · 2022-10-06T15:07:05Z

library/common/jni/jni_interface.cc

+  // TODO(danzh) this object leaks, but it's tied to the life of the engine.
+  envoy_cert_validator* api = (envoy_cert_validator*)safe_malloc(sizeof(envoy_cert_validator));
+  api->validate_cert = verify_x509_cert_chain;
+  api->release_validator = jvm_detach_thread;


In theory we could release the api in here too (as part of release_validator). Optional comment as you've already added TODO

release_validator is called at the end of each validation to detach thread. We shouldn't release the api during the life time of the engine.

Things might change if we stop creating a new thread for each validation, but coalesce validations to a few dedicated threads.

Augustyniak · 2022-10-06T15:19:02Z

library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.cc

+  bssl::UniquePtr<X509> leaf_cert(d2i_X509(
+      nullptr, const_cast<const unsigned char**>(&leaf_cert_der.bytes), leaf_cert_der.length));
+  envoy_cert_validation_result result =
+      platform_validator_->validate_cert(cert_chain.data(), cert_chain.size(), host_name.c_str());


I remember that last week (in the community meeting) there was an idea to remove the use of registerAPI calls at all and just make platform_bridge_cert_validator call into verify_x509_cert_chain directly. I think that socket tagging example was brought up https://github.com/envoyproxy/envoy-mobile/pull/2423/files#diff-bf08f22bd707e11bdfa9ac4d50f12976176e2722ad82d3e66f5b88457b98f9a0R30 as it does call into platform specific stuff directly from c++ code without going through a register.

Would that be possible to stop using registerApi and call verify_x509_cert_chain directly from here? If yes, maybe we should do it to simplify the logic - we are doing additional work to register the API and I am not sure whether we need to. Do you think that using register is helping us in some way?

I tried to remove registerApi, but ran into a very subtle difficulty in tests where we run cronet engine in MacOS, i.e. https://github.com/envoyproxy/envoy-mobile/actions/runs/3003635334/jobs/4822128405. If the platform_validator_ API gets loaded based on platform, those tests will not be able to load verify_x509_cert_chain. And to make things more complex, one day we will have IOS validation API. Those tests will be very confused about which API to load if the engine doesn't register the right one during initialization. I think java_tests_mac CI is the only blocker for me to get rid of registerApi stuff in this PR. Any thoughts?

For the record, not planning to keep blocking on this - just want to ask some questions so that I understand the current situation better. Thank you :)

The logs from https://github.com/envoyproxy/envoy-mobile/actions/runs/3003635334/jobs/4822128405 are expired - I cannot see them :( .

If the platform_validator_ API gets loaded based on platform, those tests will not be able to load verify_x509_cert_chain

Since I do not have access to the logs of the failing tests - could you elaborate on this? Any chance that you remember which tests were failing specifically? Some of our existing platform specific JNI utility methods branch on #if(_ANDROID_API_) check (example here) but not sure whether this would be useful in your case.

one day we will have IOS validation API

yes, that's when the API register will come in handy. I just wonder whether for now we could avoid that additional complexity as between now and then (when we have iOS platform cert validation) a lot can change.

Since I do not have access to the logs of the failing tests - could you elaborate on this? Any chance that you remember which tests were failing specifically?

Some tests under https://github.com/envoyproxy/envoy-mobile/tree/main/test/java/org/chromium/net/testing runs by android_tests / java_tests_mac CI, and they uses cronet engines which will always use platform API for cert validation.

Some of our existing platform specific JNI utility methods branch on #if(_ANDROID_API_) check (example here) but not sure whether this would be useful in your case.

The AndroidNetworkLibrary APIs has mock interfaces, so that even on non-Android platform we can still run Java tests which call into verify_x509_cert_chain as long as those tests setup the mock interface. And there is no need of #if(_ANDROID_API_) for all the verification related JNI calls. But if we want to call validation interfaces based on #if not defined(__APPLE__) or #if(_ANDROID_API_), the cronet engine tests running on MacOS will not be able to make the right JNI calls.

That being said, today we are not supporting iOS cert validation APIs, so a temporary solution to make java tests on MacOS happy could be hard coding those JNI calls instead of calling them in #ifdef block. As a result, any engine enables platform based cert validation will call into those JNI functions. But this PR doesn't enable swift engine to use platform based cert validation, and in C++ engine we check platform before enabling it. This won't work once we support iOS cert validation API but still want to run tests under https://github.com/envoyproxy/envoy-mobile/tree/main/test/java/org/chromium/net/testing on MacOS.

one day we will have IOS validation API

yes, that's when the API register will come in handy. I just wonder whether for now we could avoid that additional complexity as between now and then (when we have iOS platform cert validation) a lot can change.

Will there be any chance not to run tests under https://github.com/envoyproxy/envoy-mobile/tree/main/test/java/org/chromium/net/testing on MacOS in the future? If that's the case, we won't even need platform registry after we support iOS cert validation API. Those CIs are helpful for Android developer using Mac, but not very valuable in real device scenario.

Thank you for a detailed answer(s). I think that the current approach of registering filters makes sense if it helps us to have a better test coverage in here. I wish there was an easier way to make all of this works (actual implementation + tests) - maybe that's something that we will be able to make easier in the future.

The one remaining comment that I have left is the one with regard to moving the registration of the filter from its current place to ...initialize method instead? I left more details in #2525 (comment). What do you think about this one?

Will there be any chance not to run tests under https://github.com/envoyproxy/envoy-mobile/tree/main/test/java/org/chromium/net/testing on MacOS in the future?

If we decided that this is the way to go I think that there is nothing that should prevent us from doing so from the technical point of view. Not entirely sure why we run some of Android tests on macOS and some on Linux but I am guessing that we just wanted to cover testing on more platforms - not sure how useful is that, we can discuss in the community meeting if needed.

Augustyniak · 2022-10-06T15:26:21Z

library/common/jni/jni_interface.cc

+
+  set_vm(vm);
+  // At this point, we know Android APIs are available. Register cert chain validation JNI calls.
+  envoy_status_t result = jvm_register_cert_validator();


If we decided that we need to keep registerAPI logic in I think that we should move the registration to AndoridJNILibrary.initialize in here because:

The initialize method is all about the initialization so registering API sounds like a good fit for it.

The initialize method is Android specific and so is AndroidJNILibrary.initialize

find_class does not work before AndroidJNILibrary.initialize gets called and AndroidJNILibrary.initialize gets called after JNI_onLoad so in general if your implementation of jvm_register_cert_validator ever needs to access find_class it will fail.

As we decided to keep using registerAPI, doing that during AndoridJNILibrary.initialize makes sense. In order do to so, I have to move the relevant JNI calls from jni_interface.cc to android_jni_utility.cc. PTAL!

Augustyniak · 2022-10-06T15:43:10Z

library/objective-c/EnvoyConfiguration.m

@@ -192,6 +192,12 @@ - (nullable NSString *)resolveTemplate:(NSString *)templateYAML {
  [definitions
      appendFormat:@"- &stats_flush_interval %lus\n", (unsigned long)self.statsFlushSeconds];

+  NSString *cert_validator_template =
+      [[NSString alloc] initWithUTF8String:default_cert_validation_context_template];
+  [definitions appendFormat:@"- &validation_context_config_trust_chain%@\n"


This looks different compared to what we have in Java file https://github.com/envoyproxy/envoy-mobile/pull/2525/files#diff-33c52309a2e0d09eab91df02e32e8f571149afbe5a23f69509550d0f328622aeR291 and what we have in c++ builder https://github.com/envoyproxy/envoy-mobile/pull/2525/files#diff-204f30e7778ed8d1f3e6008076942d0cd78f18df61826d44ef7f2d54b2b9d2fbR198

Shouldn't they be more 1 to 1?

I think that the c++ builder is what we want other builders to look like.

Augustyniak · 2022-10-06T15:48:27Z

library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java

@@ -284,6 +287,9 @@ String resolveTemplate(final String configTemplate, final String platformFilterT
      configBuilder.append("] \n");
    }

+    // Add a new anchor to override the default anchors in config header.


Don't we need to prepend it with &validation_context or similar?

Sorry, I myself got confused about what was included in the template and what wasn't. &validation_context is already included in the template string. So in the engine implementations, there is no need to append it.
The Java engine here is doing the right thing. The C++ engine shouldn't have it appended again, though it seems still to be a valid YAML as the unit test passed. And the swift engine was out-dated.

Signed-off-by: Dan Zhang <danzh@google.com>

Augustyniak · 2022-10-10T15:43:27Z

test/cc/unit/envoy_config_test.cc

@@ -156,5 +156,25 @@ TEST(TestConfig, RemainingTemplatesThrows) {
  }
 }

+#if not defined(__APPLE__)


nit: can we add a doc string explaining that this test is effectively not run if being executed on a mac (which is the case currently)?

Instead, I added #else block where calling enablePlatformCertificatesValidation(true) causes death.

Augustyniak · 2022-10-10T15:54:12Z

library/common/extensions/cert_validator/platform_bridge/platform_bridge_cert_validator.cc

+  bssl::UniquePtr<X509> leaf_cert(d2i_X509(
+      nullptr, const_cast<const unsigned char**>(&leaf_cert_der.bytes), leaf_cert_der.length));
+  envoy_cert_validation_result result =
+      platform_validator_->validate_cert(cert_chain.data(), cert_chain.size(), host_name.c_str());


Thank you for a detailed answer(s). I think that the current approach of registering filters makes sense if it helps us to have a better test coverage in here. I wish there was an easier way to make all of this works (actual implementation + tests) - maybe that's something that we will be able to make easier in the future.

The one remaining comment that I have left is the one with regard to moving the registration of the filter from its current place to ...initialize method instead? I left more details in #2525 (comment). What do you think about this one?

Will there be any chance not to run tests under https://github.com/envoyproxy/envoy-mobile/tree/main/test/java/org/chromium/net/testing on MacOS in the future?

If we decided that this is the way to go I think that there is nothing that should prevent us from doing so from the technical point of view. Not entirely sure why we run some of Android tests on macOS and some on Linux but I am guessing that we just wanted to cover testing on more platforms - not sure how useful is that, we can discuss in the community meeting if needed.

Signed-off-by: Dan Zhang <danzh@google.com>

Augustyniak · 2022-10-13T14:55:05Z

test/cc/unit/envoy_config_test.cc

+              HasSubstr("envoy_mobile.cert_validator.platform_bridge_cert_validator"));
+  ASSERT_THAT(bootstrap.DebugString(), Not(HasSubstr("trusted_ca")));
+#else
+  EXPECT_DEATH(engine_builder.enablePlatformCertificatesValidation(true),


this is great - thank you

Augustyniak

Tests are red.

At this point I am fine with merging this PR as is but I left a few tiny comments. It would be great if you could address/respond to them.

Thank you for working with me on this PR - I've learnt a lot about the JIN and c++ engine builder while working/talking with you.

Augustyniak · 2022-10-13T15:00:05Z

library/common/jni/android_jni_interface.cc

+  envoy_status_t result =
+      register_platform_api(cert_validator_name, get_android_cert_validator_api());
+  if (result == ENVOY_FAILURE) {
+    return -1;


I wonder whether we should just return register_platform_api... in here? ENVOY_SUCCESS is equal to 1 which is different than -1 that we return in here but I guess we decided to use ENVOY_SUCCESS/ENVOY_FAILURE constants so shiould be probably fine to return to simplify this and do return register_platform_api...

Thoughts?

discussed offline - looks good the way it is.

Changed to return ENVOY_FAILURE here and made android_test_jni_inteface.cc return directly.

Augustyniak · 2022-10-13T15:03:40Z

library/common/jni/android_test_jni_interface.cc

+  // At this point, we know Android APIs are available. Register cert chain validation JNI calls.
+  envoy_status_t result =
+      register_platform_api(cert_validator_name, get_android_cert_validator_api());
+  if (result == ENVOY_FAILURE) {


same comment with regard to a returns value as in my comment above.

library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 · 2022-10-13T18:03:48Z

/retest

Augustyniak · 2022-10-13T18:22:29Z

/retest

Augustyniak · 2022-10-13T18:32:05Z

@goaway can you re-review/approve this PR? I think that your concerns have been addressed

danzh2010 · 2022-10-17T20:12:20Z

Friendly ping on this :)

Augustyniak · 2022-10-18T12:35:54Z

@goaway I am going to merge this PR as I think that your concerns with regard to configuration template have been addressed.

Description: add engine API to allow user config to use Android cert validation APIs. Risk Level: high Testing: added tests in Http2TestServerTest.java Docs Changes: Release Notes: Fixes envoyproxy#1575 Part of envoyproxy#2144 Signed-off-by: danzh <danzh2010@users.noreply.github.com>

danzh1989 added 14 commits August 18, 2022 19:22

build cert validator

f0a846a

Signed-off-by: Dan Zhang <danzh@google.com>

remove allow expired cert

464f365

Signed-off-by: Dan Zhang <danzh@google.com>

add http2 test

7edc2ef

Signed-off-by: Dan Zhang <danzh@google.com>

host name override

d8e9c6d

Signed-off-by: Dan Zhang <danzh@google.com>

platform_name str

c9894a1

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'main' into certvalidator

b1f9a04

Signed-off-by: Dan Zhang <danzh@google.com>

verify leaf in thread

79443ee

Signed-off-by: Dan Zhang <danzh@google.com>

return value

b40b230

Signed-off-by: Dan Zhang <danzh@google.com>

globalref

93ab2ed

Signed-off-by: Dan Zhang <danzh@google.com>

all tests pass

1e4f295

Signed-off-by: Dan Zhang <danzh@google.com>

EnvoyConfigurationTest pass

e13b72e

Signed-off-by: Dan Zhang <danzh@google.com>

release note

c8fdf91

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'main' into certvalidator

ffa0c1b

Signed-off-by: Dan Zhang <danzh@google.com>

remove unrelated chagne

0a8a474

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 mentioned this pull request Sep 6, 2022

Extend Envoy cert validator to use platform provided APIs #2144

Closed

RyanTheOptimist self-assigned this Sep 6, 2022

danzh1989 added 3 commits September 6, 2022 21:51

fix tests

9012dbb

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'main' into certvalidator

c6cea9d

Signed-off-by: Dan Zhang <danzh@google.com>

fix object-c builder

27129eb

Signed-off-by: Dan Zhang <danzh@google.com>

format

8384259

Signed-off-by: Dan Zhang <danzh@google.com>

RyanTheOptimist reviewed Sep 7, 2022

View reviewed changes

danzh1989 added 5 commits September 9, 2022 19:46

use FakeX509Util in CronetTestRule

3de8d89

Signed-off-by: Dan Zhang <danzh@google.com>

address comments

658a340

Signed-off-by: Dan Zhang <danzh@google.com>

Update submodule

403eca1

Signed-off-by: Dan Zhang <danzh@google.com>

fix build

3a02268

Signed-off-by: Dan Zhang <danzh@google.com>

revert wrong format change

2154876

Signed-off-by: Dan Zhang <danzh@google.com>

danzh1989 added 6 commits September 29, 2022 14:29

revert unrelated format change

09433c4

Signed-off-by: Dan Zhang <danzh@google.com>

fix c++ engine

a481c44

Signed-off-by: Dan Zhang <danzh@google.com>

check platform

406c24f

Signed-off-by: Dan Zhang <danzh@google.com>

fix build

00ecfa4

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'main' into certvalidator

66580a3

Signed-off-by: Dan Zhang <danzh@google.com>

fix envoy_config_test

1a2cc8b

Signed-off-by: Dan Zhang <danzh@google.com>

Augustyniak reviewed Oct 6, 2022

View reviewed changes

fix cpp engine and swift engine config

cebcea3

Signed-off-by: Dan Zhang <danzh@google.com>

Augustyniak reviewed Oct 10, 2022

View reviewed changes

danzh1989 added 7 commits October 12, 2022 16:45

register platform api during initialize

82f87c4

Signed-off-by: Dan Zhang <danzh@google.com>

Merge branch 'main' into certvalidator

ed215f0

Signed-off-by: Dan Zhang <danzh@google.com>

unit test

c4b6198

Signed-off-by: Dan Zhang <danzh@google.com>

revert unrelated change

7fd388b

Signed-off-by: Dan Zhang <danzh@google.com>

deps boringssl

61ab5c8

Signed-off-by: Dan Zhang <danzh@google.com>

split out cert verify calls

3e9418e

Signed-off-by: Dan Zhang <danzh@google.com>

add NOLINT

34526b3

Signed-off-by: Dan Zhang <danzh@google.com>

Augustyniak reviewed Oct 13, 2022

View reviewed changes

Augustyniak previously approved these changes Oct 13, 2022

View reviewed changes

fix build

e815550

Signed-off-by: Dan Zhang <danzh@google.com>

danzh2010 dismissed Augustyniak’s stale review via e815550 October 13, 2022 17:08

Augustyniak merged commit 6b07e7d into envoyproxy:main Oct 18, 2022

Augustyniak mentioned this pull request Oct 18, 2022

Update Envoy #2609

Merged

danzh2010 mentioned this pull request Oct 18, 2022

certificates: coalesce concurrent validations of the same host #2612

Open

cert validation: use Android cert validation APIs #2525

cert validation: use Android cert validation APIs #2525

Conversation

danzh2010 commented Sep 6, 2022 • edited Loading

danzh2010 commented Sep 6, 2022

danzh2010 commented Sep 7, 2022

danzh2010 commented Sep 7, 2022

RyanTheOptimist left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danzh2010 commented Sep 7, 2022

danzh2010 commented Oct 5, 2022

Augustyniak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danzh2010 Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danzh2010 Oct 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Augustyniak Oct 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Augustyniak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danzh2010 commented Oct 13, 2022

Augustyniak commented Oct 13, 2022

Augustyniak commented Oct 13, 2022

danzh2010 commented Oct 17, 2022

Augustyniak commented Oct 18, 2022

danzh2010 commented Sep 6, 2022 •

edited

Loading

danzh2010 Oct 6, 2022 •

edited

Loading

danzh2010 Oct 7, 2022 •

edited

Loading

Augustyniak Oct 6, 2022 •

edited

Loading