Replies: 6 comments
-
Hi @fyears nice to meet you. Just commenting with a question about the PR. After merging would there still be a way to continue improving upon remotely save. One thing I definitely liked was the consistent updates to this plugin and it would be great if that continued with remotely save. But I also understand if you can't actively maintain the project that frequently. |
Beta Was this translation helpful? Give feedback.
-
@fyears thank you for the comment! I will respond point by point later today, just wanted to say I really appreciate you building Remotely Save and for the feedback in this comment. |
Beta Was this translation helpful? Give feedback.
-
Reproducibility:
Thankfully AES-GCM is pretty standard and not home-made, and included in openssl but I do need to re-add documentation and testing code. Definitely planned (#10)! IV: I see where you added a test to ensure encryption is not random, and that makes sense to me (remotely-save@c689235). I think the salt should be 16 bytes (it is 8 bytes) per specification of Pbkdf2Params, but that does make it random even at 8 bytes (and 2^64 is pretty low chance of repeating) but I don't have a full intuition if that is safe. Instead of deriving IV from output of key derivation with random salt a suggestion - why not generate an IV similarly to the salt:
Will update Remotely Sync's readme to reflect my latest understanding of the encryption changes. Backwards compatibility:
Totally understand, I don't want to maintain multiple copies of encryption routines due to the testing demands and change to the algorithm but understand your choice to do so. I think making the suggested changes will improve security of Remotely Save but will require some additional checks for older salt/iv values, etc. Encryption improvements: I do think GCM is an improvement, but the IV -> not derived from key change doesn't seem like as high of a priority given the salt is random (though could be more bits per spec as I mentioned).
It may take a bit of time to open isolated PRs because the plugin has diverged a bit and several new features which interact with each other have been added, like syncing trash folder/deleting trash folder & attempts to refactor the sync routine to make it easier to do a partial sync. Other features are WIP which works fine for a small fork but probably not worth merging until they are complete in the main repo.
100% - I'm happy to contribute changes but I suspect the philosophy of keeping things working for existing users has a different result than our plugin's "move fast and break things" - We don't have regular beta releases and I try to fix things within a day or two if they break, but that obviously won't scale. I plan to keep this fork maintained but that doesn't stop us (or @fyears) from merging some changes upstream! |
Beta Was this translation helpful? Give feedback.
-
Updated readme! |
Beta Was this translation helpful? Give feedback.
-
Ah okay. I like the idea of Remotely Save being a stable version with some of the more polished features in this plugin being merged into it when they're ready. |
Beta Was this translation helpful? Give feedback.
-
Regarding 3.: i think there only some small things where @kepano / the Obsidian team would not be happy with: What we're not okay with:
What we're okay with:
acheong08/obi-sync#19 (comment) They don't seem to have a problem with using "sync" in the name of a plugin. |
Beta Was this translation helpful? Give feedback.
-
Hi @sboesen !
I am @fyears the original author of remotely-save. Though I am still using Obsidian and
remotely-save
for personal projects, I haven't paid attention to the development for a long while... Today I come across your project and I would like to share some thoughts.openssl
part of document.iv
should be different because thesalt
is different. So in my opinion that’s not a security problem.remotely-save
.remotely-sync
in the very beginning because it's very easy to be confused with official Obsidian Sync service. To be honest I don't want to make Obsidian team unhappy and I would like to avoid any legal issues...Beta Was this translation helpful? Give feedback.
All reactions