This repository has been archived by the owner on Aug 30, 2022. It is now read-only.
re-introduce the coordinator public key in the message header #514
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This key is actually necessary for security reasons (see [0] Donald T. Davis, "Defective Sign & Encrypt in S/MIME, PKCS#7, MOSS, PEM, PGP, and XML.", Proc. Usenix Tech. Conf. 2001 (Boston, Mass., June 25-30, 2001)) However we still need to implement these checks in the coordinator [0]: http://world.std.com/~dtd/#sign_encrypt
little-dude
requested review from
Robert-Steiner,
janpetschexain and
finiteprods
and removed request for
janpetschexain
Codecov Report
@@ Coverage Diff @@
## multipart-message #514 +/- ##
=====================================================
+ Coverage 58.02% 58.21% +0.18%
=====================================================
Files 63 63
Lines 3021 2996 -25
=====================================================
- Hits 1753 1744 -9
+ Misses 1268 1252 -16
Continue to review full report at Codecov.
|
janpetschexain
approved these changes
Sep 1, 2020
little-dude
added a commit
that referenced
this pull request
Sep 15, 2020
This refactoring started when I tried to split the message parsing service in several components, some which I could re-use for implementing a multipart message service. I tried to make some simplification, and after some yak shaving ended up making quite a lot of changes. This should make it much easier to implement the multipart message service though. ### Add a coordinator public key validation service This is a check we forgot to implement in the past. See also: - http://world.std.com/~dtd/#sign_encrypt - #514 ### `PetError` removal `xaynet_core::PetError` does not exist anymore, so we removed all uses of it. ### Remove the tracing machinery Removed `Request<T>` and the `Traceable` traits. These abstractions were too pervasive and made the code difficult to maintain. Moreover, out tracing implementation relied on `tower_tracing` which still hasn't been released. Removing all these abstractions does makes the code much more readable, and should make it easier to implement the multipart service. ### Flatten the service error type Many services were defined as follow: ```rust impl Service<MyReq> for MyType { type Response = Result<MyResp, MyError>; type Error = MyServiceError; type Future = MyFuture // .. } ``` The future returned by the service then resolved into a `Result<Result<MyResp, MyError>, MyServiceError>`. The idea was that the outer result would be for errors that are specific to the service machinery, while the inner result type would be for business logic only. There are several problems this approach. First it induces complexity and we quickly end up fighting the type checker when we start messing around with the services. Second, there's not always a clear distinction between what is business logic and what is a service implementation detail. Finally, that's a maintenance burden because it forces us to maintain several error types, and convert between them. Therefore, for message services, we unified the error type under `ServiceError`. Now services are defined as: ```rust impl Service<MyReq> for MyType { type Response = MyResp; type Error = MyError; type Future = MyFuture // .. } ``` ### Split out the decryption service Our work for supporting multipart messages led us to consider delegating the encryption layer to TLS at some point. Although there is no short term plan for that, moving the decryption logic into its own service lays the ground for this. Moreover, it allowed us to simplify our tests, because we can now test the message parsing services with non-encrypted messages
little-dude
added a commit
that referenced
this pull request
Sep 15, 2020
This refactoring started when I tried to split the message parsing service in several components, some which I could re-use for implementing a multipart message service. I tried to make some simplification, and after some yak shaving ended up making quite a lot of changes. This should make it much easier to implement the multipart message service though. ### Add a coordinator public key validation service This is a check we forgot to implement in the past. See also: - http://world.std.com/~dtd/#sign_encrypt - #514 ### `PetError` removal Only one out of the three variants is used in `xaynet_core` and `xaynet_server`, and two in `xaynet_client`, so there is no point sharing that error type accross the crates. ### Remove the tracing machinery Removed `Request<T>` and the `Traceable` traits. These abstractions were too pervasive and made the code difficult to maintain. Moreover, out tracing implementation relied on `tower_tracing` which still hasn't been released. Removing all these abstractions does makes the code much more readable, and should make it easier to implement the multipart service. ### Flatten the service error type Many services were defined as follow: ```rust impl Service<MyReq> for MyType { type Response = Result<MyResp, MyError>; type Error = MyServiceError; type Future = MyFuture // .. } ``` The future returned by the service then resolved into a `Result<Result<MyResp, MyError>, MyServiceError>`. The idea was that the outer result would be for errors that are specific to the service machinery, while the inner result type would be for business logic only. There are several problems this approach. First it induces complexity and we quickly end up fighting the type checker when we start messing around with the services. Second, there's not always a clear distinction between what is business logic and what is a service implementation detail. Finally, that's a maintenance burden because it forces us to maintain several error types, and convert between them. Therefore, for message services, we unified the error type under `ServiceError`. Now services are defined as: ```rust impl Service<MyReq> for MyType { type Response = MyResp; type Error = MyError; type Future = MyFuture // .. } ``` ### Split out the decryption service Our work for supporting multipart messages led us to consider delegating the encryption layer to TLS at some point. Although there is no short term plan for that, moving the decryption logic into its own service lays the ground for this. Moreover, it allowed us to simplify our tests, because we can now test the message parsing services with non-encrypted messages
little-dude
added a commit
that referenced
this pull request
Sep 16, 2020
This refactoring started when I tried to split the message parsing service in several components, some which I could re-use for implementing a multipart message service. I tried to make some simplification, and after some yak shaving ended up making quite a lot of changes. This should make it much easier to implement the multipart message service though. ### Add a coordinator public key validation service This is a check we forgot to implement in the past. See also: - http://world.std.com/~dtd/#sign_encrypt - #514 ### `PetError` removal Only one out of the three variants is used in `xaynet_core` and `xaynet_server`, and two in `xaynet_client`, so there is no point sharing that error type accross the crates. ### Remove the tracing machinery Removed `Request<T>` and the `Traceable` traits. These abstractions were too pervasive and made the code difficult to maintain. Moreover, out tracing implementation relied on `tower_tracing` which still hasn't been released. Removing all these abstractions does makes the code much more readable, and should make it easier to implement the multipart service. ### Flatten the service error type Many services were defined as follow: ```rust impl Service<MyReq> for MyType { type Response = Result<MyResp, MyError>; type Error = MyServiceError; type Future = MyFuture // .. } ``` The future returned by the service then resolved into a `Result<Result<MyResp, MyError>, MyServiceError>`. The idea was that the outer result would be for errors that are specific to the service machinery, while the inner result type would be for business logic only. There are several problems this approach. First it induces complexity and we quickly end up fighting the type checker when we start messing around with the services. Second, there's not always a clear distinction between what is business logic and what is a service implementation detail. Finally, that's a maintenance burden because it forces us to maintain several error types, and convert between them. Therefore, for message services, we unified the error type under `ServiceError`. Now services are defined as: ```rust impl Service<MyReq> for MyType { type Response = MyResp; type Error = MyError; type Future = MyFuture // .. } ``` ### Split out the decryption service Our work for supporting multipart messages led us to consider delegating the encryption layer to TLS at some point. Although there is no short term plan for that, moving the decryption logic into its own service lays the ground for this. Moreover, it allowed us to simplify our tests, because we can now test the message parsing services with non-encrypted messages
little-dude
added a commit
that referenced
this pull request
Sep 17, 2020
This refactoring started when I tried to split the message parsing service in several components, some which I could re-use for implementing a multipart message service. I tried to make some simplification, and after some yak shaving ended up making quite a lot of changes. This should make it much easier to implement the multipart message service though. ### Add a coordinator public key validation service This is a check we forgot to implement in the past. See also: - http://world.std.com/~dtd/#sign_encrypt - #514 ### `PetError` removal Only one out of the three variants is used in `xaynet_core` and `xaynet_server`, and two in `xaynet_client`, so there is no point sharing that error type accross the crates. ### Remove the tracing machinery Removed `Request<T>` and the `Traceable` traits. These abstractions were too pervasive and made the code difficult to maintain. Moreover, out tracing implementation relied on `tower_tracing` which still hasn't been released. Removing all these abstractions does makes the code much more readable, and should make it easier to implement the multipart service. ### Flatten the service error type Many services were defined as follow: ```rust impl Service<MyReq> for MyType { type Response = Result<MyResp, MyError>; type Error = MyServiceError; type Future = MyFuture // .. } ``` The future returned by the service then resolved into a `Result<Result<MyResp, MyError>, MyServiceError>`. The idea was that the outer result would be for errors that are specific to the service machinery, while the inner result type would be for business logic only. There are several problems this approach. First it induces complexity and we quickly end up fighting the type checker when we start messing around with the services. Second, there's not always a clear distinction between what is business logic and what is a service implementation detail. Finally, that's a maintenance burden because it forces us to maintain several error types, and convert between them. Therefore, for message services, we unified the error type under `ServiceError`. Now services are defined as: ```rust impl Service<MyReq> for MyType { type Response = MyResp; type Error = MyError; type Future = MyFuture // .. } ``` ### Split out the decryption service Our work for supporting multipart messages led us to consider delegating the encryption layer to TLS at some point. Although there is no short term plan for that, moving the decryption logic into its own service lays the ground for this. Moreover, it allowed us to simplify our tests, because we can now test the message parsing services with non-encrypted messages
little-dude
added a commit
that referenced
this pull request
Sep 17, 2020
This refactoring started when I tried to split the message parsing service in several components, some which I could re-use for implementing a multipart message service. I tried to make some simplification, and after some yak shaving ended up making quite a lot of changes. This should make it much easier to implement the multipart message service though. ### Add a coordinator public key validation service This is a check we forgot to implement in the past. See also: - http://world.std.com/~dtd/#sign_encrypt - #514 ### `PetError` removal Only one out of the three variants is used in `xaynet_core` and `xaynet_server`, and two in `xaynet_client`, so there is no point sharing that error type accross the crates. ### Remove the tracing machinery Removed `Request<T>` and the `Traceable` traits. These abstractions were too pervasive and made the code difficult to maintain. Moreover, out tracing implementation relied on `tower_tracing` which still hasn't been released. Removing all these abstractions does makes the code much more readable, and should make it easier to implement the multipart service. ### Flatten the service error type Many services were defined as follow: ```rust impl Service<MyReq> for MyType { type Response = Result<MyResp, MyError>; type Error = MyServiceError; type Future = MyFuture // .. } ``` The future returned by the service then resolved into a `Result<Result<MyResp, MyError>, MyServiceError>`. The idea was that the outer result would be for errors that are specific to the service machinery, while the inner result type would be for business logic only. There are several problems this approach. First it induces complexity and we quickly end up fighting the type checker when we start messing around with the services. Second, there's not always a clear distinction between what is business logic and what is a service implementation detail. Finally, that's a maintenance burden because it forces us to maintain several error types, and convert between them. Therefore, for message services, we unified the error type under `ServiceError`. Now services are defined as: ```rust impl Service<MyReq> for MyType { type Response = MyResp; type Error = MyError; type Future = MyFuture // .. } ``` ### Split out the decryption service Our work for supporting multipart messages led us to consider delegating the encryption layer to TLS at some point. Although there is no short term plan for that, moving the decryption logic into its own service lays the ground for this. Moreover, it allowed us to simplify our tests, because we can now test the message parsing services with non-encrypted messages
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This key is actually necessary for security reasons (see 0 Donald
T. Davis, "Defective Sign & Encrypt in S/MIME, PKCS#7, MOSS, PEM, PGP,
and XML.", Proc. Usenix Tech. Conf. 2001 (Boston, Mass., June 25-30,
2001))
However we still need to implement these checks in the coordinator