Have the file system signer read the cert files when it needs to in case they have changed #65

krmichelos · 2024-03-01T22:44:11Z

Issue #64
Fixes #64

Added file watching to update certificate contents when they change on disk so that subsequent requests after the original certificate has expired will still succeed.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

krmichelos · 2024-03-18T20:39:45Z

@13ajay, @liwadman, or @dwmw2 are any of you an owner for this repo or know who is?

13ajay

Overall, this looks good - thanks for the PR!

Not to discredit your work, but I'm wondering how useful making this change will be for customers. If the credential helper is used as a long-running process (update or serve), the shortest amount of time between credentials updates is about 10 minutes (shortest temporary credential validity is 15 minutes, and the credential helper will perform rotations five minutes before credential expiry). Since CreateSession calls don't need to be made very frequently, would it make more sense just to read the certificate and private key information right before making the CreateSession call as opposed to tracking changes in file data?

aws_signing_helper/file_system_signer.go

aws_signing_helper/signer_test.go

aws_signing_helper/signer.go

13ajay · 2024-03-18T22:14:43Z

aws_signing_helper/signer.go

+}
+
+// GetCertChain Read a certificate bundle and return a chain of all the certificates is contains
+func GetCertChain(certificateBundleId string) ([]*x509.Certificate, error) {


Also minor, but maybe it would be good to be consistent with the way in which we handle the parsing a single certificate vs. multiple certificates from a file? For a single certificate, I noticed that you added another return value to the existing ReadCertificateData function, but for multiple certificates, you created a new function. Maybe there's a good reason to do it this way though.

ReadCertificateData already had the information that I needed and was discarding it so it seemed easiest to just return the info so that everywhere that called it didn't have to didn't have to have an extra 8 lines to redecode and reparse the cret data and the caller could chose to use or ignore the new return value.

I added GetCertChain to encapsulate the process of appending all the certs from the bundle so that looping didn't need to be repeated when it is called.

If you want I can revert ReadCertificateData and create a GetCertificate that calls ReadCertificateData and then does the decoding and parsing. Up to you, just let me know.

13ajay · 2024-03-18T22:26:42Z

Sorry for the late response - I've added some comments on the PR now. You can feel free to ignore the minor comments - I don't consider them to be blocking.

krmichelos · 2024-03-19T12:50:21Z

Overall, this looks good - thanks for the PR!

Not to discredit your work, but I'm wondering how useful making this change will be for customers. If the credential helper is used as a long-running process (update or serve), the shortest amount of time between credentials updates is about 10 minutes (shortest temporary credential validity is 15 minutes, and the credential helper will perform rotations five minutes before credential expiry). Since CreateSession calls don't need to be made very frequently, would it make more sense just to read the certificate and private key information right before making the CreateSession call as opposed to tracking changes in file data?

A little background for what led me here. I plan on using this as a sidecar container in some pods that are running outside of AWS but need to interact with some AWS services. We plan on using very short lived certificates, so I was testing with certs that were only valid for an hour and were reissued at 30 minutes to exaggerate what we intend and do some testing. That is what helped me notice what was happening. It certainly could be made to read certs before calling the CreateSession. I looked at doing that initially, the conclusion that I came to was that it would make a weird interaction between the credential management process and the Signer interface. The Signer interface was meant to be generic and there are implementations that don't read from files on disk (and certainly more implementations could be added) and the interface doesn't currently have the idea of refreshing the cert(s). I didn't feel comfortable modifying the interface and thus the implementations without input from maintainers or the ability to test the changes for windows or darwin. This coupled with the potential for an indeterminate number of extra reads for all signers, for users with certs that are valid for a year or two, that might cause occasional performance issues; I decided to implement completely inside the FileSystemSigner implementation. I chose to watch the files instead of just reading every time to avoid a significant number of unneeded disk hits.

That being said if you want to shift this to modifying the interface to have a read/refresh sort of function I can certainly go down that path. I would either need someone to fix the implementations for windows/darwin cert stores or at very least test and verify for me if I take a stab at changing those.

I will take a look at and address the other comments. Let me know how you want to proceed with the larger replaced certificates issue discussed above.

krmichelos · 2024-03-19T14:44:47Z

Is there any consideration of having an official image available for this helper?

13ajay · 2024-03-20T15:35:36Z

Thanks for explaining what led you to make this change. Regarding the issue of file watching vs. reading certificate and private key data right before calling CreateSession, I don't think I can make this decision myself, so I will speak with the rest of my team and get back to you within the next few days.

Is there any consideration of having an official image available for this helper?

I'll have to discuss this with my team as well. But in the meantime, to help with prioritization, could you +1 #51?

13ajay · 2024-03-22T22:00:13Z

So it could be that there's something I'm missing, but why does there have to be a modification to the Signer interface in order to support the refreshing of certificates or private keys specifically for the FileSystemSigner? Can't all of that logic just be encapsulated within the Sign (or Certificate, or CertificateChain) method(s)? Basically, just store the path to the private key and certificate files as members of the FileSystemSigner, and in the implementation of Sign, read whatever you need to read from the file system. You could make a similar modification to the Certificate and CertificateChain methods.

This client application also integrates with PKCS#11 and OS-specific certificate stores, and in each of those cases, the private key data is never cached by the credential helper process (it can't directly access the private key data). Instead, the application will search the secure store for the right private key each time the Sign function is called and delegate the signing operation to the secure store. So, if there is an update in private key data, that will be reflected in the call to CreateSession. Maybe, in a sense, you could view that as an inconsistency between the FileSystemSigner and the other Signers (since for the FileSystemSigner, the private key data is effectively cached for the lifetime of the process).

In the certificate rotation scenario, though, you would want the long-running application to call CreateSession with the right certificate (even if it has been updated after the start of the process) without having to restart the process. I think from a consistency perspective, what may make the most sense is to read the certificate from wherever it's supposed to be read from each time, right before calling CreateSession, regardless of the Signer that was used to sign the request. And maybe that means making the update that you suggested to the Signer interface. I suppose the other option is to do the refresh directly within the Sign, Certificate, and CertificateChain methods (as I mentioned for the FileSystemSigner in the first paragraph above), but these methods (at least in this codebase) are called in quick succession (right before the CreateSession call), and if implemented in this way, would probably need to do repeated work (maybe not for the FileSystemSigner though since there is a separate file for each of the private key, certificate, and certificate chain). For example, for the DarwinCertStoreSigner, certificates and private keys are stored together as identities. So, if you search for one of them, you effectively just get the other for free. But if you implement the searching logic directly within the Sign or Certificate methods, you'd be performing some wasted work when you call Certificate and Sign in quick succession. It may make more sense to implement a Refresh method that caches a bunch of information, so that the other methods that are implemented as a part of the Signer interface can use those cached results.

What are your thoughts?

krmichelos · 2024-03-25T13:47:38Z

I obviously don't know much about the OS cert store implementations. They looked to me like they were getting a reference to a cert once, but I will take your word for those references returning updated certs when necessary. So, if the OS cert stores don't have this issue when used in a long running process then we can just focus on the FileSystemSigner. I didn't read the code closely enough to realize that Sign and the other FileSystemSigner functions were only used when preparing to call CreateSession. If that is the case then the functions in FileSystemSigner could be made to read the files on request. That would be an extra 26-144 reads of 2-3 files a day for those with longer expiration periods on their certs. If that isn't a concern and you want this switched to have the FileSystemSigner read the files every time it is used let me know and I can switch it. If that is the case, unless I am missing something else, it seems like all the cached file contents should be removed from the FileSystemSigner and its internal state should only track the paths to the certs/chain since it will be changed to read them when requested. Just curious what is the concern with the file watching?

Let me know what you think.

13ajay · 2024-03-28T22:21:32Z

Sorry, I may not have been clear in my earlier message, but I believe the certificate data is cached for the lifetime of the process in some cases, even when using the OS certificate store implementations. But the private key data necessarily can't be cached (since that data isn't directly accessible by the application). I have to refresh my memory on how it works for the OS certificate store implementations, but I know that at least for the PKCS#11 signer, the private key is searched for again, each time it needs to be used to sign.

If, for other signers, the behavior is, "search for the private key data right before signing," I was thinking that the FileSystemSigner should do the same.

That would be an extra 26-144 reads of 2-3 files a day for those with longer expiration periods on their certs.

I think that's acceptable. You could have cases in which the file changes multiple times between CreateSession calls, and in that case, you would only care about the last update that was made to the certificate data. It's a design tradeoff I guess. In some scenarios, reading the files right before calling CreateSession is better, and in other cases, watching for file updates is better. I'm guessing that usually though, certificates won't need to be rotated that frequently, thus making the file watching approach more efficient.

Just curious what is the concern with the file watching?

One of my concerns includes error handling (what happens if the certificate file is deleted at some point but then reintroduced before the next CreateSession call). I don't remember how your code handles these issues, but I think in general, there's a bit more to reason about in a file watching implementation. In a sense, this can be considered an issue of consistency between the FileSystemSigner and the other Signers (but I do acknowledge that there isn't as much consistency between the implementations right now as there should be). And my other concern is maybe just an extension of the first that I raised. The code complexity becomes greater if we have to support file watching, as compared to reading the necessary files right before calling CreateSession.

krmichelos · 2024-04-02T15:19:55Z

@13ajay I have pushed a change to have the file system signer read certs on demand.

13ajay

Thanks for implementing this!

Would you also be able to run go fmt on the files that you changed as a part of this PR? And if you have the time, could you also add unit testing?

13ajay · 2024-04-03T23:14:09Z

aws_signing_helper/serve.go

+			}
+			credentialProcessOutput, gcErr := GenerateCredentials(opts, signer, signatureAlgorithm)
+			if gcErr != nil {
+				log.Printf("Error generating credentials: %s", gcErr)


Minor: missing a \n at the end of this log line?

13ajay · 2024-04-03T23:15:13Z

aws_signing_helper/serve.go

@@ -240,6 +247,9 @@ func AllIssuesHandlers(cred *RefreshableCred, roleName string, opts *Credentials
 				return
 			}
 		} else {
+			if Debug {
+				log.Println("Using previous obtained credentials")


Minor: should "previous" be "previously" instead?

13ajay · 2024-04-03T23:22:11Z

aws_signing_helper/signer.go

@@ -205,35 +197,22 @@ func GetSigner(opts *CredentialsOpts) (signer Signer, signatureAlgorithm string,
 					" within the PKCS#12 file")
 			}
 			// Not a PEM certificate? Try PKCS#12
-			certificateChain, privateKey, err = ReadPKCS12Data(opts.CertificateId)
+			certificateChain, _, err = ReadPKCS12Data(opts.CertificateId)


Minor: I don't think certificateChain is used here?

13ajay · 2024-04-03T23:25:47Z

aws_signing_helper/signer.go

-		for _, certificate := range certificateChainPointers {
-			certificateChain = append(certificateChain, certificate)
-		}
+		certificateChain = chain


Why assign to certificateChain after checking the error if we will return from the function anyways in the case that there's an error? certificateChain is local anyways, so I don't think there's any risk of assigning nil to it right before returning from the function (in the case of error).

chain is local to the if block and therefore not available when it needs to be returned. certificateChain is scoped to the function and is available on the line when this return is called:
return GetPKCS11Signer(opts.LibPkcs11, certificate, certificateChain, opts.PrivateKeyId, opts.CertificateId, opts.ReusePin)

I may not have been clear in my comment, but I guess I'm asking why you can't do something like certificateChain, err := GetCertChain(opts.CertificateBundleId) instead of first assigning to chain and then assigning to certificateChain. The only place in which chain isn't assigned to certificateChain is in the case of an error (in which case, we would return from the function right away).

13ajay · 2024-04-03T23:30:33Z

aws_signing_helper/signer.go

-				}
-			}
-			return GetFileSystemSigner(privateKey, certificateChain[0], certificateChain)
+			return GetFileSystemSigner(opts.PrivateKeyId, opts.CertificateId, opts.CertificateBundleId, true)


If we're using PKCS#12 here, I think previous assumptions have been that the certificate bundle would be stored in the PKCS#12 container. Not sure if making this change hurts though... What happens in the case that there's a PKCS#12 bundle file referenced with a certificate bundle in it as well as a "normal" certificate bundle file? Will the contents of the two bundles add to each other?

I'm not really sure I follow. The previous implementation was calling ReadPKCS12Data and passing what was read to a file system signer to be cached for the duration of the process. The new implementation is still calling ReadPKCS12Data and if it doesn't get an error back then it passes the same path information that was available here to the new file system signer so that it can call ReadPKCS12Data when it needs to read the certs if isPkcs12 is true. They seem functionally equivalent. Am I not understanding something?

The previous implementation ignores opts.CertificateBundleId when opts.CertificateId contains a path to a PKCS#12 file. In the PKCS#12 case, we expect the certificate bundle to be provided in the PKCS#12 container specified by opts.CertificateId, so technically, opts.CertificateBundleId isn't used at all I believe. In your changes, you're passing in opts.CertificateBundleId into GetFileSystemSigner in order to create the FileSystemSigner. It's possible that they're functionally equivalent, but I wanted to make sure.

When isPkcs12 is true and the FileSystemSigner reads the cert data it ignores opts.PrivateKeyId and opts.CertificateBundleId
https://github.com/aws/rolesanywhere-credential-helper/pull/65/files#diff-9760200f6d4074d48601f0b9219340d8f7706d99fc9066d45527e99eabcbecc7R112-R119

krmichelos · 2024-04-04T13:17:56Z

Thanks for implementing this!

Would you also be able to run go fmt on the files that you changed as a part of this PR? And if you have the time, could you also add unit testing?

Is there something specific in the formatting that you think is off? My IDE runs go fmt on every save. I am adding a .pre-commit-config.yaml just to be sure, but when I run it for all files the only formatting changes are in files I didn't change. I just want to make sure I address the concern. Also, I committed the files that it formatted.

What test case are you wanting. The only test that I see that is using a FileSystemSigner is TestBuildAuthorizationHeader and it isn't testing a long running serve type scenario. Are you wanting me to replace the contents of the cert and private key on disk and then duplicate lines 142-153 to test again?

13ajay · 2024-04-04T22:18:27Z

Is there something specific in the formatting that you think is off?

I thought that maybe you hadn't since I saw a diff that looked like this:

- func (fileSystemSigner FileSystemSigner) Close() {}
+ func (fileSystemSigner *FileSystemSigner) Close() {
+ }

But it's possible that your IDE formats in a slightly different way. Either way, I don't think it really matters - no need to worry about it.

What test case are you wanting.

There isn't anything specific that I had in mind, but I did want to make sure that this code gets tested in some way prior to us merging it. Would you be able to describe how you tested it? I suppose my team will need some way of doing integration testing prior to merging this change, which I can discuss with my colleagues. But I think it would also help to learn how you did testing for these changes yourself.

krmichelos · 2024-04-05T15:25:19Z

I thought that maybe you hadn't since I saw a diff that looked like this:

go fmt doesn't seem to care about the extra linefeed(s). It was just there because I added code to that method in the previous iteration and then removed it. I removed the linefeed.

There isn't anything specific that I had in mind, but I did want to make sure that this code gets tested in some way prior to us merging it. Would you be able to describe how you tested it? I suppose my team will need some way of doing integration testing prior to merging this change, which I can discuss with my colleagues. But I think it would also help to learn how you did testing for these changes yourself.

The testing I did is the same as my initial testing that allowed me to notice the issue. I built an image and added a sidecar to an external-dns pod to allow external-dns to interact with Route53. The pod running the code currently in the PR minus the removed linefeed and unneeded variable has been running for 3 days without crashing and continuing to allow its paired external-dns to function properly while the certificate is replaced every 30 minutes. I also modified the test we discussed to verify the certificate returned by the signer matches the original read from the file and then again after the file has changed.

michaelsauter · 2024-04-11T14:40:42Z

FWIW, I had the same need and quickly put together michaelsauter@5bf1b53 before finding this PR. It differs slightly in that I did not make any change to any signer, instead just added a flag to refresh the signer before use. In general I think I prefer the change done by @krmichelos, I just didn't want to make so many changes :) Looking forward to have this PR merged!

