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

pyatls: urllib3 and requests support with cleanup #10

Merged
merged 8 commits into from
Aug 29, 2023

Conversation

HernanGatta
Copy link
Contributor

@HernanGatta HernanGatta commented Aug 27, 2023

Overview

This PR contains various improvements and features. Namely, this PR:

  1. restructures the validators into a deeper hierarchy for classes to have shorter, cleaner names, and so as to create space for additional validators in the future. For example, atls.validators.AzAasAciValidtor becomes atls.validators.azure.aas.aci.AciValidator;
  2. adds support for monkey-patching urllib3 such that, with the patch in place, all HTTPS requests automatically become HTTP/aTLS requests, enabling users of urllib3 to leverage aTLS (this is not required for our usage, I left this here because it was the first idea I had to make requests work with aTLS, but I had a better one afterward and figured I could leave the direct urllib3 support if someone wants to use it);
  3. adds support for aTLS for the requests library via an adapter that captures URLs with the httpa:// scheme and routes them into HTTP/aTLS connections, enabling users of requests to transparently adopt aTLS.

Note: Commits are structured to be reviewed independently.

Requests Adapter

The primary addition of this PR is item no. 3.

Since the requests adapter ties cleanly into the library, consumers of it can seamlessly leverage the library's support for connection pooling. With connection pooling, it is no longer necessary to create a new aTLS connection for every request. Instead, a readily established connection can be recycled for subsequent requests. This in turn dramatically reduces request latency.

The HTTPAAdapter captures requests made against URLs with the httpa:// scheme and uses this package's HTTPAConnection class. This automatically upgrades any requests made with that scheme to HTTP/aTLS.

Sample Usage:

import requests
from atls.utils.requests import HTTPAAdapter
from atls.validators.azure.aas import AciValidator

validator = AciValidator()

session = requests.Session()
session.mount("httpa://", HTTPAAdapter([validator]))

response = session.request("GET", "httpa://my.aci.confidentialservice.net/index")

print(f"Status: {response.status_code}")
print(f"Response: {response.text}")

@HernanGatta HernanGatta added feat New feature for the user refactor Refactoring production code, e.g. renaming a variable. labels Aug 27, 2023
@HernanGatta HernanGatta self-assigned this Aug 27, 2023
@HernanGatta HernanGatta marked this pull request as draft August 27, 2023 17:52
@github-actions
Copy link

github-actions bot commented Aug 27, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ ACTION actionlint 1 0 0.02s
✅ PYTHON black 17 0 0.94s
✅ PYTHON flake8 17 0 0.53s
✅ PYTHON isort 17 0 0.27s
✅ YAML yamllint 5 0 0.27s

See detailed report in MegaLinter reports

You could have the same capabilities but better runtime performances if you use a MegaLinter flavor:

MegaLinter is graciously provided by OX Security

@@ -72,12 +72,12 @@ running on a confidential ACI instance with the corresponding attestation
document issuer, and submit an HTTP request:

```python
from atls import AttestedHTTPSConnection, AttestedTLSContext
from atls import HTTPAConnection, ATLSContext
Copy link

Choose a reason for hiding this comment

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

Would be great to add the httpa example here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.



def use_requests() -> None:
session = requests.Session()
Copy link

Choose a reason for hiding this comment

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

nit: possibly low priority but better to add this as pytest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to add unit tests that run automatically but we need an ACI-capable server instance. I do want to add tests for things that can be tested without a server-side component, though; that's on my to-do list.

# Mount the HTTP/aTLS adapter such that any URL whose scheme is httpa://
# results in an HTTPAConnection object that in turn establishes an aTLS
# connection with the server.
session.mount("httpa://", HTTPAAdapter([validator]))
Copy link

Choose a reason for hiding this comment

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

How to set timeout for the connection in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

session.request(..., timeout=)

if _orig_urllib3_connection_cls is None:
return

import urllib3

Choose a reason for hiding this comment

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

Nit: can we just do this once at the top of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

from urllib3.util.retry import Retry as Retry


class _HTTPAConnectionShim(HTTPAConnection):

Choose a reason for hiding this comment

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

Does this class need to be defined twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look closely, they're slightly different.

Choose a reason for hiding this comment

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

Why can't we use this definition for the inner class in the other spot? The only difference I can see is where validators is gotten from

from urllib3.util.retry import Retry as Retry


class _HTTPAConnectionShim(HTTPAConnection):

Choose a reason for hiding this comment

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

Why can't we use this definition for the inner class in the other spot? The only difference I can see is where validators is gotten from

HTTPAConnection class.
"""

Validators: List[Validator]

Choose a reason for hiding this comment

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

Nit

Suggested change
Validators: List[Validator]
validators: List[Validator]

@HernanGatta HernanGatta merged commit cb13afe into dev Aug 29, 2023
1 check passed
@HernanGatta HernanGatta deleted the hegatta/improvements branch August 29, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature for the user refactor Refactoring production code, e.g. renaming a variable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants