Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration with secrecy crate for sensitive data #2580

Closed
byronwasti opened this issue Apr 14, 2023 · 5 comments
Closed

Integration with secrecy crate for sensitive data #2580

byronwasti opened this issue Apr 14, 2023 · 5 comments
Labels
enhancement New feature or request server Rust server SDK

Comments

@byronwasti
Copy link

When handling sensitive data that we don't want to have accidentally logged it is helpful to have them wrapped in some sort of Secret<T> type (such as the Secret<T> type from the secrecy crate) which will print something like [REDACTED] instead of the data. As of right now we have to manually do the mapping from the Smithy-rs input types to the wrapped Secret<T> types, which means we have a layer with the sensitive data not wrapped in a redacting type.

If the Smithy-rs input and output generated types provided the sensitive data pre-wrapped in Secret<T> it would remove this layer where we could accidentally log the raw data and make it easier to audit for the .expose_secret() call.

@byronwasti
Copy link
Author

It sounds like this might already be the case if using the @sensitive smithy trait with publicConstrainedShapes enabled

@david-perez david-perez added server Rust server SDK enhancement New feature or request labels Apr 14, 2023
@david-perez
Copy link
Contributor

publicConstrainedShapes is unrelated to handling of the @sensitive trait. @sensitive should be honored regardless. The way it's handled is that we adjust Debug implementations to redact sensitive contents.

So, for example, the following model:

@http(uri: "/operation", method: "POST")
operation Operation {
    input: OperationInputOutput
    output: OperationInputOutput
}

structure OperationInputOutput {
    secretStructure: SecretStructure

    secretString: SecretString

    regularString: String
}

@sensitive
structure SecretStructure {
    password: String
}

@sensitive
string SecretString

Generates:

impl std::fmt::Debug for SecretStructure {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        let mut formatter = f.debug_struct("SecretStructure");
        formatter.field("password", &"*** Sensitive Data Redacted ***");
        formatter.finish()
    }
}
impl std::fmt::Debug for OperationInput {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        let mut formatter = f.debug_struct("OperationInput");
        formatter.field("secret_structure", &"*** Sensitive Data Redacted ***");
        formatter.field("secret_string", &"*** Sensitive Data Redacted ***");
        formatter.field("regular_string", &self.regular_string);
        formatter.finish()
    }
}

An important thing to note here is that the SecretString string shape is generated in Rust as a regular std::string::String. We can't override Debug/Display on a standard library type, so if you log it, you're unprotected. If you think we should newtype @sensitive simple shapes, feel free to weigh in on #1833.

@david-perez
Copy link
Contributor

david-perez commented Apr 14, 2023

(I added the server tag because if we were to do this, we would only be able to do it in the server: adding and removing @sensitive is a backwards-compatible change in the Smithy model, so we can't newtype a shape in Secret<T> because that would be a Rust breaking change.)

@byronwasti
Copy link
Author

Ah ok, I see, thanks for the clarification! It looks like #1833 is what we're looking for. Happy to have this ticket closed as a duplicate of that one

@david-perez
Copy link
Contributor

Sure! Closing in favor of #1833 then.

If anyone stumbles across this issue and would like smithy-rs server SDKs to integrate with secrecy or another crate with a use case that is not covered by the current implementation of @sensitive and #1833, feel free to open a new issue.

@david-perez david-perez closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request server Rust server SDK
Projects
None yet
Development

No branches or pull requests

2 participants