13ajay · 2024-04-19T20:04:48Z

Sorry for the late reply.

I think this looks good, but we will have to do an internal review before this gets merged. I'll let you know when I have an update.

krmichelos · 2024-05-24T11:58:45Z

@13ajay Any update here?

13ajay · 2024-05-24T13:59:16Z

Sorry, I have received some more comments on this. I will post them on this PR later today.

13ajay

Added comments that I got from a review with security.

13ajay · 2024-05-03T20:11:52Z

.pre-commit-config.yaml

@@ -0,0 +1,6 @@
+repos:
+- repo:  https://github.com/dnephin/pre-commit-golang
+  rev: v0.5.1


This was flagged as potentially problematic in the review, as something that could change without us noticing. Maintaining a fork or scripts that implement the same functionality might be fine, but depending on how much utility there is, it might be easier to remove this and not think about it.

It's locked to a specific version. It also only does something if you configure your system to care about it. Do you want me to remove the .pre-commit-config.yaml?

Yes, I would prefer we remove it in the case there are sharp edges with respect to maintenance. That was also the recommendation from our brief internal security review.

Remove pre-commit file

13ajay · 2024-05-03T20:38:16Z

aws_signing_helper/file_system_signer.go

+				log.Printf("Failed to read certificate: %s\n", err)
+				os.Exit(1)
+			}
+		} else if len(chain) > 0 {


What happens when chain has length 0 and there also isn't a certPath? Do we fail gracefully?

I believe cert will be returned as nil. What did the previous implementation do when there was no bundle and no cert? From looking at the code I would guess that you got a file system signer with nil for the certificate and nil for the certificate chain. Would you like an else that logs and os exits?

I'm not sure what the previous behavior was. I think there's no way to trigger this case if you're using the credential helper application traditionally, but you may be able to trigger it if you take a dependency on the repository and use the FileSystemSigner directly. To be safe, let's log and exit as you suggested.

Added else with exit

13ajay · 2024-05-03T20:40:01Z

aws_signing_helper/file_system_signer.go

+		var chain []*x509.Certificate
+		if fileSystemSigner.bundlePath != "" {
+			chain, err = GetCertChain(fileSystemSigner.bundlePath)
+			if err != nil {


If we've already loaded the private key and we fail somewhere else, can there be a leak of private key data? Can we zero the data?

I set privateKey to nil in that paths of code that os.Exit

13ajay · 2024-05-03T20:53:05Z

aws_signing_helper/signer.go

+	for _, certificate := range certificateChainPointers {
+		chain = append(chain, certificate)
+	}
+	return chain, nil


Should we fail in the case that chain has length 0?

ReadCertificateBundleData appears to return an error if the file isn't a valid cert chain. If you would like I can add a check for a 0 length chain and return an "empty cert chain" error. Do you want me to do that?

No, it's fine - I think we can leave it as is.

krmichelos · 2024-05-24T23:39:56Z

@13ajay I have responded to the comments, please let me know how to proceed.

krmichelos · 2024-05-29T17:39:28Z

@13ajay I have made changes and comments. Let me know what you think

krmichelos · 2024-06-03T15:54:08Z

@13ajay how do you want me to proceed on the couple of open conversation items?

13ajay

I apologize for the delay. I've responded to your comments now.

13ajay · 2024-06-06T16:08:29Z

.pre-commit-config.yaml

@@ -0,0 +1,6 @@
+repos:
+- repo:  https://github.com/dnephin/pre-commit-golang
+  rev: v0.5.1


Yes, I would prefer we remove it in the case there are sharp edges with respect to maintenance. That was also the recommendation from our brief internal security review.

13ajay · 2024-06-06T17:24:32Z

aws_signing_helper/file_system_signer.go

+				log.Printf("Failed to read certificate: %s\n", err)
+				os.Exit(1)
+			}
+		} else if len(chain) > 0 {


I'm not sure what the previous behavior was. I think there's no way to trigger this case if you're using the credential helper application traditionally, but you may be able to trigger it if you take a dependency on the repository and use the FileSystemSigner directly. To be safe, let's log and exit as you suggested.

13ajay · 2024-06-06T17:26:39Z

aws_signing_helper/signer.go

+	for _, certificate := range certificateChainPointers {
+		chain = append(chain, certificate)
+	}
+	return chain, nil


No, it's fine - I think we can leave it as is.

…ase they have changed

krmichelos · 2024-06-06T19:34:14Z

@13ajay I have made the requested changes. Let me know what you think.

13ajay

Sorry that this process has taken so long, and thanks for the contribution. Approved.

krmichelos · 2024-06-07T13:54:39Z

Sorry that this process has taken so long, and thanks for the contribution. Approved.

@13ajay No worries, thanks for staying with it and getting it merged. Do you have timeline on when these changes might be available in a released version?

13ajay · 2024-06-10T17:58:15Z

As of now, our next release is scheduled for about two months from now, and these changes should be made available as a part of that release. It's possible (but probably unlikely) that we have another release before then, in which case, these changes would be made available as a part of that release.

13ajay reviewed Mar 18, 2024

View reviewed changes

krmichelos force-pushed the main branch 2 times, most recently from aa68ad6 to a078c4a Compare March 19, 2024 14:41

krmichelos requested a review from 13ajay March 19, 2024 14:45

krmichelos force-pushed the main branch from a078c4a to aa0867e Compare April 2, 2024 15:16

krmichelos changed the title ~~Have the file system signer watch the files on disk for changes and reload them~~ Have the file system signer read the cert files when it needs to in case they have changed Apr 2, 2024

13ajay reviewed Apr 3, 2024

View reviewed changes

krmichelos force-pushed the main branch from aa0867e to 906de75 Compare April 4, 2024 13:19

krmichelos force-pushed the main branch from 906de75 to 778f72f Compare April 5, 2024 15:25

krmichelos requested a review from 13ajay April 8, 2024 23:47

13ajay reviewed May 24, 2024

View reviewed changes

krmichelos force-pushed the main branch from 778f72f to 2481d8d Compare May 24, 2024 23:37

13ajay reviewed Jun 6, 2024

View reviewed changes

Have the file system signer read the cert files when it needs to in c…

44ae9d3

…ase they have changed

krmichelos force-pushed the main branch from 2481d8d to 44ae9d3 Compare June 6, 2024 19:31

13ajay approved these changes Jun 6, 2024

View reviewed changes

13ajay merged commit 9e9f3c4 into aws:main Jun 6, 2024
1 check passed

Have the file system signer read the cert files when it needs to in case they have changed #65

Have the file system signer read the cert files when it needs to in case they have changed #65

Conversation

krmichelos commented Mar 1, 2024 • edited Loading

krmichelos commented Mar 18, 2024

13ajay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

13ajay commented Mar 18, 2024

krmichelos commented Mar 19, 2024

krmichelos commented Mar 19, 2024

13ajay commented Mar 20, 2024

13ajay commented Mar 22, 2024 • edited Loading

krmichelos commented Mar 25, 2024

13ajay commented Mar 28, 2024

krmichelos commented Apr 2, 2024

13ajay 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

krmichelos Apr 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krmichelos Apr 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krmichelos commented Apr 4, 2024 • edited Loading

13ajay commented Apr 4, 2024

krmichelos commented Apr 5, 2024 • edited Loading

michaelsauter commented Apr 11, 2024

13ajay commented Apr 19, 2024

krmichelos commented May 24, 2024

13ajay commented May 24, 2024

13ajay 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

krmichelos commented May 24, 2024

krmichelos commented May 29, 2024

krmichelos commented Jun 3, 2024

13ajay left a comment • 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

krmichelos commented Jun 6, 2024

13ajay left a comment

Choose a reason for hiding this comment

krmichelos commented Jun 7, 2024

13ajay commented Jun 10, 2024

krmichelos commented Mar 1, 2024 •

edited

Loading

13ajay commented Mar 22, 2024 •

edited

Loading

krmichelos Apr 4, 2024 •

edited

Loading

krmichelos Apr 4, 2024 •

edited

Loading

krmichelos commented Apr 4, 2024 •

edited

Loading

krmichelos commented Apr 5, 2024 •

edited

Loading

13ajay left a comment •

edited

Loading