Skip to content

Encrypted secrets #33

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

Closed
wants to merge 3 commits into from
Closed

Encrypted secrets #33

wants to merge 3 commits into from

Conversation

nicolas-grekas
Copy link
Owner

/cc @jderusse to ease review

@nicolas-grekas nicolas-grekas force-pushed the encrypted-secrets-jd branch 6 times, most recently from 8d84eb5 to c630297 Compare October 15, 2019 22:06
@nicolas-grekas nicolas-grekas force-pushed the encrypted-secrets-jd branch 4 times, most recently from 956712c to b1477e0 Compare October 16, 2019 09:31
if ($this->vault->remove($input->getArgument('name'))) {
$io->success('Secret was successfully removed.');
} else {
$io->text('No secret was found with that name.');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be an exception ? or a red/yellow warning?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the end state is the same (the secret is not available), I don't think so.

@nicolas-grekas
Copy link
Owner Author

Submitted as symfony#33997
Thanks @jderusse for the early reviews.

@nicolas-grekas nicolas-grekas changed the title Encrypted secrets jd Encrypted secrets Oct 16, 2019
fabpot added a commit to symfony/symfony that referenced this pull request Oct 20, 2019
…ecret:...)%` processor to deal with secrets seamlessly (Tobion, jderusse, nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[FrameworkBundle] Add `secrets:*` commands and `%env(secret:...)%` processor to deal with secrets seamlessly

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #27351
| License       | MIT
| Doc PR        | symfony/symfony-docs/pull/11396

This PR continues #31101, please see there for previous discussions. The attached patch has been fine-tuned on nicolas-grekas#33 with @jderusse.

This PR is more opinionated and thus a lot simpler than #31101: only Sodium is supported to encrypt/decrypt (polyfill possible), and only local filesystem is available as a storage, with little to no extension point. That's on purpose: the goal here is to provide an experience, not software building blocks. In 5.1, this might be extended and might lead to a new component, but we'd first need reports from real-world needs. Having this straight-to-the-point in 4.4 will allow gathering these needs (if they exist) and will immediately provide a nice workflow for the need we do want to solve now: forwarding secrets from dev to prod using git in a secure way.

The workflow this will allow is the following:
- public/private key pairs are generated in the `config/secrets/%kernel.environment%/` folder using `bin/console secrets:generate-keys`
- for the prod env, the corresponding private key should be deployed to the server using whatever means the hosting provider allows - this key MUST NOT be committed
- the public key is used to encrypt secrets and thus *may* be committed in the git repository to allow anyone *that can commit* to add secrets - this is done using `bin/console secrets:set`

DI configuration can reference secrets using `%env(secret:...)%` in e.g `services.yaml`.
There is also `bin/console secrets:remove` and `bin/console debug:secrets` to complete the toolbox.

In terms of design, vs #31101, this groups the dual "encoder" + "storage" concepts in a single "vault" one. That's part of what makes this PR simpler.

That's all folks :)

Commits
-------

c4653e1 Restrict secrets management to sodium+filesystem
02b5d74 Add secrets management
8c8f623 Proof of concept for encrypted secrets
nicolas-grekas pushed a commit that referenced this pull request Sep 28, 2020
…rrabus)

This PR was merged into the 5.2-dev branch.

Discussion
----------

[VarDumper] Support for ReflectionAttribute

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | N/A
| License       | MIT
| Doc PR        | not needed

VarDumper currently does not understand that certain reflection objects might have attributes attached to it. Dumping a `ReflectionAttribute` just yields `ReflectionAttribute {symfony#4711}` which is not really helpful. This PR attempts to fix this.

```
ReflectionAttribute {symfony#4711
  name: "App\MyAttribute"
  arguments: array:2 [
    0 => "one"
    "extra" => "hello"
  ]
}
```

While working on this, I noticed that class constants (which can be reflected on since PHP 7.1) are just dumped as plain values, so I've also added a caster for `ReflectionClasConstant` as bonus.

The full output for the `LotsOfAttributes` fixture class that is included with is PR looks like this:

<details>

```
^ ReflectionClass {#7
  +name: "Symfony\Component\VarDumper\Tests\Fixtures\LotsOfAttributes"
  modifiers: "final"
  implements: []
  constants: array:1 [
    0 => ReflectionClassConstant {#20
      +name: "SOME_CONSTANT"
      +class: "Symfony\Component\VarDumper\Tests\Fixtures\LotsOfAttributes"
      modifiers: "public"
      value: "some value"
      attributes: array:2 [
        0 => ReflectionAttribute {#33
          name: "Symfony\Component\VarDumper\Tests\Fixtures\RepeatableAttribute"
          arguments: array:1 [
            0 => "one"
          ]
        }
        1 => ReflectionAttribute {#34
          name: "Symfony\Component\VarDumper\Tests\Fixtures\RepeatableAttribute"
          arguments: array:1 [
            0 => "two"
          ]
        }
      ]
    }
  ]
  properties: array:1 [
    "someProperty" => ReflectionProperty {#19
      +name: "someProperty"
      +class: "Symfony\Component\VarDumper\Tests\Fixtures\LotsOfAttributes"
      modifiers: "private"
      attributes: array:1 [
        0 => ReflectionAttribute {#30
          name: "Symfony\Component\VarDumper\Tests\Fixtures\MyAttribute"
          arguments: array:2 [
            0 => "one"
            "extra" => "hello"
          ]
        }
      ]
    }
  ]
  methods: array:1 [
    "someMethod" => ReflectionMethod {#21
      +name: "someMethod"
      +class: "Symfony\Component\VarDumper\Tests\Fixtures\LotsOfAttributes"
      returnType: "void"
      parameters: {
        $someParameter: ReflectionParameter {#28
          +name: "someParameter"
          position: 0
          attributes: array:1 [
            0 => ReflectionAttribute {#42
              name: "Symfony\Component\VarDumper\Tests\Fixtures\MyAttribute"
              arguments: array:1 [
                0 => "three"
              ]
            }
          ]
          typeHint: "string"
        }
      }
      attributes: array:1 [
        0 => ReflectionAttribute {#27
          name: "Symfony\Component\VarDumper\Tests\Fixtures\MyAttribute"
          arguments: array:1 [
            0 => "two"
          ]
        }
      ]
      modifiers: "public"
    }
  ]
  attributes: array:1 [
    0 => ReflectionAttribute {#22
      name: "Symfony\Component\VarDumper\Tests\Fixtures\MyAttribute"
      arguments: []
    }
  ]
  extra: {
    file: "./src/Symfony/Component/VarDumper/Tests/Fixtures/LotsOfAttributes.php"
    line: "15 to 28"
    isUserDefined: true
  }
}
```

</details>

Commits
-------

34dbf01 [VarDumper] Support for ReflectionAttribute.
nicolas-grekas pushed a commit that referenced this pull request Mar 7, 2024
…hen publishing a message. (jwage)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[Messenger] [Amqp] Handle AMQPConnectionException when publishing a message.

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix symfony#36538 Fix symfony#48241
| License       | MIT

If you have a message handler that dispatches messages to another queue, you can encounter `AMQPConnectionException` with the message "Library error: a SSL error occurred" or "a socket error occurred"  depending on if you are using tls or not or if you are running behind a load balancer or not.

You can manually reproduce this issue by dispatching a message where the handler then dispatches another message to a different queue, then go to rabbitmq admin and close the connection manually, then dispatch another message and when the message handler goes to dispatch the other message, you will get this exception:

```
a socket error occurred
#0 /vagrant/vendor/symfony/amqp-messenger/Transport/AmqpTransport.php(60): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpSender->send()
#1 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(62): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpTransport->send()
#2 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle()
#3 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(61): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle()
#4 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle()
#5 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle()
#6 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle()
#7 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle()
#8 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch()
#9 /vagrant/src/Messenger/MessageBus.php(37): Symfony\Component\Messenger\TraceableMessageBus->dispatch()
#10 /vagrant/vendor/symfony/mailer/Mailer.php(66): App\Messenger\MessageBus->dispatch()
#11 /vagrant/src/Mailer/Mailer.php(83): Symfony\Component\Mailer\Mailer->send()
#12 /vagrant/src/Mailer/Mailer.php(96): App\Mailer\Mailer->send()
#13 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(118): App\Mailer\Mailer->sendEmail()
#14 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(72): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->handle()
#15 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(152): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->__invoke()
#16 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(91): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->callHandler()
#17 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(71): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->handle()
#18 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle()
#19 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(68): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle()
#20 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle()
#21 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle()
#22 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle()
#23 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle()
#24 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch()
#25 /vagrant/vendor/symfony/messenger/RoutableMessageBus.php(54): Symfony\Component\Messenger\TraceableMessageBus->dispatch()
#26 /vagrant/vendor/symfony/messenger/Worker.php(162): Symfony\Component\Messenger\RoutableMessageBus->dispatch()
#27 /vagrant/vendor/symfony/messenger/Worker.php(109): Symfony\Component\Messenger\Worker->handleMessage()
#28 /vagrant/vendor/symfony/messenger/Command/ConsumeMessagesCommand.php(238): Symfony\Component\Messenger\Worker->run()
#29 /vagrant/vendor/symfony/console/Command/Command.php(326): Symfony\Component\Messenger\Command\ConsumeMessagesCommand->execute()
#30 /vagrant/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run()
#31 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(126): Symfony\Component\Console\Application->doRunCommand()
#32 /vagrant/vendor/symfony/console/Application.php(324): Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand()
#33 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(80): Symfony\Component\Console\Application->doRun()
#34 /vagrant/vendor/symfony/console/Application.php(175): Symfony\Bundle\FrameworkBundle\Console\Application->doRun()
#35 /vagrant/vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php(49): Symfony\Component\Console\Application->run()
#36 /vagrant/vendor/autoload_runtime.php(29): Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner->run()
#37 /vagrant/bin/console(11): require_once('...')
#38 {main}
```

TODO:

- [x] Add test for retry logic when publishing messages

Commits
-------

f123370 [Messenger] [Amqp] Handle AMQPConnectionException when publishing a message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants